erichkeane updated this revision to Diff 472093.
erichkeane added a comment.
ACTUALLY add the libcxx testing, also rebase so hopefully it applies cleanly.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136975/new/
https://reviews.llvm.org/D136975
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaConcept.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaTemplate/concepts-GH53213.cpp
clang/test/SemaTemplate/concepts-recursive-inst.cpp
libcxx/DELETE_ME.txt
Index: libcxx/DELETE_ME.txt
===================================================================
--- /dev/null
+++ libcxx/DELETE_ME.txt
@@ -0,0 +1 @@
+D136975
Index: clang/test/SemaTemplate/concepts-recursive-inst.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaTemplate/concepts-recursive-inst.cpp
@@ -0,0 +1,122 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+namespace GH53213 {
+template<typename T>
+concept c = requires(T t) { f(t); }; // #CDEF
+
+auto f(c auto); // #FDEF
+
+void g() {
+ f(0);
+ // expected-error@-1{{no matching function for call to 'f'}}
+ // expected-note@#FDEF{{constraints not satisfied}}
+ // expected-note@#FDEF{{because 'int' does not satisfy 'c'}}
+ // expected-note@#CDEF{{because 'f(t)' would be invalid: no matching function for call to 'f'}}
+}
+} // namespace GH53213
+
+namespace GH45736 {
+struct constrained;
+
+template<typename T>
+ struct type {
+ };
+template<typename T>
+ constexpr bool f(type<T>) {
+ return true;
+ }
+
+template<typename T>
+ concept matches = f(type<T>());
+
+
+struct constrained {
+ template<typename U> requires matches<U>
+ explicit constrained(U value) {
+ }
+};
+
+bool f(constrained const &) {
+ return true;
+}
+
+struct outer {
+ constrained state;
+};
+
+bool f(outer const & x) {
+ return f(x.state);
+}
+} // namespace GH45736
+
+namespace DirectRecursiveCheck {
+template<class T>
+concept NotInf = true;
+template<class T>
+concept Inf = requires(T& v){ // #INF_REQ
+ {begin(v)}; // #INF_BEGIN_EXPR
+};
+
+void begin(NotInf auto& v){ } // #NOTINF_BEGIN
+// This lookup should fail, since it results in a recursive check.
+// However, this is a 'hard failure'(not a SFINAE failure or constraints
+// violation), so it needs to cause the entire lookup to fail.
+void begin(Inf auto& v){ } // #INF_BEGIN
+
+struct my_range{
+} rng;
+
+void baz() {
+auto it = begin(rng); // #BEGIN_CALL
+// expected-error@#INF_BEGIN {{satisfaction of constraint 'Inf<Inf auto>' depends on itself}}
+// expected-note@#INF_BEGIN {{while substituting template arguments into constraint expression here}}
+// expected-note@#INF_BEGIN_EXPR {{while checking constraint satisfaction for template 'begin<DirectRecursiveCheck::my_range>' required here}}
+// expected-note@#INF_BEGIN_EXPR {{while substituting deduced template arguments into function template 'begin'}}
+// expected-note@#INF_BEGIN_EXPR {{in instantiation of requirement here}}
+// expected-note@#INF_REQ {{while substituting template arguments into constraint expression here}}
+// expected-note@#INF_BEGIN {{while checking the satisfaction of concept 'Inf<DirectRecursiveCheck::my_range>' requested here}}
+// expected-note@#INF_BEGIN {{while substituting template arguments into constraint expression here}}
+// expected-note@#BEGIN_CALL {{while checking constraint satisfaction for template 'begin<DirectRecursiveCheck::my_range>' required here}}
+// expected-note@#BEGIN_CALL {{in instantiation of function template specialization}}
+
+// Fallout of the failure is failed lookup, which is necessary to stop odd
+// cascading errors.
+// expected-error@#BEGIN_CALL {{no matching function for call to 'begin'}}
+// expected-note@#NOTINF_BEGIN {{candidate function}}
+// expected-note@#INF_BEGIN{{candidate template ignored: constraints not satisfied}}
+}
+} // namespace DirectRecursiveCheck
+
+namespace GH50891 {
+ template <typename T>
+ concept Numeric = requires(T a) { // #NUMERIC
+ foo(a); // #FOO_CALL
+ };
+
+ struct Deferred {
+ friend void foo(Deferred);
+ template <Numeric TO> operator TO(); // #OP_TO
+ };
+
+ static_assert(Numeric<Deferred>); // #STATIC_ASSERT
+ // expected-error@#OP_TO {{satisfaction of constraint 'Numeric<TO>' depends on itself}}
+ // expected-note@#OP_TO {{while substituting template arguments into constraint expression here}}
+ // FIXME: The following two refer to type-parameter-0-0, it would be nice to
+ // see if we could instead diagnose with the sugared name.
+ // expected-note@#FOO_CALL {{while checking constraint satisfaction for template}}
+ // expected-note@#FOO_CALL {{while substituting deduced template arguments into function template}}
+ // expected-note@#FOO_CALL {{in instantiation of requirement here}}
+ // expected-note@#NUMERIC {{while substituting template arguments into constraint expression here}}
+ // expected-note@#OP_TO {{skipping 2 contexts in backtrace}}
+ // expected-note@#FOO_CALL {{while checking constraint satisfaction for template}}
+ // expected-note@#FOO_CALL {{in instantiation of function template specialization}}
+ // expected-note@#FOO_CALL {{in instantiation of requirement here}}
+ // expected-note@#NUMERIC {{while substituting template arguments into constraint expression here}}
+ // expected-note@#STATIC_ASSERT{{while checking the satisfaction of concept 'Numeric<Deferred>' requested here}}
+
+ // Fallout of that failure is that deferred does not satisfy numeric,
+ // which is unfortunate, but about what we can accomplish here.
+ // expected-error@#STATIC_ASSERT {{static assertion failed}}
+ // expected-note@#STATIC_ASSERT{{because 'Deferred' does not satisfy 'Numeric'}}
+ // expected-note@#FOO_CALL {{because 'foo(a)' would be invalid}}
+} // namespace GH50891
+
Index: clang/test/SemaTemplate/concepts-GH53213.cpp
===================================================================
--- clang/test/SemaTemplate/concepts-GH53213.cpp
+++ /dev/null
@@ -1,49 +0,0 @@
-// RUN: %clang_cc1 -std=c++20 -verify %s
-namespace GH53213 {
-template<typename T>
-concept c = requires(T t) { f(t); }; // #CDEF
-
-auto f(c auto); // #FDEF
-
-void g() {
- f(0);
- // expected-error@-1{{no matching function for call to 'f'}}
- // expected-note@#FDEF{{constraints not satisfied}}
- // expected-note@#FDEF{{because 'int' does not satisfy 'c'}}
- // expected-note@#CDEF{{because 'f(t)' would be invalid: no matching function for call to 'f'}}
-}
-} // namespace GH53213
-
-namespace GH45736 {
-struct constrained;
-
-template<typename T>
- struct type {
- };
-template<typename T>
- constexpr bool f(type<T>) {
- return true;
- }
-
-template<typename T>
- concept matches = f(type<T>());
-
-
-struct constrained {
- template<typename U> requires matches<U>
- explicit constrained(U value) {
- }
-};
-
-bool f(constrained const &) {
- return true;
-}
-
-struct outer {
- constrained state;
-};
-
-bool f(outer const & x) {
- return f(x.state);
-}
-} // namespace GH45736
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -13147,14 +13147,19 @@
namespace {
class BuildRecoveryCallExprRAII {
Sema &SemaRef;
+ llvm::SmallVector<llvm::FoldingSetNodeID, 10> SatisfactionStack;
public:
BuildRecoveryCallExprRAII(Sema &S) : SemaRef(S) {
assert(SemaRef.IsBuildingRecoveryCallExpr == false);
SemaRef.IsBuildingRecoveryCallExpr = true;
+ // Ensure we clear the satisfaction stack here, so that we don't fail
+ // incorrectly for recursive lookups.
+ SemaRef.SwapSatisfactionStack(SatisfactionStack);
}
~BuildRecoveryCallExprRAII() {
SemaRef.IsBuildingRecoveryCallExpr = false;
+ SemaRef.SwapSatisfactionStack(SatisfactionStack);
}
};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -7283,6 +7283,11 @@
if (!Ctor && !ClassDecl->isLiteral())
return false;
+ // Suppress duplicate constraint checking here, in case a constraint check
+ // caused us to decide to do this. Any truely recursive checks will get
+ // caught during these checks anyway.
+ Sema::SatisfactionStackRAII SSRAII{S};
+
// -- every constructor involved in initializing [...] base class
// sub-objects shall be a constexpr constructor;
// -- the assignment operator selected to copy/move each direct base
@@ -7290,7 +7295,6 @@
for (const auto &B : ClassDecl->bases()) {
const RecordType *BaseType = B.getType()->getAs<RecordType>();
if (!BaseType) continue;
-
CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(BaseType->getDecl());
if (!specialMemberIsConstexpr(S, BaseClassDecl, CSM, 0, ConstArg,
InheritedCtor, Inherited))
Index: clang/lib/Sema/SemaConcept.cpp
===================================================================
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -146,6 +146,18 @@
return true;
}
+namespace {
+ struct SatisfactionStackRAII {
+ Sema &SemaRef;
+ SatisfactionStackRAII(Sema &SemaRef, llvm::FoldingSetNodeID FSNID) :SemaRef(SemaRef) {
+ SemaRef.PushSatisfactionStackEntry(FSNID);
+ }
+ ~SatisfactionStackRAII() {
+ SemaRef.PopSatisfactionStackEntry();
+ }
+ };
+}
+
template <typename AtomicEvaluator>
static ExprResult
calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
@@ -206,6 +218,7 @@
// Evaluator has decided satisfaction without yielding an expression.
return ExprEmpty();
+
// We don't have the ability to evaluate this, since it contains a
// RecoveryExpr, so we want to fail overload resolution. Otherwise,
// we'd potentially pick up a different overload, and cause confusing
@@ -258,6 +271,29 @@
return SubstitutedAtomicExpr;
}
+static bool
+DiagRecursiveConstraintEval(Sema &S, llvm::FoldingSetNodeID &ID, const Expr *E,
+ const MultiLevelTemplateArgumentList &MLTAL) {
+ E->Profile(ID, S.Context, /*Canonical=*/true);
+ for (const auto &List : MLTAL)
+ for (const auto &TemplateArg : List.Args)
+ TemplateArg.Profile(ID, S.Context);
+
+ // Note that we have to do this with our own collection, because there are
+ // times where a constraint-expression check can cause us to need to evaluate
+ // other constriants that are unrelated, such as when evaluating a recovery
+ // 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)) {
+ S.Diag(E->getExprLoc(), diag::err_constraint_depends_on_self)
+ << const_cast<Expr *>(E) << E->getSourceRange();
+ return true;
+ }
+
+ return false;
+}
+
static ExprResult calculateConstraintSatisfaction(
Sema &S, const NamedDecl *Template, SourceLocation TemplateNameLoc,
const MultiLevelTemplateArgumentList &MLTAL, const Expr *ConstraintExpr,
@@ -278,6 +314,16 @@
AtomicExpr->getSourceRange());
if (Inst.isInvalid())
return ExprError();
+
+ llvm::FoldingSetNodeID ID;
+ if (DiagRecursiveConstraintEval(S, ID, AtomicExpr, MLTAL)) {
+ Satisfaction.IsSatisfied = false;
+ Satisfaction.ContainsErrors = true;
+ return ExprEmpty();
+ }
+
+ SatisfactionStackRAII StackRAII(S, ID);
+
// We do not want error diagnostics escaping here.
Sema::SFINAETrap Trap(S);
SubstitutedExpression =
@@ -414,6 +460,7 @@
if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
ConvertedConstraints, TemplateArgsLists,
TemplateIDRange, *Satisfaction)) {
+ OutSatisfaction = *Satisfaction;
return true;
}
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -7221,7 +7221,43 @@
FunctionDecl *FD, llvm::Optional<ArrayRef<TemplateArgument>> TemplateArgs,
LocalInstantiationScope &Scope);
+private:
+ // The current stack of constraint satisfactions, so we can exit-early.
+ llvm::SmallVector<llvm::FoldingSetNodeID, 10>
+ SatisfactionStack;
public:
+ void PushSatisfactionStackEntry(const llvm::FoldingSetNodeID &ID) {
+ SatisfactionStack.push_back(ID);
+ }
+
+ void PopSatisfactionStackEntry() {
+ SatisfactionStack.pop_back();
+ }
+
+ bool SatisfactionStackContains(const llvm::FoldingSetNodeID &ID) {
+ return llvm::find(SatisfactionStack, ID) != SatisfactionStack.end();
+ }
+
+ class SatisfactionStackRAII {
+ llvm::SmallVector<llvm::FoldingSetNodeID, 10>
+ BackupSatisfactionStack;
+ Sema &SemaRef;
+
+ public:
+ SatisfactionStackRAII(Sema &S) : SemaRef(S){
+ SemaRef.SwapSatisfactionStack(BackupSatisfactionStack);
+ }
+
+ ~SatisfactionStackRAII() {
+ SemaRef.SwapSatisfactionStack(BackupSatisfactionStack);
+ }
+ };
+
+ void
+ SwapSatisfactionStack(llvm::SmallVectorImpl<llvm::FoldingSetNodeID> &NewSS) {
+ SatisfactionStack.swap(NewSS);
+ }
+
const NormalizedConstraint *
getNormalizedAssociatedConstraints(
NamedDecl *ConstrainedDecl, ArrayRef<const Expr *> AssociatedConstraints);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5205,6 +5205,8 @@
def err_template_recursion_depth_exceeded : Error<
"recursive template instantiation exceeded maximum depth of %0">,
DefaultFatal, NoSFINAE;
+def err_constraint_depends_on_self : Error<
+ "satisfaction of constraint '%0' depends on itself">, NoSFINAE;
def note_template_recursion_depth : Note<
"use -ftemplate-depth=N to increase recursive template instantiation depth">;
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -268,6 +268,10 @@
functions. `Issue 56154 <https://github.com/llvm/llvm-project/issues/56154>`_
- Fix handling of unexpanded packs in template argument expressions.
`Issue 58679 <https://github.com/llvm/llvm-project/issues/58679>`_
+- Fix a number of recursively-instantiated constraint issues, which would possibly
+ result in clang's stack-exhaustion.
+ `Issue 44304 <https://github.com/llvm/llvm-project/issues/44304>`_
+ `Issue 50891 <https://github.com/llvm/llvm-project/issues/50891>`_
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
