vsapsai added a comment.
Thanks for the changes, they look good! While I was looking how we handle
"requires" constraints in other cases, I've noticed that they are suspiciously
similar. That's why I offer the following change to unify comparison of the
constraint expressions
diff --git a/clang/include/clang/AST/ASTContext.h
b/clang/include/clang/AST/ASTContext.h
index 92293622cc3d..555669f027a7 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2664,6 +2664,11 @@ public:
/// that they may be used in declarations of the same template.
bool isSameTemplateParameter(const NamedDecl *X, const NamedDecl *Y) const;
+ /// Determine whether two 'requires' expressions are similar enough.
+ /// Use of 'requires' isn't mandatory, works with constraints expressed in
+ /// other ways too.
+ bool isSameConstraintExpr(const Expr *XCE, const Expr *YCE) const;
+
/// Retrieve the "canonical" template argument.
///
/// The canonical template argument is the simplest template argument
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index aced0ab39ace..f3937d6304f9 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -6245,14 +6245,10 @@ bool ASTContext::isSameTemplateParameter(const
NamedDecl *X,
auto *TYTCArgs = TYTC->getTemplateArgsAsWritten();
if (TXTCArgs->NumTemplateArgs != TYTCArgs->NumTemplateArgs)
return false;
- llvm::FoldingSetNodeID XID, YID;
- for (auto &ArgLoc : TXTCArgs->arguments())
- ArgLoc.getArgument().Profile(XID, X->getASTContext());
- for (auto &ArgLoc : TYTCArgs->arguments())
- ArgLoc.getArgument().Profile(YID, Y->getASTContext());
- if (XID != YID)
- return false;
}
+ if (!isSameConstraintExpr(TXTC->getImmediatelyDeclaredConstraint(),
+ TYTC->getImmediatelyDeclaredConstraint()))
+ return false;
}
return true;
}
@@ -6279,15 +6275,20 @@ bool ASTContext::isSameTemplateParameterList(
if (!isSameTemplateParameter(X->getParam(I), Y->getParam(I)))
return false;
- const Expr *XRC = X->getRequiresClause();
- const Expr *YRC = Y->getRequiresClause();
- if (!XRC != !YRC)
+ if (!isSameConstraintExpr(X->getRequiresClause(), Y->getRequiresClause()))
return false;
- if (XRC) {
- llvm::FoldingSetNodeID XRCID, YRCID;
- XRC->Profile(XRCID, *this, /*Canonical=*/true);
- YRC->Profile(YRCID, *this, /*Canonical=*/true);
- if (XRCID != YRCID)
+
+ return true;
+}
+
+bool ASTContext::isSameConstraintExpr(const Expr *XCE, const Expr *YCE)
const {
+ if (!XCE != !YCE)
+ return false;
+ if (XCE) {
+ llvm::FoldingSetNodeID XCEID, YCEID;
+ XCE->Profile(XCEID, *this, /*Canonical=*/true);
+ YCE->Profile(YCEID, *this, /*Canonical=*/true);
+ if (XCEID != YCEID)
return false;
}
@@ -6450,17 +6451,9 @@ bool ASTContext::isSameEntity(const NamedDecl *X,
const NamedDecl *Y) const {
return false;
}
- const Expr *XRC = FuncX->getTrailingRequiresClause();
- const Expr *YRC = FuncY->getTrailingRequiresClause();
- if (!XRC != !YRC)
+ if (!isSameConstraintExpr(FuncX->getTrailingRequiresClause(),
+ FuncY->getTrailingRequiresClause()))
return false;
- if (XRC) {
- llvm::FoldingSetNodeID XRCID, YRCID;
- XRC->Profile(XRCID, *this, /*Canonical=*/true);
- YRC->Profile(YRCID, *this, /*Canonical=*/true);
- if (XRCID != YRCID)
- return false;
- }
auto GetTypeAsWritten = [](const FunctionDecl *FD) {
// Map to the first declaration that we've already merged into this
one.
In `ASTContext::isSameTemplateParameter` it is exactly the same change as yours
modulo comments.
================
Comment at: clang/test/Modules/concept.cppm:38
+ template <__integer_like _Tp, C<_Tp> Sentinel>
+ constexpr _Tp operator()(_Tp &&__t, Sentinel &&last) const {
+ return __t;
----------------
In what cases `operator()` is critical for the test? I was thinking about
replacing with something like "funcA", "funcB", "funcC", so that diagnostic
verification is easier because it is tricky to understand which method
"__fn::operator()" refers to.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129068/new/
https://reviews.llvm.org/D129068
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
