[clang-tools-extra] [clang-tidy] Simplify conjunction with comparison in readability-simplify-boolean-expr (PR #233080)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 17 06:08:23 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: PrahladGowtham
<details>
<summary>Changes</summary>
Extend `readability-simplify-boolean-expr` to simplify boolean comparisons inside logical conjunctions (`&&`) where one operand determines the truth value of the variable in the other operand.
### Examples
- `a && a != b` -> `a && !b`
- `a && a == b` -> `a && b`
- `!a && a != b` -> `!a && b`
- `!a && a == b` -> `!a && !b`
- `a == b && !a` -> `!b && !a`
Includes test coverage, documentation updates, and release notes.
Closes #<!-- -->232311
---
Full diff: https://github.com/llvm/llvm-project/pull/233080.diff
5 Files Affected:
- (modified) clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (+78-9)
- (modified) clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h (+2)
- (modified) clang-tools-extra/docs/ReleaseNotes.md (+5)
- (modified) clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.md (+5)
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp (+79)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index 121c327dc3b583..a87abc39e46878 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "SimplifyBooleanExprCheck.h"
+#include "../utils/ASTUtils.h"
#include "clang/AST/Expr.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/DiagnosticIDs.h"
@@ -38,6 +39,8 @@ static constexpr char SimplifyConditionDiagnostic[] =
"redundant boolean literal in if statement condition";
static constexpr char SimplifyConditionalReturnDiagnostic[] =
"redundant boolean literal in conditional return statement";
+static constexpr char SimplifyConjunctionDiagnostic[] =
+ "redundant comparison in boolean expression";
static bool needsParensAfterUnaryNegation(const Expr *E) {
E = E->IgnoreImpCasts();
@@ -305,6 +308,7 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
bool VisitBinaryOperator(const BinaryOperator *Op) const {
Check->reportBinOp(Context, Op);
+ Check->reportConjunctionWithComparison(Context, Op);
return true;
}
@@ -649,15 +653,14 @@ void SimplifyBooleanExprCheck::reportBinOp(const ASTContext &Context,
const bool BoolValue = Bool->getValue();
- const auto ReplaceWithExpression = [this, &Context, LHS, RHS,
- Bool](const Expr *ReplaceWith,
- bool Negated) {
- const std::string Replacement =
- replacementExpression(Context, Negated, ReplaceWith);
- const SourceRange Range(LHS->getBeginLoc(), RHS->getEndLoc());
- issueDiag(Context, Bool->getBeginLoc(), SimplifyOperatorDiagnostic, Range,
- Replacement);
- };
+ const auto ReplaceWithExpression =
+ [this, &Context, LHS, RHS, Bool](const Expr *ReplaceWith, bool Negated) {
+ const std::string Replacement =
+ replacementExpression(Context, Negated, ReplaceWith);
+ const SourceRange Range(LHS->getBeginLoc(), RHS->getEndLoc());
+ issueDiag(Context, Bool->getBeginLoc(), SimplifyOperatorDiagnostic,
+ Range, Replacement);
+ };
switch (Op->getOpcode()) {
case BO_LAnd:
@@ -689,6 +692,72 @@ void SimplifyBooleanExprCheck::reportBinOp(const ASTContext &Context,
}
}
+static std::pair<const Expr *, bool> parseCondition(const Expr *E) {
+ E = E->IgnoreParenImpCasts();
+ if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {
+ if (UnOp->getOpcode() == UO_LNot)
+ return {UnOp->getSubExpr()->IgnoreParenImpCasts(), true};
+ }
+ return {E, false};
+}
+
+void SimplifyBooleanExprCheck::reportConjunctionWithComparison(
+ const ASTContext &Context, const BinaryOperator *Op) {
+ if (canBeBypassed(Op))
+ return;
+
+ if (Op->getOpcode() != BO_LAnd || !Op->getType()->isBooleanType())
+ return;
+
+ const auto CheckOperands = [this, &Context](const Expr *CondExpr,
+ const Expr *CompRaw) -> bool {
+ const auto [ExprA, CondNegated] = parseCondition(CondExpr);
+ if (!ExprA->getType()->isBooleanType() || ExprA->HasSideEffects(Context))
+ return false;
+
+ const Expr *CompUnwrapped = CompRaw->IgnoreParenImpCasts();
+ const auto *BinOp = dyn_cast<BinaryOperator>(CompUnwrapped);
+ if (!BinOp || (BinOp->getOpcode() != BO_EQ && BinOp->getOpcode() != BO_NE))
+ return false;
+
+ const Expr *CompLHS = BinOp->getLHS()->IgnoreParenImpCasts();
+ const Expr *CompRHS = BinOp->getRHS()->IgnoreParenImpCasts();
+
+ if (!CompLHS->getType()->isBooleanType() ||
+ !CompRHS->getType()->isBooleanType())
+ return false;
+
+ if (CompLHS->HasSideEffects(Context) || CompRHS->HasSideEffects(Context))
+ return false;
+
+ if (isa<CXXBoolLiteralExpr>(ExprA) || isa<CXXBoolLiteralExpr>(CompLHS) ||
+ isa<CXXBoolLiteralExpr>(CompRHS))
+ return false;
+
+ if (utils::areStatementsIdentical(CompLHS, CompRHS, Context, true))
+ return false;
+
+ const Expr *ExprB = nullptr;
+ if (utils::areStatementsIdentical(CompLHS, ExprA, Context, true))
+ ExprB = CompRHS;
+ else if (utils::areStatementsIdentical(CompRHS, ExprA, Context, true))
+ ExprB = CompLHS;
+ else
+ return false;
+
+ const bool CompNeedsNegation = (BinOp->getOpcode() == BO_NE) ^ CondNegated;
+ const std::string Replacement =
+ replacementExpression(Context, CompNeedsNegation, ExprB);
+
+ return issueDiag(Context, BinOp->getOperatorLoc(),
+ SimplifyConjunctionDiagnostic, CompRaw->getSourceRange(),
+ Replacement);
+ };
+
+ if (!CheckOperands(Op->getLHS(), Op->getRHS()))
+ CheckOperands(Op->getRHS(), Op->getLHS());
+}
+
void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IgnoreMacros", IgnoreMacros);
Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn);
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
index d43116b102ce32..83031d1eb313de 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -36,6 +36,8 @@ class SimplifyBooleanExprCheck : public ClangTidyCheck {
class Visitor;
void reportBinOp(const ASTContext &Context, const BinaryOperator *Op);
+ void reportConjunctionWithComparison(const ASTContext &Context,
+ const BinaryOperator *Op);
void replaceWithThenStatement(const ASTContext &Context,
const IfStmt *IfStatement,
diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md
index 69f3ecee49926b..9b4c8da353df22 100644
--- a/clang-tools-extra/docs/ReleaseNotes.md
+++ b/clang-tools-extra/docs/ReleaseNotes.md
@@ -398,6 +398,11 @@ infrastructure are described first, followed by tool-specific sections.
<clang-tidy/checks/readability/simplify-boolean-expr>` producing invalid
fixes when applying De Morgan's theorem to overloaded comparison operators.
+- Improved {doc}`readability-simplify-boolean-expr
+ <clang-tidy/checks/readability/simplify-boolean-expr>` by simplifying
+ redundant equality comparisons in logical conjunctions (such as `a && a != b`
+ to `a && !b`, and `a == b && !a` to `!b && !a`).
+
- Improved {doc}`readability-suspicious-call-argument
<clang-tidy/checks/readability/suspicious-call-argument>` check by fixing the
default `dist` and `dst` abbreviations of `distance` not being recognized.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.md b/clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.md
index 12344ce7d71382..83d02674b3fbc4 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.md
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.md
@@ -33,6 +33,11 @@ Examples:
| `!(!a && b)` | `a \|\| !b` |
| `!(a && !b)` | `!a \|\| b` |
| `!(!a && !b)` | `a \|\| b` |
+| `a && a != b` | `a && !b` |
+| `a == b && !a` | `!b && !a` |
+| `a && a == b` | `a && b` |
+| `!a && a != b` | `!a && b` |
+| `!a && a == b` | `!a && !b` |
The resulting expression `e` is modified as follows:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp
index 0b99cb89262cdf..978522eec2c0a2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr.cpp
@@ -1035,3 +1035,82 @@ void instantiate() {
ignoreInstantiations<true>();
ignoreInstantiations<false>();
}
+
+bool conj_comp1(bool a, bool b) {
+ return a && a != b;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return a && !b;{{$}}
+
+bool conj_comp2(bool a, bool b) {
+ return a == b && !a;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return !b && !a;{{$}}
+
+bool conj_comp3(bool a, bool b) {
+ return a && a == b;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return a && b;{{$}}
+
+bool conj_comp4(bool a, bool b) {
+ return !a && a != b;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:18: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return !a && b;{{$}}
+
+bool conj_comp5(bool a, bool b) {
+ return !a && a == b;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:18: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return !a && !b;{{$}}
+
+bool conj_comp6(bool a, bool b) {
+ return (a != b) && a;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return !b && a;{{$}}
+
+bool conj_comp7(bool a, bool b) {
+ return (a == b) && a;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return b && a;{{$}}
+
+bool conj_comp8(bool a, bool b) {
+ return (a != b) && !a;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return b && !a;{{$}}
+
+bool conj_comp9(bool a, bool b) {
+ return b != a && a;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return !b && a;{{$}}
+
+bool conj_comp10(bool a, bool b) {
+ return b == a && !a;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return !b && !a;{{$}}
+
+bool conj_comp11(bool a, bool b) {
+ return a && (a != b);
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:18: warning: redundant comparison in boolean expression [readability-simplify-boolean-expr]
+// CHECK-FIXES: return a && !b;{{$}}
+
+bool conj_comp_neg1(bool a, bool b) {
+ return a && b;
+}
+
+bool conj_comp_neg2(bool a, bool b, bool c) {
+ return a && b == c;
+}
+
+bool conj_comp_neg3(int a, int b) {
+ return a && a != b;
+}
+
``````````
</details>
https://github.com/llvm/llvm-project/pull/233080
More information about the cfe-commits
mailing list