[llvm-branch-commits] [clang] 5cc5d71 - Fix recursive error for constraints depending on itself incorrectly
Tom Stellard via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Jan 27 20:42:55 PST 2023
Author: Erich Keane
Date: 2023-01-27T20:39:29-08:00
New Revision: 5cc5d71a10077f4ce4c83ac7c2aee13dc8ea2739
URL: https://github.com/llvm/llvm-project/commit/5cc5d71a10077f4ce4c83ac7c2aee13dc8ea2739
DIFF: https://github.com/llvm/llvm-project/commit/5cc5d71a10077f4ce4c83ac7c2aee13dc8ea2739.diff
LOG: Fix recursive error for constraints depending on itself incorrectly
Fixes: #60323
https://github.com/llvm/llvm-project/issues/60323
The problem is that we are profiling the 'Expr' components directly,
however when they contain an unresolved lookup, those canonicalize
identically. The result was the two versions of calls to 'go' were
canonicalized identically.
This patch fixes this by ensuring we consider the declaration the
constraint is attached to, when possible. When not, we skip the
diagnostic.
The result is that we are relaxing our diagnostic in some cases (Of
which I couldn't come up with a reproducer), such that we might see
overflows when evaluating constraints that depend on themselves in a way
that they are not attached to a declaration directly, such as if
they are nested requirements, though the hope is this won't be a
problem, since the 'parent' named constraint would catch this. I'm
hopeful that the 'worst case' is that we catch recursion 'later' in the
process, instead of immediately.
(cherry picked from commit 42667563721e139a93ab886119ea2780ebc3fecc)
Added:
Modified:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaConcept.cpp
clang/test/SemaTemplate/concepts-recursive-inst.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e52e853e1df86..66b018d8fba1a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7279,24 +7279,34 @@ class Sema final {
private:
// The current stack of constraint satisfactions, so we can exit-early.
- llvm::SmallVector<llvm::FoldingSetNodeID, 10> SatisfactionStack;
+ using SatisfactionStackEntryTy =
+ std::pair<const NamedDecl *, llvm::FoldingSetNodeID>;
+ llvm::SmallVector<SatisfactionStackEntryTy, 10>
+ SatisfactionStack;
public:
- void PushSatisfactionStackEntry(const llvm::FoldingSetNodeID &ID) {
- SatisfactionStack.push_back(ID);
+ void PushSatisfactionStackEntry(const NamedDecl *D,
+ const llvm::FoldingSetNodeID &ID) {
+ const NamedDecl *Can = cast<NamedDecl>(D->getCanonicalDecl());
+ SatisfactionStack.emplace_back(Can, ID);
}
void PopSatisfactionStackEntry() { SatisfactionStack.pop_back(); }
- bool SatisfactionStackContains(const llvm::FoldingSetNodeID &ID) const {
- return llvm::find(SatisfactionStack, ID) != SatisfactionStack.end();
+ bool SatisfactionStackContains(const NamedDecl *D,
+ const llvm::FoldingSetNodeID &ID) const {
+ const NamedDecl *Can = cast<NamedDecl>(D->getCanonicalDecl());
+ return llvm::find(SatisfactionStack,
+ SatisfactionStackEntryTy{Can, ID}) !=
+ SatisfactionStack.end();
}
// Resets the current SatisfactionStack for cases where we are instantiating
// constraints as a 'side effect' of normal instantiation in a way that is not
// indicative of recursive definition.
class SatisfactionStackResetRAII {
- llvm::SmallVector<llvm::FoldingSetNodeID, 10> BackupSatisfactionStack;
+ llvm::SmallVector<SatisfactionStackEntryTy, 10>
+ BackupSatisfactionStack;
Sema &SemaRef;
public:
@@ -7309,8 +7319,8 @@ class Sema final {
}
};
- void
- SwapSatisfactionStack(llvm::SmallVectorImpl<llvm::FoldingSetNodeID> &NewSS) {
+ void SwapSatisfactionStack(
+ llvm::SmallVectorImpl<SatisfactionStackEntryTy> &NewSS) {
SatisfactionStack.swap(NewSS);
}
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 4d4b2482d046e..86e63f88378c7 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -150,11 +150,19 @@ bool Sema::CheckConstraintExpression(const Expr *ConstraintExpression,
namespace {
struct SatisfactionStackRAII {
Sema &SemaRef;
- SatisfactionStackRAII(Sema &SemaRef, llvm::FoldingSetNodeID FSNID)
+ bool Inserted = false;
+ SatisfactionStackRAII(Sema &SemaRef, const NamedDecl *ND,
+ llvm::FoldingSetNodeID FSNID)
: SemaRef(SemaRef) {
- SemaRef.PushSatisfactionStackEntry(FSNID);
+ if (ND) {
+ SemaRef.PushSatisfactionStackEntry(ND, FSNID);
+ Inserted = true;
+ }
+ }
+ ~SatisfactionStackRAII() {
+ if (Inserted)
+ SemaRef.PopSatisfactionStackEntry();
}
- ~SatisfactionStackRAII() { SemaRef.PopSatisfactionStackEntry(); }
};
} // namespace
@@ -273,7 +281,8 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
}
static bool
-DiagRecursiveConstraintEval(Sema &S, llvm::FoldingSetNodeID &ID, const Expr *E,
+DiagRecursiveConstraintEval(Sema &S, llvm::FoldingSetNodeID &ID,
+ const NamedDecl *Templ, const Expr *E,
const MultiLevelTemplateArgumentList &MLTAL) {
E->Profile(ID, S.Context, /*Canonical=*/true);
for (const auto &List : MLTAL)
@@ -286,7 +295,7 @@ DiagRecursiveConstraintEval(Sema &S, llvm::FoldingSetNodeID &ID, const Expr *E,
// expression, or when trying to determine the constexpr-ness of special
// members. Otherwise we could just use the
// Sema::InstantiatingTemplate::isAlreadyBeingInstantiated function.
- if (S.SatisfactionStackContains(ID)) {
+ if (S.SatisfactionStackContains(Templ, ID)) {
S.Diag(E->getExprLoc(), diag::err_constraint_depends_on_self)
<< const_cast<Expr *>(E) << E->getSourceRange();
return true;
@@ -317,13 +326,14 @@ static ExprResult calculateConstraintSatisfaction(
return ExprError();
llvm::FoldingSetNodeID ID;
- if (DiagRecursiveConstraintEval(S, ID, AtomicExpr, MLTAL)) {
+ if (Template &&
+ DiagRecursiveConstraintEval(S, ID, Template, AtomicExpr, MLTAL)) {
Satisfaction.IsSatisfied = false;
Satisfaction.ContainsErrors = true;
return ExprEmpty();
}
- SatisfactionStackRAII StackRAII(S, ID);
+ SatisfactionStackRAII StackRAII(S, Template, ID);
// We do not want error diagnostics escaping here.
Sema::SFINAETrap Trap(S);
diff --git a/clang/test/SemaTemplate/concepts-recursive-inst.cpp b/clang/test/SemaTemplate/concepts-recursive-inst.cpp
index d370f0e2a641b..9330df8cdd039 100644
--- a/clang/test/SemaTemplate/concepts-recursive-inst.cpp
+++ b/clang/test/SemaTemplate/concepts-recursive-inst.cpp
@@ -113,3 +113,33 @@ namespace GH50891 {
} // namespace GH50891
+
+namespace GH60323 {
+ // This should not diagnose, as it does not depend on itself.
+ struct End {
+ template<class T>
+ void go(T t) { }
+
+ template<class T>
+ auto endparens(T t)
+ requires requires { go(t); }
+ { return go(t); }
+ };
+
+ struct Size {
+ template<class T>
+ auto go(T t)
+ { return End().endparens(t); }
+
+ template<class T>
+ auto sizeparens(T t)
+ requires requires { go(t); }
+ { return go(t); }
+ };
+
+ int f()
+ {
+ int i = 42;
+ Size().sizeparens(i);
+ }
+}
More information about the llvm-branch-commits
mailing list