Author: Zinovy Nis Date: 2026-09-22T09:46:56+03:00 New Revision: d2d28852845aaf0588a4c4a9db341cb217440247
URL: https://github.com/llvm/llvm-project/commit/d2d28852845aaf0588a4c4a9db341cb217440247 DIFF: https://github.com/llvm/llvm-project/commit/d2d28852845aaf0588a4c4a9db341cb217440247.diff LOG: [clang-tidy] Fix De Morgan for overloaded comparisons (#221432) Before my patch: (https://source.chromium.org/chromium/chromium/src/+/main:base/trace_event/memory_allocator_dump.cc;l=174) ```diff - if (!(name == rhs.name && units == rhs.units && - entry_type == rhs.entry_type)) { + if (!name == rhs.name || !units == rhs.units || + entry_type != rhs.entry_type) { ``` With the patch: ```diff - if (!(name == rhs.name && units == rhs.units && - entry_type == rhs.entry_type)) { + if (name != rhs.name || units != rhs.units || + entry_type != rhs.entry_type) { ``` Tests were written with the assistance of ChatGPT 5.6 Terra Added: Modified: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp clang-tools-extra/docs/ReleaseNotes.md clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-demorgan.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp index 3a63b6f84eed3..121c327dc3b58 100644 --- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp @@ -250,6 +250,21 @@ static bool containsDiscardedTokens(const ASTContext &Context, return false; } +static std::optional<bool> +tryFixCXXOperator(const Expr *E, SmallVectorImpl<FixItHint> &Fixes) { + if (const auto *OpCall = dyn_cast<CXXOperatorCallExpr>(E)) { + const StringRef NegatedOperator = negatedOperator(OpCall); + if (!NegatedOperator.empty()) { + if (OpCall->getOperatorLoc().isMacroID()) + return true; + Fixes.push_back(FixItHint::CreateReplacement(OpCall->getOperatorLoc(), + NegatedOperator)); + return false; + } + } + return std::nullopt; +} + class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> { using Base = RecursiveASTVisitor<Visitor>; @@ -916,9 +931,17 @@ static bool flipDemorganSide(SmallVectorImpl<FixItHint> &Fixes, } if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) return flipDemorganBinaryOperator(Fixes, Ctx, BinOp, OuterBO); + // Overloaded comparisons are represented as CXXOperatorCallExpr rather than + // BinaryOperator, so negate them by replacing their operator location. + if (auto Fixed = tryFixCXXOperator(E, Fixes)) + return *Fixed; + if (const auto *Paren = dyn_cast<ParenExpr>(E)) { if (const auto *BinOp = dyn_cast<BinaryOperator>(Paren->getSubExpr())) return flipDemorganBinaryOperator(Fixes, Ctx, BinOp, OuterBO, Paren); + // Overloaded comparisons in parentheses, e.g. (T1 < T2). + if (auto Fixed = tryFixCXXOperator(Paren->getSubExpr(), Fixes)) + return *Fixed; } // Fallback case just insert a logical not operator. if (E->getBeginLoc().isMacroID()) diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index d0906f9ee6be8..0441aef375c54 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -279,7 +279,7 @@ infrastructure are described first, followed by tool-specific sections. typedef or type alias that provides the only name of an otherwise unnamed tag, such as `typedef enum {} MyEnum;`, against the style configured for that tag kind instead of the typedef or type alias style. - + - Added support for naming lambda init-captures (e.g. `[Captured = Var]`) via the new `LambdaCapture` options. Simple, non-init captures continue to follow the naming style of the variable they capture. @@ -304,6 +304,10 @@ infrastructure are described first, followed by tool-specific sections. <clang-tidy/checks/readability/redundant-parentheses>` check by fixing a false positive on the required parentheses of `typeof` and `typeof_unqual` operands. +- Fixed {doc}`readability-simplify-boolean-expr + <clang-tidy/checks/readability/simplify-boolean-expr>` producing invalid + fixes when applying De Morgan's theorem to overloaded comparison operators. + - Improved {doc}`readability-trailing-comma <clang-tidy/checks/readability/trailing-comma>` check: diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-demorgan.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-demorgan.cpp index bab9e17a7775b..e194bb4eb1745 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-demorgan.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-demorgan.cpp @@ -105,4 +105,61 @@ void foo(bool A1, bool A2, bool A3, bool A4) { // CHECK-FIXES: X = A1 && A2 && A3; // CHECK-FIXES-NEXT: X = A1 || (A2 && A3); // CHECK-FIXES-NEXT: X = A1 && (A2 || A3); + + struct T { + bool operator==(const T&) const; + bool operator<(const T&) const; + }; + + T T1, T2; + X = !(T1 == T2 && A1 == A2); + X = !(T1 < T2 || (A1 || !A2)); + X = !((T1 < T2) || (A1 || !A2)); + // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem + // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem + // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem + // CHECK-FIXES: X = T1 != T2 || A1 != A2; + // CHECK-FIXES-NEXT: X = T1 >= T2 && !A1 && A2; + // CHECK-FIXES-NEXT: X = (T1 >= T2) && !A1 && A2; } + +// Equality on a user-defined type is an overloaded operator, so its negation +// must replace `==` with `!=`, rather than insert `!` before the left operand. +namespace overloaded_comparisons { +struct Entry { + struct String { + bool operator==(const String &) const; + bool operator!=(const String &) const; + bool operator<(const String &) const; + bool operator>(const String &) const; + bool operator<=(const String &) const; + bool operator>=(const String &) const; + }; + + String name; + String units; + int entry_type; + + bool matches(const Entry &rhs) const { + return !(name == rhs.name && units == rhs.units && + entry_type == rhs.entry_type); + } + // CHECK-MESSAGES: :[[@LINE-3]]:12: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr] + // CHECK-FIXES: return name != rhs.name || units != rhs.units || + // CHECK-FIXES-NEXT: entry_type != rhs.entry_type; + + bool diff ersFrom(const Entry &rhs) const { + return !(name != rhs.name || entry_type != rhs.entry_type); + } + // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr] + // CHECK-FIXES: return name == rhs.name && entry_type == rhs.entry_type; + + bool comparesTo(const Entry &rhs) const { + return !(name < rhs.name && units > rhs.units && name <= rhs.name && + units >= rhs.units && entry_type < rhs.entry_type); + } + // CHECK-MESSAGES: :[[@LINE-3]]:12: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr] + // CHECK-FIXES: return name >= rhs.name || units <= rhs.units || name > rhs.name || + // CHECK-FIXES-NEXT: units < rhs.units || entry_type >= rhs.entry_type; +}; +} // namespace overloaded_comparisons _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
