LegalizeAdulthood updated this revision to Diff 426163.
LegalizeAdulthood added a comment.
Surround replacements with parens
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124650/new/
https://reviews.llvm.org/D124650
Files:
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t -- -config="{CheckOptions: [{key: "readability-simplify-boolean-expr.SimplifyDeMorgan", value: true}]}" --
+
+bool a1 = false;
+bool a2 = false;
+
+bool aa = !(!a1 || a2);
+bool ab = !(a1 || !a2);
+bool ac = !(!a1 || !a2);
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^bool aa = (a1 && !a2);$}}
+// CHECK-FIXES-NEXT: {{^bool ab = (!a1 && a2);$}}
+// CHECK-FIXES-NEXT: {{^bool ac = (a1 && a2);$}}
+
+bool ba = !(!a1 && a2);
+bool bb = !(a1 && !a2);
+bool bc = !(!a1 && !a2);
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^bool ba = (a1 }}||{{ !a2);$}}
+// CHECK-FIXES-NEXT: {{^bool bb = (!a1 }}||{{ a2);$}}
+// CHECK-FIXES-NEXT: {{^bool bc = (a1 }}||{{ a2);$}}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
@@ -4,7 +4,8 @@
=================================
Looks for boolean expressions involving boolean constants and simplifies
-them to use the appropriate boolean expression directly.
+them to use the appropriate boolean expression directly. Simplifies
+boolean expressions by application of DeMorgan's Theorem.
Examples:
@@ -27,6 +28,12 @@
``if (e) b = false; else b = true;`` ``b = !e;``
``if (e) return true; return false;`` ``return e;``
``if (e) return false; return true;`` ``return !e;``
+``!(!a || b)`` ``(a && !b)``
+``!(a || !b)`` ``(!a && b)``
+``!(!a || !b)`` ``(a && b)``
+``!(!a && b)`` ``(a || !b)``
+``!(a && !b)`` ``(!a || b)``
+``!(!a && !b)`` ``(a || b)``
=========================================== ================
The resulting expression ``e`` is modified as follows:
@@ -84,3 +91,8 @@
If `true`, conditional boolean assignments at the end of an ``if/else
if`` chain will be transformed. Default is `false`.
+
+.. option:: SimplifyDeMorgan
+
+ If `true`, DeMorgan's Theorem will be applied to simplify negated
+ conjunctions and disjunctions. Default is `true`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -136,6 +136,12 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
+ <clang-tidy/checks/altera-struct-pack-align>` check for empty structs.
+
+- Fixed some false positives in :doc:`bugprone-infinite-loop
+ <clang-tidy/checks/bugprone-infinite-loop>` involving dependent expressions.
+
- Fixed a crash in :doc:`bugprone-sizeof-expression
<clang-tidy/checks/bugprone-sizeof-expression>` when `sizeof(...)` is
compared against a `__int128_t`.
@@ -158,26 +164,24 @@
<clang-tidy/checks/misc-redundant-expression>` involving assignments in
conditions. This fixes `Issue 35853 <https://github.com/llvm/llvm-project/issues/35853>`_.
-- Fixed a crash in :doc:`readability-const-return-type
- <clang-tidy/checks/readability-const-return-type>` when a pure virtual function
- overrided has a const return type. Removed the fix for a virtual function.
-
-- Fixed a false positive in :doc:`readability-non-const-parameter
- <clang-tidy/checks/readability-non-const-parameter>` when the parameter is
- referenced by an lvalue.
-
- Improved :doc:`performance-inefficient-vector-operation
<clang-tidy/checks/performance-inefficient-vector-operation>` to work when
the vector is a member of a structure.
-- Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
- <clang-tidy/checks/altera-struct-pack-align>` check for empty structs.
+- Fixed a crash in :doc:`readability-const-return-type
+ <clang-tidy/checks/readability-const-return-type>` when a pure virtual function
+ overrided has a const return type. Removed the fix for a virtual function.
- Fixed incorrect suggestions for :doc:`readability-container-size-empty
<clang-tidy/checks/readability-container-size-empty>` when smart pointers are involved.
-- Fixed some false positives in :doc:`bugprone-infinite-loop
- <clang-tidy/checks/bugprone-infinite-loop>` involving dependent expressions.
+- Fixed a false positive in :doc:`readability-non-const-parameter
+ <clang-tidy/checks/readability-non-const-parameter>` when the parameter is
+ referenced by an lvalue.
+
+- Expanded :doc:`readability-simplify-boolean-expr
+ <clang-tidy/checks/readability-simplify-boolean-expr>` to simplify expressions
+ using DeMorgan's Theorem.
Removed checks
^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -61,6 +61,8 @@
void matchLabelIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
StringRef Id);
+ void matchDeMorganExpr(ast_matchers::MatchFinder *Finder);
+
void
replaceWithThenStatement(const ast_matchers::MatchFinder::MatchResult &Result,
const Expr *BoolLiteral);
@@ -77,10 +79,6 @@
const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If,
bool Negated);
- void
- replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
- const IfStmt *If, bool Negated);
-
void replaceCompoundReturnWithCondition(
const ast_matchers::MatchFinder::MatchResult &Result,
const CompoundStmt *Compound, bool Negated);
@@ -98,12 +96,22 @@
void replaceLabelCompoundReturnWithCondition(
const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
+ void
+ replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
+ const IfStmt *If, bool Negated);
+
+ bool
+ simplifyDeMorganExpr(const ast_matchers::MatchFinder::MatchResult &Result,
+ const Expr *OrLeft, bool NotLeft, StringRef Op,
+ bool NotRight);
+
void issueDiag(const ast_matchers::MatchFinder::MatchResult &Result,
SourceLocation Loc, StringRef Description,
SourceRange ReplacementRange, StringRef Replacement);
const bool ChainedConditionalReturn;
const bool ChainedConditionalAssignment;
+ const bool SimplifyDeMorgan;
};
} // namespace readability
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -61,6 +61,14 @@
static constexpr char LabelCompoundBoolId[] = "label-compound-bool";
static constexpr char LabelCompoundNotBoolId[] = "label-compound-bool-not";
static constexpr char IfStmtId[] = "if";
+static constexpr char LeftId[] = "left";
+static constexpr char RightId[] = "right";
+static constexpr char DeMorganOrLeftId[] = "demorgan-or-left";
+static constexpr char DeMorganOrRightId[] = "demorgan-or-right";
+static constexpr char DeMorganOrBothId[] = "demorgan-or-both";
+static constexpr char DeMorganAndLeftId[] = "demorgan-and-left";
+static constexpr char DeMorganAndRightId[] = "demorgan-and-right";
+static constexpr char DeMorganAndBothId[] = "demorgan-and-both";
static constexpr char SimplifyOperatorDiagnostic[] =
"redundant boolean literal supplied to boolean operator";
@@ -371,7 +379,8 @@
: ClangTidyCheck(Name, Context),
ChainedConditionalReturn(Options.get("ChainedConditionalReturn", false)),
ChainedConditionalAssignment(
- Options.get("ChainedConditionalAssignment", false)) {}
+ Options.get("ChainedConditionalAssignment", false)),
+ SimplifyDeMorgan(Options.get("SimplifyDeMorgan", true)) {}
static bool containsBoolLiteral(const Expr *E) {
if (!E)
@@ -570,10 +579,60 @@
Finder->addMatcher(CompoundStmt, this);
}
+void SimplifyBooleanExprCheck::matchDeMorganExpr(MatchFinder *Finder) {
+ if (!SimplifyDeMorgan)
+ return;
+
+ auto Not = hasOperatorName("!");
+ auto Or = hasOperatorName("||");
+ auto And = hasOperatorName("&&");
+ auto LHSExpr = expr().bind(LeftId);
+ auto RHSExpr = expr().bind(RightId);
+ auto NotOp = unaryOperator(Not);
+ auto LHS = hasLHS(LHSExpr);
+ auto RHS = hasRHS(RHSExpr);
+ auto NotLHS = hasLHS(unaryOperator(Not, hasUnaryOperand(LHSExpr)));
+ auto NotRHS = hasRHS(unaryOperator(Not, hasUnaryOperand(RHSExpr)));
+ auto UnlessNotRHS = unless(hasRHS(NotOp));
+ auto UnlessNotLHS = unless(hasLHS(NotOp));
+ // match !(!a || b)
+ Finder->addMatcher(unaryOperator(Not, hasUnaryOperand(binaryOperator(
+ Or, UnlessNotRHS, NotLHS, RHS)))
+ .bind(DeMorganOrLeftId),
+ this);
+ // match !(a || !b)
+ Finder->addMatcher(unaryOperator(Not, hasUnaryOperand(binaryOperator(
+ Or, UnlessNotLHS, LHS, NotRHS)))
+ .bind(DeMorganOrRightId),
+ this);
+ // match !(!a || !b)
+ Finder->addMatcher(
+ unaryOperator(Not, hasUnaryOperand(binaryOperator(Or, NotLHS, NotRHS)))
+ .bind(DeMorganOrBothId),
+ this);
+
+ // match !(!a && b)
+ Finder->addMatcher(unaryOperator(Not, hasUnaryOperand(binaryOperator(
+ And, UnlessNotRHS, NotLHS, RHS)))
+ .bind(DeMorganAndLeftId),
+ this);
+ // match !(a && !b)
+ Finder->addMatcher(unaryOperator(Not, hasUnaryOperand(binaryOperator(
+ And, UnlessNotLHS, LHS, NotRHS)))
+ .bind(DeMorganAndRightId),
+ this);
+ // match !(!a && !b)
+ Finder->addMatcher(
+ unaryOperator(Not, hasUnaryOperand(binaryOperator(And, NotLHS, NotRHS)))
+ .bind(DeMorganAndBothId),
+ this);
+}
+
void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn);
Options.store(Opts, "ChainedConditionalAssignment",
ChainedConditionalAssignment);
+ Options.store(Opts, "SimplifyDeMorgan", SimplifyDeMorgan);
}
void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
@@ -602,6 +661,8 @@
matchLabelIfReturnsBool(Finder, true, LabelCompoundBoolId);
matchLabelIfReturnsBool(Finder, false, LabelCompoundNotBoolId);
+
+ matchDeMorganExpr(Finder);
}
void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
@@ -650,6 +711,22 @@
replaceLabelCompoundReturnWithCondition(Result, true);
else if (const auto TU = Result.Nodes.getNodeAs<Decl>("top"))
Visitor(this, Result).TraverseDecl(const_cast<Decl *>(TU));
+ else if (const auto *OrLeft = Result.Nodes.getNodeAs<Expr>(DeMorganOrLeftId))
+ simplifyDeMorganExpr(Result, OrLeft, false, "&&", true);
+ else if (const auto *OrRight =
+ Result.Nodes.getNodeAs<Expr>(DeMorganOrRightId))
+ simplifyDeMorganExpr(Result, OrRight, true, "&&", false);
+ else if (const auto *OrBoth = Result.Nodes.getNodeAs<Expr>(DeMorganOrBothId))
+ simplifyDeMorganExpr(Result, OrBoth, false, "&&", false);
+ else if (const auto *AndLeft =
+ Result.Nodes.getNodeAs<Expr>(DeMorganAndLeftId))
+ simplifyDeMorganExpr(Result, AndLeft, false, "||", true);
+ else if (const auto *AndRight =
+ Result.Nodes.getNodeAs<Expr>(DeMorganAndRightId))
+ simplifyDeMorganExpr(Result, AndRight, true, "||", false);
+ else if (const auto *AndBoth =
+ Result.Nodes.getNodeAs<Expr>(DeMorganAndBothId))
+ simplifyDeMorganExpr(Result, AndBoth, false, "||", false);
}
void SimplifyBooleanExprCheck::issueDiag(const MatchFinder::MatchResult &Result,
@@ -787,6 +864,26 @@
Replacement);
}
+bool SimplifyBooleanExprCheck::simplifyDeMorganExpr(
+ const MatchFinder::MatchResult &Result, const Expr *OrLeft, bool NotLeft,
+ StringRef Op, bool NotRight) {
+ if (!SimplifyDeMorgan)
+ return true;
+
+ const Expr *Left = Result.Nodes.getNodeAs<Expr>(LeftId);
+ const Expr *Right = Result.Nodes.getNodeAs<Expr>(RightId);
+ std::string Replacement =
+ (StringRef{"("} + (NotLeft ? "!" : "") + getText(Result, *Left) + " " +
+ Op + " " + (NotRight ? "!" : "") + getText(Result, *Right) + ")")
+ .str();
+ SourceLocation Loc = OrLeft->getBeginLoc();
+ SourceRange Range = OrLeft->getSourceRange();
+ issueDiag(Result, Loc,
+ "boolean expression can be simplified by DeMorgan's theorem", Range,
+ Replacement);
+ return false;
+}
+
} // namespace readability
} // namespace tidy
} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
