Index: cfe/trunk/docs/ReleaseNotes.rst =================================================================== --- cfe/trunk/docs/ReleaseNotes.rst +++ cfe/trunk/docs/ReleaseNotes.rst @@ -51,7 +51,8 @@ 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 ------------------------------------------------- Index: cfe/trunk/lib/Analysis/CFG.cpp =================================================================== --- cfe/trunk/lib/Analysis/CFG.cpp +++ cfe/trunk/lib/Analysis/CFG.cpp @@ -70,11 +70,35 @@ 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(E)) { + if (CE->getCastKind() != CK_IntegralCast) + return false; + E = CE->getSubExpr(); + } + + // Allow negative numbers. + if (const auto *UO = dyn_cast(E)) { + if (UO->getOpcode() != UO_Minus) + return false; + E = UO->getSubExpr(); + } + + return isa(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(E)) + if (IsIntegerLiteralConstantExpr(E)) return E; if (auto *DR = dyn_cast(E->IgnoreParenImpCasts())) return isa(DR->getDecl()) ? DR : nullptr; @@ -121,11 +145,11 @@ 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(E1) != isa(E2)) + if (isa(E1) != isa(E2)) return false; // Integer literal comparisons, regardless of literal type, are acceptable. - if (isa(E1)) + if (!isa(E1)) return true; // IntegerLiterals are handled above and only EnumConstantDecls are expected @@ -1081,6 +1105,10 @@ // * 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 @@ 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); } Index: cfe/trunk/lib/Analysis/ReachableCode.cpp =================================================================== --- cfe/trunk/lib/Analysis/ReachableCode.cpp +++ cfe/trunk/lib/Analysis/ReachableCode.cpp @@ -247,7 +247,7 @@ } case Stmt::UnaryOperatorClass: { const UnaryOperator *UO = cast(S); - if (UO->getOpcode() != UO_LNot) + if (UO->getOpcode() != UO_LNot && UO->getOpcode() != UO_Minus) return false; bool SilenceableCondValNotSet = SilenceableCondVal && SilenceableCondVal->getBegin().isInvalid(); Index: cfe/trunk/test/Analysis/cfg.cpp =================================================================== --- cfe/trunk/test/Analysis/cfg.cpp +++ cfe/trunk/test/Analysis/cfg.cpp @@ -547,6 +547,27 @@ } } // 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() // CHECK: [B2 (ENTRY)] // CHECK-NEXT: Succs (1): B1 Index: cfe/trunk/test/Sema/warn-overlap.c =================================================================== --- cfe/trunk/test/Sema/warn-overlap.c +++ cfe/trunk/test/Sema/warn-overlap.c @@ -141,3 +141,20 @@ 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. +} Index: cfe/trunk/test/Sema/warn-unreachable.c =================================================================== --- cfe/trunk/test/Sema/warn-unreachable.c +++ cfe/trunk/test/Sema/warn-unreachable.c @@ -433,7 +433,7 @@ } void unaryOpNoFixit() { - if (- 1) + if (~ 1) return; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]] unaryOpNoFixit(); // expected-warning {{code will never be executed}} } Index: cfe/trunk/test/SemaCXX/warn-unreachable.cpp =================================================================== --- cfe/trunk/test/SemaCXX/warn-unreachable.cpp +++ cfe/trunk/test/SemaCXX/warn-unreachable.cpp @@ -393,6 +393,9 @@ 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 @@ else calledFun(); - if (y == -1 && y != -1) - calledFun(); }