[clang] [clang-tools-extra] [llvm] [Sema] When checking for constraint equivalence, do not calculate satisfaction (PR #74490)
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 21 08:01:53 PST 2023
https://github.com/ilya-biryukov updated https://github.com/llvm/llvm-project/pull/74490
>From 51d6e95d67c3cc6070d926ef04b168c2ea429c7d Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Tue, 5 Dec 2023 14:28:01 +0100
Subject: [PATCH 1/3] [Sema] When checking for constraint equivalence, do not
calculate satisfaction
... and only look at equivalence of substituted expressions, not results
of constraint satisfaction.
Fixes #74314.
There is already some existing machinery for that in
`TemplateInstantiator` and `Sema` exposed separate functions for
substituting expressions with intention to do that:
- `Sema::SubstExpr` should not evaluate constraints.
- `Sema::SubstConstraintExpr` should.
However, both functions used to be equivalent. This commit changes the
former to actually avoid calculating constraint satisfaction and uses it
in the code path that matches definition and declaration.
---
clang/include/clang/Sema/Template.h | 1 +
clang/lib/Sema/SemaConcept.cpp | 15 ++++++-------
clang/lib/Sema/SemaTemplateInstantiate.cpp | 8 +++++++
.../SemaTemplate/concepts-out-of-line-def.cpp | 21 +++++++++++++++++++
4 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h
index 2a553054a0ce51..ce44aca797b0fb 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -564,6 +564,7 @@ enum class TemplateSubstitutionKind : char {
const MultiLevelTemplateArgumentList &TemplateArgs;
Sema::LateInstantiatedAttrVec* LateAttrs = nullptr;
LocalInstantiationScope *StartingScope = nullptr;
+ // Whether to evaluate the C++20 constraints or simply substitute into them.
bool EvaluateConstraints = true;
/// A list of out-of-line class template partial
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 719c6aab74e017..be3541573377bb 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -771,10 +771,9 @@ namespace {
};
} // namespace
-static const Expr *
-SubstituteConstraintExpression(Sema &S,
- const Sema::TemplateCompareNewDeclInfo &DeclInfo,
- const Expr *ConstrExpr) {
+static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
+ Sema &S, const Sema::TemplateCompareNewDeclInfo &DeclInfo,
+ const Expr *ConstrExpr) {
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
DeclInfo.getDecl(), DeclInfo.getLexicalDeclContext(), /*Final=*/false,
/*Innermost=*/nullptr,
@@ -798,7 +797,7 @@ SubstituteConstraintExpression(Sema &S,
if (auto *RD = dyn_cast<CXXRecordDecl>(DeclInfo.getDeclContext()))
ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers());
ExprResult SubstConstr =
- S.SubstConstraintExpr(const_cast<clang::Expr *>(ConstrExpr), MLTAL);
+ S.SubstExpr(const_cast<clang::Expr *>(ConstrExpr), MLTAL);
if (SFINAE.hasErrorOccurred() || !SubstConstr.isUsable())
return nullptr;
return SubstConstr.get();
@@ -814,12 +813,14 @@ bool Sema::AreConstraintExpressionsEqual(const NamedDecl *Old,
if (Old && !New.isInvalid() && !New.ContainsDecl(Old) &&
Old->getLexicalDeclContext() != New.getLexicalDeclContext()) {
if (const Expr *SubstConstr =
- SubstituteConstraintExpression(*this, Old, OldConstr))
+ SubstituteConstraintExpressionWithoutSatisfaction(*this, Old,
+ OldConstr))
OldConstr = SubstConstr;
else
return false;
if (const Expr *SubstConstr =
- SubstituteConstraintExpression(*this, New, NewConstr))
+ SubstituteConstraintExpressionWithoutSatisfaction(*this, New,
+ NewConstr))
NewConstr = SubstConstr;
else
return false;
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 6ad70109c8cee9..971a4c99add0cb 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1186,6 +1186,7 @@ namespace {
const MultiLevelTemplateArgumentList &TemplateArgs;
SourceLocation Loc;
DeclarationName Entity;
+ // Whether to evaluate the C++20 constraints or simply substitute into them.
bool EvaluateConstraints = true;
public:
@@ -2489,6 +2490,12 @@ TemplateInstantiator::TransformNestedRequirement(
Req->getConstraintExpr()->getBeginLoc(), Req,
Sema::InstantiatingTemplate::ConstraintsCheck{},
Req->getConstraintExpr()->getSourceRange());
+ if (!getEvaluateConstraints()) {
+ ExprResult TransConstraint = TransformExpr(Req->getConstraintExpr());
+ if (TransConstraint.isInvalid() || !TransConstraint.get())
+ return nullptr;
+ return RebuildNestedRequirement(TransConstraint.get());
+ }
ExprResult TransConstraint;
ConstraintSatisfaction Satisfaction;
@@ -4077,6 +4084,7 @@ Sema::SubstExpr(Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs) {
TemplateInstantiator Instantiator(*this, TemplateArgs,
SourceLocation(),
DeclarationName());
+ Instantiator.setEvaluateConstraints(false);
return Instantiator.TransformExpr(E);
}
diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
index f134394615fb24..7e79eeb6d279a0 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -504,3 +504,24 @@ struct bar {
bar<int> x;
} // namespace GH61763
+
+namespace GH74314 {
+template <class T, class U> constexpr bool is_same_v = __is_same(T, U);
+template <class T, class U> constexpr bool is_not_same_v = !__is_same(T, U);
+
+template <class Result>
+concept something_interesting = requires {
+ true;
+ requires is_same_v<int, Result>;
+};
+
+template <class T>
+struct X {
+ void foo() requires requires { requires is_not_same_v<T, int>; };
+};
+
+template <class T>
+void X<T>::foo() requires requires { requires something_interesting<T>; } {}
+// expected-error at -1{{definition of 'foo' does not match any declaration}}
+// expected-note@*{{}}
+} // namespace GH74314
\ No newline at end of file
>From 87a9ac815782723c63b0d57935049ba4e67c274f Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Thu, 21 Dec 2023 14:58:24 +0100
Subject: [PATCH 2/3] Provide a more localized fix, add tests with requires
expressions inside of decltype, unifty implementation of SubstConstraintExpr
and SubstExpr and add corresponding comments
---
clang/include/clang/Sema/Sema.h | 6 ++++--
clang/lib/Sema/SemaConcept.cpp | 4 ++--
clang/lib/Sema/SemaTemplateInstantiate.cpp | 18 ++++++++++++++----
.../SemaTemplate/concepts-out-of-line-def.cpp | 11 +++++++++++
4 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 38377f01a10086..f8ae488cdf2bda 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10240,11 +10240,13 @@ class Sema final {
~ConstraintEvalRAII() { TI.setEvaluateConstraints(OldValue); }
};
- // Unlike the above, this evaluates constraints, which should only happen at
- // 'constraint checking' time.
+ // Must be used instead of SubstExpr at 'constraint checking' time.
ExprResult
SubstConstraintExpr(Expr *E,
const MultiLevelTemplateArgumentList &TemplateArgs);
+ // Unlike the above, this does not evaluates constraints.
+ ExprResult SubstConstraintExprWithoutSatisfaction(
+ Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs);
/// Substitute the given template arguments into a list of
/// expressions, expanding pack expansions if required.
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index be3541573377bb..acfc00f4125407 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -796,8 +796,8 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
std::optional<Sema::CXXThisScopeRAII> ThisScope;
if (auto *RD = dyn_cast<CXXRecordDecl>(DeclInfo.getDeclContext()))
ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers());
- ExprResult SubstConstr =
- S.SubstExpr(const_cast<clang::Expr *>(ConstrExpr), MLTAL);
+ ExprResult SubstConstr = S.SubstConstraintExprWithoutSatisfaction(
+ const_cast<clang::Expr *>(ConstrExpr), MLTAL);
if (SFINAE.hasErrorOccurred() || !SubstConstr.isUsable())
return nullptr;
return SubstConstr.get();
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 971a4c99add0cb..dee7d1196e3227 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2494,7 +2494,12 @@ TemplateInstantiator::TransformNestedRequirement(
ExprResult TransConstraint = TransformExpr(Req->getConstraintExpr());
if (TransConstraint.isInvalid() || !TransConstraint.get())
return nullptr;
- return RebuildNestedRequirement(TransConstraint.get());
+ if (TransConstraint.get()->isInstantiationDependent())
+ return new (SemaRef.Context)
+ concepts::NestedRequirement(TransConstraint.get());
+ ConstraintSatisfaction Satisfaction;
+ return new (SemaRef.Context) concepts::NestedRequirement(
+ SemaRef.Context, TransConstraint.get(), Satisfaction);
}
ExprResult TransConstraint;
@@ -4084,20 +4089,25 @@ Sema::SubstExpr(Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs) {
TemplateInstantiator Instantiator(*this, TemplateArgs,
SourceLocation(),
DeclarationName());
- Instantiator.setEvaluateConstraints(false);
return Instantiator.TransformExpr(E);
}
ExprResult
Sema::SubstConstraintExpr(Expr *E,
const MultiLevelTemplateArgumentList &TemplateArgs) {
+ // FIXME: should call SubstExpr directly if this function is equivalent or
+ // should it be different?
+ return SubstExpr(E, TemplateArgs);
+}
+
+ExprResult Sema::SubstConstraintExprWithoutSatisfaction(
+ Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs) {
if (!E)
return E;
- // This is where we need to make sure we 'know' constraint checking needs to
- // happen.
TemplateInstantiator Instantiator(*this, TemplateArgs, SourceLocation(),
DeclarationName());
+ Instantiator.setEvaluateConstraints(false);
return Instantiator.TransformExpr(E);
}
diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
index 7e79eeb6d279a0..ea21f2720e45af 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -518,10 +518,21 @@ concept something_interesting = requires {
template <class T>
struct X {
void foo() requires requires { requires is_not_same_v<T, int>; };
+ void bar(decltype(requires { requires is_not_same_v<T, int>; }));
};
template <class T>
void X<T>::foo() requires requires { requires something_interesting<T>; } {}
// expected-error at -1{{definition of 'foo' does not match any declaration}}
// expected-note@*{{}}
+
+template <class T>
+void X<T>::foo() requires requires { requires is_not_same_v<T, int>; } {} // ok
+
+template <class T>
+void X<T>::bar(decltype(requires { requires something_interesting<T>; })) {}
+// expected-error at -1{{definition of 'bar' does not match any declaration}}
+
+template <class T>
+void X<T>::bar(decltype(requires { requires is_not_same_v<T, int>; })) {}
} // namespace GH74314
\ No newline at end of file
>From 934d0c586b9653ab02d912b1378967afbf455f7e Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Thu, 21 Dec 2023 17:01:29 +0100
Subject: [PATCH 3/3] add newline
---
clang/test/SemaTemplate/concepts-out-of-line-def.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
index ea21f2720e45af..c4e8e6f720c492 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -535,4 +535,4 @@ void X<T>::bar(decltype(requires { requires something_interesting<T>; })) {}
template <class T>
void X<T>::bar(decltype(requires { requires is_not_same_v<T, int>; })) {}
-} // namespace GH74314
\ No newline at end of file
+} // namespace GH74314
More information about the cfe-commits
mailing list