https://github.com/irishrover updated https://github.com/llvm/llvm-project/pull/221432
>From b453e7ef721df8062735b731a8347acc6db605d6 Mon Sep 17 00:00:00 2001 From: Zinovy Nis <[email protected]> Date: Sat, 5 Sep 2026 13:59:09 +0300 Subject: [PATCH] [clang-tidy] Fix DeMorgan for overloaded comparisons 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) { ``` --- .../readability/SimplifyBooleanExprCheck.cpp | 12 ++++++ clang-tools-extra/docs/ReleaseNotes.md | 4 ++ .../simplify-boolean-expr-demorgan.cpp | 41 +++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp index 3a63b6f84eed3..50ec10ca0cc24 100644 --- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp @@ -916,6 +916,18 @@ 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 (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; + } + } if (const auto *Paren = dyn_cast<ParenExpr>(E)) { if (const auto *BinOp = dyn_cast<BinaryOperator>(Paren->getSubExpr())) return flipDemorganBinaryOperator(Fixes, Ctx, BinOp, OuterBO, Paren); diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index b32b8a51e0606..8d5522277854a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -240,6 +240,10 @@ infrastructure are described first, followed by tool-specific sections. exclusively for overload resolution. Added the {option}`IgnoredTypes` option to allow customizing the set of ignored types. +- Fixed {doc}`readability-simplify-boolean-expr + <clang-tidy/checks/readability/simplify-boolean-expr>` producing invalid + fixes when applying DeMorgan'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..4f3660b4a1c46 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 @@ -106,3 +106,44 @@ void foo(bool A1, bool A2, bool A3, bool A4) { // CHECK-FIXES-NEXT: X = A1 || (A2 && A3); // CHECK-FIXES-NEXT: X = A1 && (A2 || A3); } + +// 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 differsFrom(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
