[clang-tools-extra] [clang-tidy] Fix false positive in readability-redundant-parentheses … (PR #192827)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Yuta Nakamura (nakasan617)

<details>
<summary>Changes</summary>

…for overloaded operators

Overloaded comparison operators (e.g. iterator !=, std::string >=) are represented as CXXOperatorCallExpr, a subclass of CallExpr. The matcher previously matched all CallExpr, causing spurious warnings for parenthesized overloaded-operator expressions while silently accepting equivalent built-in operator expressions (e.g. int == int). Exclude CXXOperatorCallExpr from the CallExpr match to make the behavior consistent.

@<!-- -->PiotrZSL @<!-- -->vbvictor Could you take a look at this small fix?                                                                               
I'm a PhD researcher who wanted to delve further into the LLVM codebase out of pure curiosity — this is my first diff, kept intentionally small and non-harmful. Any feedback is very welcome! 

Fixes: #<!-- -->192463, #<!-- -->192438, #<!-- -->192435

---
Full diff: https://github.com/llvm/llvm-project/pull/192827.diff


2 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp (+2-1) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses.cpp (+20) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
index 9b3948a1c50c0..a1500ad80869e 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
@@ -55,7 +55,8 @@ void RedundantParenthesesCheck::registerMatchers(MatchFinder *Finder) {
                     parenExpr(), ConstantExpr,
                     declRefExpr(to(namedDecl(unless(
                         matchers::matchesAnyListedRegexName(AllowedDecls))))),
-                    memberExpr(), callExpr())),
+                    memberExpr(),
+                    callExpr(unless(cxxOperatorCallExpr())))),
                 unless(anyOf(isInMacro(),
                              // sizeof(...) is common used.
                              hasParent(unaryExprOrTypeTraitExpr()))))
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses.cpp
index 9a8a0d4d73483..d080f3f5a3f56 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses.cpp
@@ -121,3 +121,23 @@ void memberExpr() {
    // CHECK-FIXES:    if (foo.fooBar().z) {
   }
 }
+
+struct Iterator {
+  bool operator!=(const Iterator &) const;
+  bool operator==(const Iterator &) const;
+  bool operator>=(const Iterator &) const;
+};
+
+// Overloaded operators look like binary expressions to readers; no warning.
+bool overloadedOperatorNoWarn(Iterator it, Iterator end) {
+  if ((it != end))
+    return true;
+  if ((it == end))
+    return false;
+  return (it >= end);
+}
+
+// Built-in comparison operators also produce no warning.
+bool builtinComparisonNoWarn(int i) {
+  return (i == 0);
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/192827