hazohelet updated this revision to Diff 532557.
hazohelet marked an inline comment as done.
hazohelet added a comment.
Address review comment
- NFC stylistic change
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152093/new/
https://reviews.llvm.org/D152093
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Analysis/CFG.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/CFG.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/Analysis/temp-obj-dtors-cfg-output.cpp
clang/test/Misc/warning-wall.c
clang/test/SemaCXX/tautological-negation-compare.cpp
clang/test/SemaCXX/warn-infinite-recursion.cpp
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);
+}
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/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/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/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/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<BinaryOperator>(B->getLHS()->IgnoreParens());
- const BinaryOperator *RHS =
- dyn_cast<BinaryOperator>(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<UnaryOperator>(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<BinaryOperator>(LHSExpr);
+ const auto *RHS = dyn_cast<BinaryOperator>(RHSExpr);
if (!LHS || !RHS)
return {};
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<TautologicalBitwiseCompare>, DefaultIgnore;
+def warn_tautological_negation_and_compare: Warning<
+ "'&&' against a variable and its negation always evaluates to false">,
+ InGroup<TautologicalNegationCompare>, DefaultIgnore;
+def warn_tautological_negation_or_compare: Warning<
+ "'||' against a variable and its negation always evaluates to true">,
+ InGroup<TautologicalNegationCompare>, DefaultIgnore;
def warn_tautological_overlap_comparison : Warning<
"overlapping comparisons always evaluate to %select{false|true}0">,
InGroup<TautologicalOverlapCompare>, DefaultIgnore;
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/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/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: <https://github.com/llvm/llvm-project/issues/56035>`_).
Bug Fixes in This Version
-------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
