From 90a8f27f144233b53cac0c88a1595f7f05105b7e Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Tue, 13 Jul 2010 19:41:32 +0000 Subject: [PATCH] Add a warning to catch a bug recently caught by code review, like this: t2.c:2:12: warning: use of logical && with constant operand; switch to bitwise & or remove constant [-Wlogical-bitwise-confusion] return x && 4; ^ ~ wording improvement suggestions are welcome. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@108260 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 4 ++++ lib/Sema/Sema.h | 2 +- lib/Sema/SemaExpr.cpp | 18 ++++++++++++++++-- test/CodeGenCXX/static-init-2.cpp | 2 +- test/Sema/exprs.c | 3 +++ test/Sema/i-c-e.c | 3 ++- test/Sema/switch.c | 6 ++++-- test/SemaCXX/bool.cpp | 4 ++-- test/SemaCXX/switch.cpp | 3 ++- 9 files changed, 35 insertions(+), 10 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 8fac1edecf..f6542afa07 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1986,6 +1986,10 @@ def note_precedence_bitwise_first : Note< "place parentheses around the %0 expression to evaluate it first">; def note_precedence_bitwise_silence : Note< "place parentheses around the %0 expression to silence this warning">; + +def warn_logical_instead_of_bitwise : Warning< + "use of logical %0 with constant operand; switch to bitwise %1 or " + "remove constant">, InGroup>; def err_sizeof_nonfragile_interface : Error< "invalid application of '%select{alignof|sizeof}1' to interface %0 in " diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index c4153b6cab..d902dfea1f 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -4353,7 +4353,7 @@ public: QualType CheckBitwiseOperands( // C99 6.5.[10...12] Expr *&lex, Expr *&rex, SourceLocation OpLoc, bool isCompAssign = false); QualType CheckLogicalOperands( // C99 6.5.[13,14] - Expr *&lex, Expr *&rex, SourceLocation OpLoc); + Expr *&lex, Expr *&rex, SourceLocation OpLoc, unsigned Opc); // CheckAssignmentOperands is used for both simple and compound assignment. // For simple assignment, pass both expressions and a null converted type. // For compound assignment, pass both expressions and the converted type. diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 5d53ddeb98..cb3518ae83 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -5735,7 +5735,21 @@ inline QualType Sema::CheckBitwiseOperands( } inline QualType Sema::CheckLogicalOperands( // C99 6.5.[13,14] - Expr *&lex, Expr *&rex, SourceLocation Loc) { + Expr *&lex, Expr *&rex, SourceLocation Loc, unsigned Opc) { + + // Diagnose cases where the user write a logical and/or but probably meant a + // bitwise one. We do this when the LHS is a non-bool integer and the RHS + // is a constant. + if (lex->getType()->isIntegerType() && !lex->getType()->isBooleanType() && + rex->getType()->isIntegerType() && rex->isEvaluatable(Context) && + !Loc.isMacroID()) + Diag(Loc, diag::warn_logical_instead_of_bitwise) + << rex->getSourceRange() + << (Opc == BinaryOperator::LAnd ? "&&" : "||") + << (Opc == BinaryOperator::LAnd ? "&" : "|"); + + + if (!Context.getLangOptions().CPlusPlus) { UsualUnaryConversions(lex); UsualUnaryConversions(rex); @@ -6363,7 +6377,7 @@ Action::OwningExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, break; case BinaryOperator::LAnd: case BinaryOperator::LOr: - ResultTy = CheckLogicalOperands(lhs, rhs, OpLoc); + ResultTy = CheckLogicalOperands(lhs, rhs, OpLoc, Opc); break; case BinaryOperator::MulAssign: case BinaryOperator::DivAssign: diff --git a/test/CodeGenCXX/static-init-2.cpp b/test/CodeGenCXX/static-init-2.cpp index 65ab3bb126..7eb4a7d5aa 100644 --- a/test/CodeGenCXX/static-init-2.cpp +++ b/test/CodeGenCXX/static-init-2.cpp @@ -3,4 +3,4 @@ // Make sure we don't crash generating y; its value is constant, but the // initializer has side effects, so EmitConstantExpr should fail. int x(); -int y = x() && 0; +int y = x() && 0; // expected-warning {{use of logical && with constant operand}} diff --git a/test/Sema/exprs.c b/test/Sema/exprs.c index b22b5220f2..54e44693e0 100644 --- a/test/Sema/exprs.c +++ b/test/Sema/exprs.c @@ -142,3 +142,6 @@ void test19() { *(volatile int*)0 = 0; // Ok. } +int test20(int x) { + return x && 4; // expected-warning {{use of logical && with constant operand; switch to bitwise & or remove constant}} +} diff --git a/test/Sema/i-c-e.c b/test/Sema/i-c-e.c index c86a93fed8..eb77bbe3b9 100644 --- a/test/Sema/i-c-e.c +++ b/test/Sema/i-c-e.c @@ -51,7 +51,8 @@ char z[__builtin_constant_p(4) ? 1 : -1]; // Comma tests int comma1[0?1,2:3]; // expected-warning {{expression result unused}} -int comma2[1||(1,2)]; // expected-warning {{expression result unused}} +int comma2[1||(1,2)]; // expected-warning {{expression result unused}} \ + // expected-warning {{use of logical || with constant operand}} int comma3[(1,2)]; // expected-warning {{size of static array must be an integer constant expression}} \ // expected-warning {{expression result unused}} diff --git a/test/Sema/switch.c b/test/Sema/switch.c index bb4822916c..4e39e0f0c2 100644 --- a/test/Sema/switch.c +++ b/test/Sema/switch.c @@ -50,12 +50,14 @@ void test4() } switch (cond) { - case g() && 0: // expected-error {{expression is not an integer constant expression}} // expected-note {{subexpression not valid in an integer constant expression}} + case g() && 0: // expected-error {{expression is not an integer constant expression}} // expected-note {{subexpression not valid in an integer constant expression}} \ + expected-warning {{use of logical && with constant operand}} break; } switch (cond) { - case 0 ... g() || 1: // expected-error {{expression is not an integer constant expression}} // expected-note {{subexpression not valid in an integer constant expression}} + case 0 ... g() || 1: // expected-error {{expression is not an integer constant expression}} // expected-note {{subexpression not valid in an integer constant expression}} \\ + expected-warning {{use of logical || with constant operand}} break; } } diff --git a/test/SemaCXX/bool.cpp b/test/SemaCXX/bool.cpp index 44e17ce62b..726fa6cb60 100644 --- a/test/SemaCXX/bool.cpp +++ b/test/SemaCXX/bool.cpp @@ -25,6 +25,6 @@ void static_assert_arg_is_bool(T x) { void test2() { int n = 2; - static_assert_arg_is_bool(n && 4); - static_assert_arg_is_bool(n || 5); + static_assert_arg_is_bool(n && 4); // expected-warning {{use of logical && with constant operand}} + static_assert_arg_is_bool(n || 5); // expected-warning {{use of logical || with constant operand}} } diff --git a/test/SemaCXX/switch.cpp b/test/SemaCXX/switch.cpp index fc13630bbf..54240dcc30 100644 --- a/test/SemaCXX/switch.cpp +++ b/test/SemaCXX/switch.cpp @@ -8,7 +8,8 @@ void test() { } int n = 3; - switch (n && 1) { // expected-warning {{bool}} + switch (n && 1) { // expected-warning {{bool}} \ + // expected-warning {{use of logical && with constant operand}} case 1: break; } -- 2.40.0