llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: David Meng (davidmenggx)
<details>
<summary>Changes</summary>
An operand of a requires clause or a concept definition must be a primary
expression. Parentheses around a call, a member access, or a subscript are
required.
```cpp
template <typename T>
requires (valid<T>()) // previously reported as redundant
constexpr T forward(T val) { return val; }
```
Parentheses around a literal, a name, or another parenthesized expression are
still reported, since those stay valid operands.
Fixes https://github.com/llvm/llvm-project/issues/218956
---
Full diff: https://github.com/llvm/llvm-project/pull/219347.diff
3 Files Affected:
- (modified)
clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp (+59-6)
- (modified) clang-tools-extra/docs/ReleaseNotes.md (+4)
- (added)
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses-cxx20.cpp
(+70)
``````````diff
diff --git
a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
index 8435c438360a5..9dab49baa25ed 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp
@@ -9,7 +9,10 @@
#include "RedundantParenthesesCheck.h"
#include "../utils/Matchers.h"
#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/ExprConcepts.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/ASTMatchers/ASTMatchersMacros.h"
@@ -35,6 +38,51 @@ AST_MATCHER(ParenExpr, isInMacro) {
} // namespace
+// Returns true if `E` is an operand of a requires-clause or of a concept
+// definition.
+static bool isConstraintOperand(const Expr *E, ASTContext &Ctx) {
+ for (const DynTypedNode &Parent : Ctx.getParents(*E)) {
+ const auto *ParentExpr = Parent.get<Expr>();
+ const auto *BO = dyn_cast_or_null<BinaryOperator>(ParentExpr);
+ if (ParentExpr &&
+ (ParentExpr->IgnoreImplicit() == E || (BO && BO->isLogicalOp()))) {
+ if (isConstraintOperand(ParentExpr, Ctx))
+ return true;
+ continue;
+ }
+ // The 'requires (E);' of a nested requirement. Other expressions inside a
+ // requires-expression are unrestricted.
+ if (const auto *RE = Parent.get<RequiresExpr>()) {
+ if (llvm::any_of(
+ RE->getRequirements(), [E](const concepts::Requirement *R) {
+ const auto *NR = dyn_cast<concepts::NestedRequirement>(R);
+ return NR && !NR->hasInvalidConstraint() &&
+ NR->getConstraintExpr() == E;
+ }))
+ return true;
+ continue;
+ }
+ const auto *D = Parent.get<Decl>();
+ if (!D)
+ continue;
+ // The requires-clause of a template parameter list.
+ const auto *TD = dyn_cast<TemplateDecl>(D);
+ const TemplateParameterList *TPL =
+ TD ? TD->getTemplateParameters() : D->getDescribedTemplateParams();
+ if (TPL && TPL->getRequiresClause() == E)
+ return true;
+ // The right hand side of a concept definition.
+ if (const auto *CD = dyn_cast<ConceptDecl>(D);
+ CD && CD->getConstraintExpr() == E)
+ return true;
+ // A trailing requires-clause.
+ if (const auto *FD = dyn_cast<FunctionDecl>(D);
+ FD && FD->getTrailingRequiresClause().ConstraintExpr == E)
+ return true;
+ }
+ return false;
+}
+
static FixItHint createSpacedRemoval(SourceLocation Loc,
const SourceManager &SM,
const LangOptions &LangOpts) {
@@ -68,15 +116,17 @@ void
RedundantParenthesesCheck::registerMatchers(MatchFinder *Finder) {
const auto ConstantExpr =
expr(anyOf(integerLiteral(), floatLiteral(), characterLiteral(),
cxxBoolLiteral(), stringLiteral(), cxxNullPtrLiteralExpr()));
+ const auto ConstraintSensitiveExpr =
+ expr(anyOf(parenExpr(), memberExpr(),
+ callExpr(unless(cxxOperatorCallExpr(
+ unless(hasAnyOperatorName("()", "[]"))))),
+ arraySubscriptExpr()));
Finder->addMatcher(
parenExpr(subExpr(anyOf(
- parenExpr(), ConstantExpr,
+ ConstraintSensitiveExpr.bind("constraint_sensitive"),
+ ConstantExpr,
declRefExpr(to(namedDecl(unless(
- matchers::matchesAnyListedRegexName(AllowedDecls))))),
- memberExpr(),
- callExpr(unless(cxxOperatorCallExpr(
- unless(hasAnyOperatorName("()", "[]"))))),
- arraySubscriptExpr())),
+
matchers::matchesAnyListedRegexName(AllowedDecls))))))),
unless(anyOf(isInMacro(),
// sizeof(...) is common used.
hasParent(unaryExprOrTypeTraitExpr()))))
@@ -86,6 +136,9 @@ void RedundantParenthesesCheck::registerMatchers(MatchFinder
*Finder) {
void RedundantParenthesesCheck::check(const MatchFinder::MatchResult &Result) {
const auto *PE = Result.Nodes.getNodeAs<ParenExpr>("dup");
+ if (Result.Nodes.getNodeAs<Expr>("constraint_sensitive") &&
+ isConstraintOperand(PE, *Result.Context))
+ return;
diag(PE->getBeginLoc(), "redundant parentheses around expression")
<< createSpacedRemoval(PE->getLParen(), *Result.SourceManager,
getLangOpts())
diff --git a/clang-tools-extra/docs/ReleaseNotes.md
b/clang-tools-extra/docs/ReleaseNotes.md
index 6fe497e5f6eaf..127f2a385a264 100644
--- a/clang-tools-extra/docs/ReleaseNotes.md
+++ b/clang-tools-extra/docs/ReleaseNotes.md
@@ -207,6 +207,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.
+- Improved {doc}`readability-redundant-parentheses
+ <clang-tidy/checks/readability/redundant-parentheses>` check by fixing false
+ positives on parentheses in a requires-clause or a concept definition.
+
- Improved {doc}`readability-trailing-comma
<clang-tidy/checks/readability/trailing-comma>` check by fixing false
positives on designated initializers, where initializer lists synthesized
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses-cxx20.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses-cxx20.cpp
new file mode 100644
index 0000000000000..c60614d9137a7
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-parentheses-cxx20.cpp
@@ -0,0 +1,70 @@
+// RUN: %check_clang_tidy -std=c++20-or-later %s
readability-redundant-parentheses %t
+
+template <typename T>
+constexpr bool valid() { return sizeof(T) >= 1; }
+
+struct S { bool M; static constexpr bool SM = true; };
+constexpr S Obj{true};
+constexpr bool Arr[2] = {true, true};
+constexpr bool Cond = true;
+
+template <typename T>
+requires (valid<T>())
+constexpr T forward(T val) { return val; }
+
+template <typename T>
+void trailing(T) requires (valid<T>());
+
+template <typename T> struct PartialSpec;
+template <typename T>
+requires (valid<T>()) struct PartialSpec<T *> {};
+
+template <typename T>
+concept Concept = (valid<T>());
+
+template <typename T>
+concept NestedRequirement = requires { requires (valid<T>()); };
+
+auto Lambda = []<typename T> requires (valid<T>()) (T) {};
+
+template <typename T>
+requires (valid<T>()) && (valid<T>()) || (valid<T>())
+void chained(T);
+
+template <typename T>
+requires (Obj.M) && (Arr[0])
+void chainedLValue(T);
+
+template <typename T>
+requires ((valid<T *>()))
+void nested(T);
+// CHECK-MESSAGES: :[[@LINE-2]]:11: warning: redundant parentheses around
expression [readability-redundant-parentheses]
+// CHECK-FIXES: requires (valid<T *>())
+
+template <typename T>
+concept SimpleRequirement = requires { (valid<T>()); };
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: redundant parentheses around
expression [readability-redundant-parentheses]
+// CHECK-FIXES: concept SimpleRequirement = requires { valid<T>(); };
+
+template <typename T>
+requires (!(valid<T>()))
+void negated(T);
+// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: redundant parentheses around
expression [readability-redundant-parentheses]
+// CHECK-FIXES: requires (!valid<T>())
+
+template <typename T>
+requires (true)
+void primaryLiteral(T);
+// CHECK-MESSAGES: :[[@LINE-2]]:10: warning: redundant parentheses around
expression [readability-redundant-parentheses]
+// CHECK-FIXES: requires true
+
+template <typename T>
+requires (Cond) && (valid<T>())
+void primaryChained(T);
+// CHECK-MESSAGES: :[[@LINE-2]]:10: warning: redundant parentheses around
expression [readability-redundant-parentheses]
+// CHECK-FIXES: requires Cond && (valid<T>())
+
+template <typename T>
+requires (S::SM) void qualified(T);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: redundant parentheses around
expression [readability-redundant-parentheses]
+// CHECK-FIXES: requires S::SM void qualified(T);
``````````
</details>
https://github.com/llvm/llvm-project/pull/219347
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
