Author: rtrieu Date: Fri Sep 20 19:37:10 2019 New Revision: 372448 URL: http://llvm.org/viewvc/llvm-project?rev=372448&view=rev Log: Improve -Wtautological-overlap-compare
Allow this warning to detect a larger number of constant values, including negative numbers, and handle non-int types better. Differential Revision: https://reviews.llvm.org/D66044 Modified: cfe/trunk/docs/ReleaseNotes.rst cfe/trunk/lib/Analysis/CFG.cpp cfe/trunk/lib/Analysis/ReachableCode.cpp cfe/trunk/test/Analysis/cfg.cpp cfe/trunk/test/Sema/warn-overlap.c cfe/trunk/test/Sema/warn-unreachable.c cfe/trunk/test/SemaCXX/warn-unreachable.cpp Modified: cfe/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=372448&r1=372447&r2=372448&view=diff ============================================================================== --- cfe/trunk/docs/ReleaseNotes.rst (original) +++ cfe/trunk/docs/ReleaseNotes.rst Fri Sep 20 19:37:10 2019 @@ -51,7 +51,8 @@ Major New Features Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- ... +- -Wtautological-overlap-compare will warn on negative numbers and non-int + types. Non-comprehensive list of changes in this release ------------------------------------------------- Modified: cfe/trunk/lib/Analysis/CFG.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=372448&r1=372447&r2=372448&view=diff ============================================================================== --- cfe/trunk/lib/Analysis/CFG.cpp (original) +++ cfe/trunk/lib/Analysis/CFG.cpp Fri Sep 20 19:37:10 2019 @@ -70,11 +70,35 @@ static SourceLocation GetEndLoc(Decl *D) return D->getLocation(); } +/// Returns true on constant values based around a single IntegerLiteral. +/// Allow for use of parentheses, integer casts, and negative signs. +static bool IsIntegerLiteralConstantExpr(const Expr *E) { + // Allow parentheses + E = E->IgnoreParens(); + + // Allow conversions to different integer kind. + if (const auto *CE = dyn_cast<CastExpr>(E)) { + if (CE->getCastKind() != CK_IntegralCast) + return false; + E = CE->getSubExpr(); + } + + // Allow negative numbers. + if (const auto *UO = dyn_cast<UnaryOperator>(E)) { + if (UO->getOpcode() != UO_Minus) + return false; + E = UO->getSubExpr(); + } + + return isa<IntegerLiteral>(E); +} + /// Helper for tryNormalizeBinaryOperator. Attempts to extract an IntegerLiteral -/// or EnumConstantDecl from the given Expr. If it fails, returns nullptr. +/// constant expression or EnumConstantDecl from the given Expr. If it fails, +/// returns nullptr. static const Expr *tryTransformToIntOrEnumConstant(const Expr *E) { E = E->IgnoreParens(); - if (isa<IntegerLiteral>(E)) + if (IsIntegerLiteralConstantExpr(E)) return E; if (auto *DR = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts())) return isa<EnumConstantDecl>(DR->getDecl()) ? DR : nullptr; @@ -121,11 +145,11 @@ tryNormalizeBinaryOperator(const BinaryO static bool areExprTypesCompatible(const Expr *E1, const Expr *E2) { // User intent isn't clear if they're mixing int literals with enum // constants. - if (isa<IntegerLiteral>(E1) != isa<IntegerLiteral>(E2)) + if (isa<DeclRefExpr>(E1) != isa<DeclRefExpr>(E2)) return false; // Integer literal comparisons, regardless of literal type, are acceptable. - if (isa<IntegerLiteral>(E1)) + if (!isa<DeclRefExpr>(E1)) return true; // IntegerLiterals are handled above and only EnumConstantDecls are expected @@ -1081,6 +1105,10 @@ private: // * Variable x is equal to the largest literal. // * Variable x is greater than largest literal. bool AlwaysTrue = true, AlwaysFalse = true; + // Track value of both subexpressions. If either side is always + // true/false, another warning should have already been emitted. + bool LHSAlwaysTrue = true, LHSAlwaysFalse = true; + bool RHSAlwaysTrue = true, RHSAlwaysFalse = true; for (const llvm::APSInt &Value : Values) { TryResult Res1, Res2; Res1 = analyzeLogicOperatorCondition(BO1, Value, L1); @@ -1096,10 +1124,16 @@ private: AlwaysTrue &= (Res1.isTrue() || Res2.isTrue()); AlwaysFalse &= !(Res1.isTrue() || Res2.isTrue()); } + + LHSAlwaysTrue &= Res1.isTrue(); + LHSAlwaysFalse &= Res1.isFalse(); + RHSAlwaysTrue &= Res2.isTrue(); + RHSAlwaysFalse &= Res2.isFalse(); } if (AlwaysTrue || AlwaysFalse) { - if (BuildOpts.Observer) + if (!LHSAlwaysTrue && !LHSAlwaysFalse && !RHSAlwaysTrue && + !RHSAlwaysFalse && BuildOpts.Observer) BuildOpts.Observer->compareAlwaysTrue(B, AlwaysTrue); return TryResult(AlwaysTrue); } Modified: cfe/trunk/lib/Analysis/ReachableCode.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ReachableCode.cpp?rev=372448&r1=372447&r2=372448&view=diff ============================================================================== --- cfe/trunk/lib/Analysis/ReachableCode.cpp (original) +++ cfe/trunk/lib/Analysis/ReachableCode.cpp Fri Sep 20 19:37:10 2019 @@ -247,7 +247,7 @@ static bool isConfigurationValue(const S } case Stmt::UnaryOperatorClass: { const UnaryOperator *UO = cast<UnaryOperator>(S); - if (UO->getOpcode() != UO_LNot) + if (UO->getOpcode() != UO_LNot && UO->getOpcode() != UO_Minus) return false; bool SilenceableCondValNotSet = SilenceableCondVal && SilenceableCondVal->getBegin().isInvalid(); Modified: cfe/trunk/test/Analysis/cfg.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg.cpp?rev=372448&r1=372447&r2=372448&view=diff ============================================================================== --- cfe/trunk/test/Analysis/cfg.cpp (original) +++ cfe/trunk/test/Analysis/cfg.cpp Fri Sep 20 19:37:10 2019 @@ -547,6 +547,27 @@ int foo() { } } // namespace statement_expression_in_return +// CHECK-LABEL: int overlap_compare(int x) +// CHECK: [B2] +// CHECK-NEXT: 1: 1 +// CHECK-NEXT: 2: return [B2.1]; +// CHECK-NEXT: Preds (1): B3(Unreachable) +// CHECK-NEXT: Succs (1): B0 +// CHECK: [B3] +// CHECK-NEXT: 1: x +// CHECK-NEXT: 2: [B3.1] (ImplicitCastExpr, LValueToRValue, int) +// CHECK-NEXT: 3: 5 +// CHECK-NEXT: 4: [B3.2] > [B3.3] +// CHECK-NEXT: T: if [B4.5] && [B3.4] +// CHECK-NEXT: Preds (1): B4 +// CHECK-NEXT: Succs (2): B2(Unreachable) B1 +int overlap_compare(int x) { + if (x == -1 && x > 5) + return 1; + + return 2; +} + // CHECK-LABEL: template<> int *PR18472<int>() // CHECK: [B2 (ENTRY)] // CHECK-NEXT: Succs (1): B1 Modified: cfe/trunk/test/Sema/warn-overlap.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-overlap.c?rev=372448&r1=372447&r2=372448&view=diff ============================================================================== --- cfe/trunk/test/Sema/warn-overlap.c (original) +++ cfe/trunk/test/Sema/warn-overlap.c Fri Sep 20 19:37:10 2019 @@ -141,3 +141,20 @@ int returns(int x) { return x < 1 || x != 0; // expected-warning@-1{{overlapping comparisons always evaluate to true}} } + +int integer_conversion(unsigned x, int y) { + return x > 4 || x < 10; + // expected-warning@-1{{overlapping comparisons always evaluate to true}} + return y > 4u || y < 10u; + // expected-warning@-1{{overlapping comparisons always evaluate to true}} +} + +int negative_compare(int x) { + return x > -1 || x < 1; + // expected-warning@-1{{overlapping comparisons always evaluate to true}} +} + +int no_warning(unsigned x) { + return x >= 0 || x == 1; + // no warning since "x >= 0" is caught by a different tautological warning. +} Modified: cfe/trunk/test/Sema/warn-unreachable.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-unreachable.c?rev=372448&r1=372447&r2=372448&view=diff ============================================================================== --- cfe/trunk/test/Sema/warn-unreachable.c (original) +++ cfe/trunk/test/Sema/warn-unreachable.c Fri Sep 20 19:37:10 2019 @@ -433,7 +433,7 @@ void wrapOneInFixit(struct StructWithPoi } void unaryOpNoFixit() { - if (- 1) + if (~ 1) return; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]] unaryOpNoFixit(); // expected-warning {{code will never be executed}} } Modified: cfe/trunk/test/SemaCXX/warn-unreachable.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unreachable.cpp?rev=372448&r1=372447&r2=372448&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/warn-unreachable.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-unreachable.cpp Fri Sep 20 19:37:10 2019 @@ -393,6 +393,9 @@ void tautological_compare(bool x, int y) else calledFun(); // expected-warning {{will never be executed}} + if (y == -1 && y != -1) // expected-note {{silence}} + calledFun(); // expected-warning {{will never be executed}} + // TODO: Extend warning to the following code: if (x < -1) calledFun(); @@ -408,6 +411,4 @@ void tautological_compare(bool x, int y) else calledFun(); - if (y == -1 && y != -1) - calledFun(); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
