[clang-tidy][readability-redundant-parentheses] add option to prevent widely used work around #164827

HerrCai0907 · 2025-10-23T14:39:44Z

Part of #164125
Add a new option to ignore some decls.

… widely used work around Fixed: llvm#164125 Add a new option to ignore some decls. Because it is a new check in this version, no release note needs to be updated.

HerrCai0907 · 2025-10-23T14:39:54Z

Because it is a new check in this version, no release note needs to be updated.

llvmbot · 2025-10-23T14:40:30Z

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

Author: Congcong Cai (HerrCai0907)

Changes

Fixed: #164125
Add a new option to ignore some decls.

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

4 Files Affected:

(modified) clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp (+22)
(modified) clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.h (+5-2)
(modified) clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst (+13)
(modified) clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses.cpp (+9)

diff --git a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
index 0ab59fff39d88..2b5cf12603609 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "RedundantParenthesesCheck.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/Expr.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
@@ -32,6 +33,18 @@ AST_MATCHER(ParenExpr, isInMacro) {
 
 } // namespace
 
+RedundantParenthesesCheck::RedundantParenthesesCheck(StringRef Name,
+                                                     ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      AllowedDecls(utils::options::parseStringList(
+          Options.get("AllowedDecls", "std::max;std::min"))) {}
+
+void RedundantParenthesesCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "AllowedDecls",
+                utils::options::serializeStringList(AllowedDecls));
+}
+
 void RedundantParenthesesCheck::registerMatchers(MatchFinder *Finder) {
   const auto ConstantExpr =
       expr(anyOf(integerLiteral(), floatLiteral(), characterLiteral(),
@@ -47,6 +60,15 @@ void RedundantParenthesesCheck::registerMatchers(MatchFinder *Finder) {
 
 void RedundantParenthesesCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *PE = Result.Nodes.getNodeAs<ParenExpr>("dup");
+  if (auto *DRE = dyn_cast<DeclRefExpr>(PE->getSubExpr())) {
+    const std::string Name = DRE->getDecl()->getQualifiedNameAsString();
+    llvm::errs() << Name << "\n";
+    bool Allowed = llvm::any_of(AllowedDecls, [&Name](const llvm::Regex &NM) {
+      return NM.isValid() && NM.match(Name);
+    });
+    if (Allowed)
+      return;
+  }
   diag(PE->getBeginLoc(), "redundant parentheses around expression")
       << FixItHint::CreateRemoval(PE->getLParen())
       << FixItHint::CreateRemoval(PE->getRParen());
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.h
index 9a0409b83fff3..2638a09730f7e 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.h
@@ -20,13 +20,16 @@ namespace clang::tidy::readability {
 /// https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-parentheses.html
 class RedundantParenthesesCheck : public ClangTidyCheck {
 public:
-  RedundantParenthesesCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  RedundantParenthesesCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus | LangOpts.C99;
   }
+
+private:
+  const std::vector<StringRef> AllowedDecls;
 };
 
 } // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst
index 23d975e646490..66b31fae2f2ed 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst
@@ -27,3 +27,16 @@ affect the semantics.
 .. code-block:: c++
 
   int a = (1 * 2) + 3; // no warning
+
+Options
+-------
+
+.. option:: AllowedDecls
+
+  A semicolon-separated list of names of declarations included variables and
+  functions to ignore when the parenthese around it.
+  The default is an `std::max;std::min`.
+  
+  Some std library functions may have the same name as widely used function-like
+  macro. For example, ``std::max`` and ``max`` macro. A work around to distinguish
+  them is adding parenthese around functions to prevent function-like macro.
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 926cb118c77cf..c77608c66469c 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
@@ -62,3 +62,12 @@ void exceptions() {
   // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant parentheses around expression [readability-redundant-parentheses]
   // CHECK-FIXES:    alignof(3);
 }
+
+namespace std {
+  template<class T> T max(T, T);
+  template<class T> T min(T, T);
+} // namespace std
+void ignoreStdMaxMin() {
+  (std::max)(1,2);
+  (std::min)(1,2);
+}

llvmbot · 2025-10-23T14:40:31Z

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes

Fixed: #164125
Add a new option to ignore some decls.

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

4 Files Affected:

(modified) clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp (+22)
(modified) clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.h (+5-2)
(modified) clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst (+13)
(modified) clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses.cpp (+9)

diff --git a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
index 0ab59fff39d88..2b5cf12603609 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "RedundantParenthesesCheck.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/Expr.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
@@ -32,6 +33,18 @@ AST_MATCHER(ParenExpr, isInMacro) {
 
 } // namespace
 
+RedundantParenthesesCheck::RedundantParenthesesCheck(StringRef Name,
+                                                     ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      AllowedDecls(utils::options::parseStringList(
+          Options.get("AllowedDecls", "std::max;std::min"))) {}
+
+void RedundantParenthesesCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "AllowedDecls",
+                utils::options::serializeStringList(AllowedDecls));
+}
+
 void RedundantParenthesesCheck::registerMatchers(MatchFinder *Finder) {
   const auto ConstantExpr =
       expr(anyOf(integerLiteral(), floatLiteral(), characterLiteral(),
@@ -47,6 +60,15 @@ void RedundantParenthesesCheck::registerMatchers(MatchFinder *Finder) {
 
 void RedundantParenthesesCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *PE = Result.Nodes.getNodeAs<ParenExpr>("dup");
+  if (auto *DRE = dyn_cast<DeclRefExpr>(PE->getSubExpr())) {
+    const std::string Name = DRE->getDecl()->getQualifiedNameAsString();
+    llvm::errs() << Name << "\n";
+    bool Allowed = llvm::any_of(AllowedDecls, [&Name](const llvm::Regex &NM) {
+      return NM.isValid() && NM.match(Name);
+    });
+    if (Allowed)
+      return;
+  }
   diag(PE->getBeginLoc(), "redundant parentheses around expression")
       << FixItHint::CreateRemoval(PE->getLParen())
       << FixItHint::CreateRemoval(PE->getRParen());
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.h
index 9a0409b83fff3..2638a09730f7e 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.h
@@ -20,13 +20,16 @@ namespace clang::tidy::readability {
 /// https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-parentheses.html
 class RedundantParenthesesCheck : public ClangTidyCheck {
 public:
-  RedundantParenthesesCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  RedundantParenthesesCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus | LangOpts.C99;
   }
+
+private:
+  const std::vector<StringRef> AllowedDecls;
 };
 
 } // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst
index 23d975e646490..66b31fae2f2ed 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst
@@ -27,3 +27,16 @@ affect the semantics.
 .. code-block:: c++
 
   int a = (1 * 2) + 3; // no warning
+
+Options
+-------
+
+.. option:: AllowedDecls
+
+  A semicolon-separated list of names of declarations included variables and
+  functions to ignore when the parenthese around it.
+  The default is an `std::max;std::min`.
+  
+  Some std library functions may have the same name as widely used function-like
+  macro. For example, ``std::max`` and ``max`` macro. A work around to distinguish
+  them is adding parenthese around functions to prevent function-like macro.
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 926cb118c77cf..c77608c66469c 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
@@ -62,3 +62,12 @@ void exceptions() {
   // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant parentheses around expression [readability-redundant-parentheses]
   // CHECK-FIXES:    alignof(3);
 }
+
+namespace std {
+  template<class T> T max(T, T);
+  template<class T> T min(T, T);
+} // namespace std
+void ignoreStdMaxMin() {
+  (std::max)(1,2);
+  (std::min)(1,2);
+}

clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp

clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst

clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp

clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst

…heck.cpp

clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst

…-parentheses.rst Co-authored-by: EugeneZelenko <[email protected]>

vbvictor

LGTM

firewave · 2025-10-28T13:20:50Z

This option does not help with my problem because it happens with local declarations. So I either have suppressions all over the code or I have an growing lists of symbols I need to add to the configuration.

HerrCai0907 · 2025-10-28T16:22:23Z

is option does not help with my problem because it happens with local declarations. So I either have suppressions all over the code or I have an growing lists of symbols I need to add to the configuration.

In my opinion, it shouldn't be too much that macro and function have the same name.

The lists of symbols grow in the future meams there are some developers writing new code in this way. I am not sure why they want to add parenthesses instead just rename the fucntion..

The biggest reason to add this options is Microsoft declare min and max in the system header.

firewave · 2025-10-28T17:19:37Z

The lists of symbols grow in the future meams there are some developers writing new code in this way. I am not sure why they want to add parenthesses instead just rename the fucntion..

That seems to go beyond this check. It is a readability check about detecting "redundant" parentheses but in those cases they are not redundant.

If the added option to to discourage writing such code, then there should not be not option at all and no warning for such cases but a separate check that warns about the pattern.

HerrCai0907 · 2025-10-29T02:40:44Z

If the added option to to discourage writing such code, then there should not be not option at all and no warning for such cases but a separate check that warns about the pattern.

In theory true, but since c++ STL and Microsoft system header have conflict in min / max. we need to pay for them anyway.
I prefer to keep this option as it for this more common scenario.
Maybe another option which is false by default to prevent this rare scenario at least in my eyes.

firewave · 2025-10-29T11:01:25Z

Maybe another option which is false by default to prevent this rare scenario at least in my eyes.

At least to me it is not rare. So far I have encountered this is any major code base I was involved on at work. It might have been bad style but nobody ever raised concerns about it.

… widely used work around (llvm#164827) Part of llvm#164125 Add a new option to ignore some decls. --------- Co-authored-by: EugeneZelenko <[email protected]>

[clang-tidy][readability-redundant-parentheses] add option to prevent…

525b015

… widely used work around Fixed: llvm#164125 Add a new option to ignore some decls. Because it is a new check in this version, no release note needs to be updated.

llvmbot added clang-tools-extra clang-tidy labels Oct 23, 2025

EugeneZelenko requested review from carlosgalvezp, localspook and vbvictor October 23, 2025 15:24

EugeneZelenko reviewed Oct 23, 2025

View reviewed changes

clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp Outdated Show resolved Hide resolved

clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst Outdated Show resolved Hide resolved

vbvictor reviewed Oct 23, 2025

View reviewed changes

HerrCai0907 added 3 commits October 24, 2025 10:25

Update clang-tools-extra/clang-tidy/readability/RedundantParenthesesC…

8f84ad5

…heck.cpp

fix review

5e37c83

fix reviwe

50e451f

EugeneZelenko reviewed Oct 28, 2025

View reviewed changes

clang-tools-extra/docs/clang-tidy/checks/readability/redundant-parentheses.rst Outdated Show resolved Hide resolved

Update clang-tools-extra/docs/clang-tidy/checks/readability/redundant…

1cdb401

…-parentheses.rst Co-authored-by: EugeneZelenko <[email protected]>

HerrCai0907 requested a review from vbvictor October 28, 2025 06:00

vbvictor approved these changes Oct 28, 2025

View reviewed changes

HerrCai0907 merged commit e65d52a into llvm:main Oct 31, 2025
18 of 20 checks passed

HerrCai0907 deleted the add-option-AllowedDecls branch October 31, 2025 02:52

vbvictor mentioned this pull request Nov 7, 2025

clang tidy: add header ignore option to include duplicate files #167046

Open

[clang-tidy][readability-redundant-parentheses] add option to prevent widely used work around #164827

[clang-tidy][readability-redundant-parentheses] add option to prevent widely used work around #164827

Conversation

HerrCai0907 commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HerrCai0907 commented Oct 23, 2025

Uh oh!

llvmbot commented Oct 23, 2025

Uh oh!

llvmbot commented Oct 23, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vbvictor left a comment

Choose a reason for hiding this comment

Uh oh!

firewave commented Oct 28, 2025

Uh oh!

HerrCai0907 commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

firewave commented Oct 28, 2025

Uh oh!

HerrCai0907 commented Oct 29, 2025

Uh oh!

firewave commented Oct 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

HerrCai0907 commented Oct 23, 2025 •

edited

Loading

HerrCai0907 commented Oct 28, 2025 •

edited

Loading