Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -355,6 +355,10 @@ - The Fix-It emitted for unused labels used to expand to the next line, which caused visual oddities now that Clang shows more than one line of code snippet. This has been fixed and the Fix-It now only spans to the end of the ``:``. +- Clang's ``-Wtautological-negation-compare`` flag now diagnoses logical + tautologies like ``x && !x`` and ``!x || x`` in expressions. This also + makes ``-Winfinite-recursion`` diagnose more cases. + (`#56035: `_). Bug Fixes in This Version ------------------------- Index: clang/include/clang/Analysis/CFG.h =================================================================== --- clang/include/clang/Analysis/CFG.h +++ clang/include/clang/Analysis/CFG.h @@ -1209,6 +1209,7 @@ CFGCallback() = default; virtual ~CFGCallback() = default; + virtual void logicAlwaysTrue(const BinaryOperator *B, bool isAlwaysTrue) {} virtual void compareAlwaysTrue(const BinaryOperator *B, bool isAlwaysTrue) {} virtual void compareBitwiseEquality(const BinaryOperator *B, bool isAlwaysTrue) {} Index: clang/include/clang/Basic/DiagnosticGroups.td =================================================================== --- clang/include/clang/Basic/DiagnosticGroups.td +++ clang/include/clang/Basic/DiagnosticGroups.td @@ -667,13 +667,15 @@ def TautologicalBitwiseCompare : DiagGroup<"tautological-bitwise-compare">; def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">; def TautologicalObjCBoolCompare : DiagGroup<"tautological-objc-bool-compare">; +def TautologicalNegationCompare : DiagGroup<"tautological-negation-compare">; def TautologicalCompare : DiagGroup<"tautological-compare", [TautologicalConstantCompare, TautologicalPointerCompare, TautologicalOverlapCompare, TautologicalBitwiseCompare, TautologicalUndefinedCompare, - TautologicalObjCBoolCompare]>; + TautologicalObjCBoolCompare, + TautologicalNegationCompare]>; def HeaderHygiene : DiagGroup<"header-hygiene">; def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">; def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">; Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -9740,6 +9740,12 @@ def warn_comparison_bitwise_or : Warning< "bitwise or with non-zero value always evaluates to true">, InGroup, DefaultIgnore; +def warn_tautological_negation_and_compare: Warning< + "'&&' against a variable and its negation always evaluates to false">, + InGroup, DefaultIgnore; +def warn_tautological_negation_or_compare: Warning< + "'||' against a variable and its negation always evaluates to true">, + InGroup, DefaultIgnore; def warn_tautological_overlap_comparison : Warning< "overlapping comparisons always evaluate to %select{false|true}0">, InGroup, DefaultIgnore; Index: clang/lib/Analysis/CFG.cpp =================================================================== --- clang/lib/Analysis/CFG.cpp +++ clang/lib/Analysis/CFG.cpp @@ -1077,16 +1077,41 @@ } } - /// Find a pair of comparison expressions with or without parentheses + /// There are two checks handled by this function: + /// 1. Find a law-of-excluded-middle or law-of-noncontradiction expression + /// e.g. if (x || !x), if (x && !x) + /// 2. Find a pair of comparison expressions with or without parentheses /// with a shared variable and constants and a logical operator between them /// that always evaluates to either true or false. /// e.g. if (x != 3 || x != 4) TryResult checkIncorrectLogicOperator(const BinaryOperator *B) { assert(B->isLogicalOp()); - const BinaryOperator *LHS = - dyn_cast(B->getLHS()->IgnoreParens()); - const BinaryOperator *RHS = - dyn_cast(B->getRHS()->IgnoreParens()); + const Expr *LHSExpr = B->getLHS()->IgnoreParens(); + const Expr *RHSExpr = B->getRHS()->IgnoreParens(); + + auto CheckLogicalOpWithNegatedVariable = [this, B](const Expr *E1, + const Expr *E2) { + if (const auto *Negate = dyn_cast(E1)) { + if (Negate->getOpcode() == UO_LNot && + Expr::isSameComparisonOperand(Negate->getSubExpr(), E2)) { + bool AlwaysTrue = B->getOpcode() == BO_LOr; + if (BuildOpts.Observer) + BuildOpts.Observer->logicAlwaysTrue(B, AlwaysTrue); + return TryResult(AlwaysTrue); + } + } + return TryResult(); + }; + + TryResult Result = CheckLogicalOpWithNegatedVariable(LHSExpr, RHSExpr); + if (Result.isKnown()) + return Result; + Result = CheckLogicalOpWithNegatedVariable(RHSExpr, LHSExpr); + if (Result.isKnown()) + return Result; + + const auto *LHS = dyn_cast(LHSExpr); + const auto *RHS = dyn_cast(RHSExpr); if (!LHS || !RHS) return {}; Index: clang/lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- clang/lib/Sema/AnalysisBasedWarnings.cpp +++ clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -158,6 +158,17 @@ return false; } + void logicAlwaysTrue(const BinaryOperator *B, bool isAlwaysTrue) override { + if (HasMacroID(B)) + return; + + unsigned DiagID = isAlwaysTrue + ? diag::warn_tautological_negation_or_compare + : diag::warn_tautological_negation_and_compare; + SourceRange DiagRange = B->getSourceRange(); + S.Diag(B->getExprLoc(), DiagID) << DiagRange; + } + void compareAlwaysTrue(const BinaryOperator *B, bool isAlwaysTrue) override { if (HasMacroID(B)) return; @@ -188,7 +199,8 @@ static bool hasActiveDiagnostics(DiagnosticsEngine &Diags, SourceLocation Loc) { return !Diags.isIgnored(diag::warn_tautological_overlap_comparison, Loc) || - !Diags.isIgnored(diag::warn_comparison_bitwise_or, Loc); + !Diags.isIgnored(diag::warn_comparison_bitwise_or, Loc) || + !Diags.isIgnored(diag::warn_tautological_negation_and_compare, Loc); } }; } // anonymous namespace Index: clang/test/Analysis/temp-obj-dtors-cfg-output.cpp =================================================================== --- clang/test/Analysis/temp-obj-dtors-cfg-output.cpp +++ clang/test/Analysis/temp-obj-dtors-cfg-output.cpp @@ -1393,13 +1393,13 @@ // CHECK: Succs (2): B2 B1 // CHECK: [B4 (NORETURN)] // CHECK: 1: ~NoReturn() (Temporary object destructor) -// CHECK: Preds (1): B5 +// CHECK: Preds (1): B5(Unreachable) // CHECK: Succs (1): B0 // CHECK: [B5] // CHECK: 1: [B8.3] || [B7.2] || [B6.7] // CHECK: T: (Temp Dtor) [B6.4] // CHECK: Preds (3): B6 B7 B8 -// CHECK: Succs (2): B4 B3 +// CHECK: Succs (2): B4(Unreachable) B3 // CHECK: [B6] // CHECK: 1: check // CHECK: 2: [B6.1] (ImplicitCastExpr, FunctionToPointerDecay, _Bool (*)(const NoReturn &)) Index: clang/test/Misc/warning-wall.c =================================================================== --- clang/test/Misc/warning-wall.c +++ clang/test/Misc/warning-wall.c @@ -55,6 +55,7 @@ CHECK-NEXT: -Wtautological-bitwise-compare CHECK-NEXT: -Wtautological-undefined-compare CHECK-NEXT: -Wtautological-objc-bool-compare +CHECK-NEXT: -Wtautological-negation-compare CHECK-NEXT: -Wtrigraphs CHECK-NEXT: -Wuninitialized CHECK-NEXT: -Wsometimes-uninitialized Index: clang/test/SemaCXX/tautological-negation-compare.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/tautological-negation-compare.cpp @@ -0,0 +1,41 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-negation-compare -Wno-constant-logical-operand %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-compare -Wno-constant-logical-operand %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused -Wno-loop-analysis -Wno-constant-logical-operand %s + +#define COPY(x) x + +void test_int(int x) { + if (x || !x) {} // expected-warning {{'||' against a variable and its negation always evaluates to true}} + if (!x || x) {} // expected-warning {{'||' against a variable and its negation always evaluates to true}} + if (x && !x) {} // expected-warning {{'&&' against a variable and its negation always evaluates to false}} + if (!x && x) {} // expected-warning {{'&&' against a variable and its negation always evaluates to false}} + + // parentheses are ignored + if (x || (!x)) {} // expected-warning {{'||' against a variable and its negation always evaluates to true}} + if (!(x) || x) {} // expected-warning {{'||' against a variable and its negation always evaluates to true}} + + // don't warn on macros + if (COPY(x) || !x) {} + if (!x || COPY(x)) {} + if (x && COPY(!x)) {} + if (COPY(!x && x)) {} + + // dont' warn on literals + if (1 || !1) {} + if (!42 && 42) {} + + + // don't warn on overloads + struct Foo{ + int val; + Foo operator!() const { return Foo{!val}; } + bool operator||(const Foo other) const { return val || other.val; } + bool operator&&(const Foo other) const { return val && other.val; } + }; + + Foo f{3}; + if (f || !f) {} + if (!f || f) {} + if (f.val || !f.val) {} // expected-warning {{'||' against a variable and its negation always evaluates to true}} + if (!f.val && f.val) {} // expected-warning {{'&&' against a variable and its negation always evaluates to false}} +} Index: clang/test/SemaCXX/warn-infinite-recursion.cpp =================================================================== --- clang/test/SemaCXX/warn-infinite-recursion.cpp +++ clang/test/SemaCXX/warn-infinite-recursion.cpp @@ -203,3 +203,13 @@ (void)typeid(unevaluated_recursive_function()); return 0; } + +void func1(int i) { // expected-warning {{call itself}} + if (i || !i) + func1(i); +} +void func2(int i) { // expected-warning {{call itself}} + if (!i && i) {} + else + func2(i); +}