-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[Clang] Substitute non dependent concepts in constraints #163827
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Clang] Substitute non dependent concepts in constraints #163827
Conversation
This is ``` to form CE, any non-dependent concept template argument Ai is substituted into the constraint-expression of C. If any such substitution results in an invalid concept-id, the program is ill-formed; no diagnostic is required. ``` And continues the implementation of P2841R7 (C++26). No changelog, we will add an entry for P2841R7 closer to the next release, depending on the state of avancement.
@llvm/pr-subscribers-clang Author: Corentin Jabot (cor3ntin) ChangesThis is
https://eel.is/c++draft/temp.constr.normal#1.4 And continues the implementation of P2841R7 (C++26). No changelog, we will add an entry for P2841R7 closer to the next release, depending on the state of avancement. Full diff: https://github.com/llvm/llvm-project/pull/163827.diff 6 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 37598f8530c09..e2444bc03327f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13385,6 +13385,13 @@ class Sema final : public SemaBase {
const MultiLevelTemplateArgumentList &TemplateArgs,
TemplateArgumentListInfo &Outputs);
+ /// Substitute concept template arguments in the constraint expression
+ /// of a concept-id. This is used to implement [temp.constr.normal].
+ ExprResult
+ SubstConceptTemplateArguments(const ConceptSpecializationExpr *CSE,
+ const Expr *ConstraintExpr,
+ const MultiLevelTemplateArgumentList &MLTAL);
+
bool SubstTemplateArgumentsInParameterMapping(
ArrayRef<TemplateArgumentLoc> Args, SourceLocation BaseLoc,
const MultiLevelTemplateArgumentList &TemplateArgs,
diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp
index 76f96fb8c5dcc..131ae6e8a478f 100644
--- a/clang/lib/AST/TemplateBase.cpp
+++ b/clang/lib/AST/TemplateBase.cpp
@@ -340,13 +340,14 @@ bool TemplateArgument::isPackExpansion() const {
}
bool TemplateArgument::isConceptOrConceptTemplateParameter() const {
- if (getKind() == TemplateArgument::Template) {
- if (isa<ConceptDecl>(getAsTemplate().getAsTemplateDecl()))
- return true;
- else if (auto *TTP = dyn_cast_if_present<TemplateTemplateParmDecl>(
- getAsTemplate().getAsTemplateDecl()))
- return TTP->templateParameterKind() == TNK_Concept_template;
- }
+ if (getKind() != TemplateArgument::Template)
+ return false;
+
+ if (isa_and_nonnull<ConceptDecl>(getAsTemplate().getAsTemplateDecl()))
+ return true;
+ if (auto *TTP = llvm::dyn_cast_or_null<TemplateTemplateParmDecl>(
+ getAsTemplate().getAsTemplateDecl()))
+ return TTP->templateParameterKind() == TNK_Concept_template;
return false;
}
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 87dd68269d44a..d352e5974f772 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -1217,10 +1217,48 @@ bool Sema::CheckConstraintSatisfaction(
return false;
}
+static const ExprResult
+SubstituteConceptsInConstrainExpression(Sema &S, const NamedDecl *D,
+ const ConceptSpecializationExpr *CSE,
+ UnsignedOrNone SubstIndex) {
+
+ // [C++2c] [temp.constr.normal]
+ // Otherwise, to form CE, any non-dependent concept template argument Ai
+ // is substituted into the constraint-expression of C.
+ // If any such substitution results in an invalid concept-id,
+ // the program is ill-formed; no diagnostic is required.
+
+ ConceptDecl *Concept = CSE->getNamedConcept()->getCanonicalDecl();
+ Sema::ArgPackSubstIndexRAII _(S, SubstIndex);
+
+ const auto *ArgsAsWritten = CSE->getTemplateArgsAsWritten();
+ if (llvm::none_of(
+ ArgsAsWritten->arguments(), [&](const TemplateArgumentLoc &ArgLoc) {
+ return !ArgLoc.getArgument().isDependent() &&
+ ArgLoc.getArgument().isConceptOrConceptTemplateParameter();
+ })) {
+ return Concept->getConstraintExpr();
+ }
+
+ MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
+ Concept, Concept->getLexicalDeclContext(),
+ /*Final=*/false, CSE->getTemplateArguments(),
+ /*RelativeToPrimary=*/true,
+ /*Pattern=*/nullptr,
+ /*ForConstraintInstantiation=*/true);
+ return S.SubstConceptTemplateArguments(CSE, Concept->getConstraintExpr(),
+ MLTAL);
+}
+
bool Sema::CheckConstraintSatisfaction(
const ConceptSpecializationExpr *ConstraintExpr,
ConstraintSatisfaction &Satisfaction) {
+ ExprResult Res = SubstituteConceptsInConstrainExpression(
+ *this, nullptr, ConstraintExpr, ArgPackSubstIndex);
+ if (!Res.isUsable())
+ return true;
+
llvm::SmallVector<AssociatedConstraint, 1> Constraints;
Constraints.emplace_back(
ConstraintExpr->getNamedConcept()->getConstraintExpr());
@@ -2249,8 +2287,14 @@ NormalizedConstraint *NormalizedConstraint::fromConstraintExpr(
// Use canonical declarations to merge ConceptDecls across
// different modules.
ConceptDecl *CD = CSE->getNamedConcept()->getCanonicalDecl();
+
+ ExprResult Res =
+ SubstituteConceptsInConstrainExpression(S, D, CSE, SubstIndex);
+ if (!Res.isUsable())
+ return nullptr;
+
SubNF = NormalizedConstraint::fromAssociatedConstraints(
- S, CD, AssociatedConstraint(CD->getConstraintExpr(), SubstIndex));
+ S, CD, AssociatedConstraint(Res.get(), SubstIndex));
if (!SubNF)
return nullptr;
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index bec282011b3fa..29e6b276d43c8 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -4487,6 +4487,109 @@ ExprResult Sema::SubstConstraintExprWithoutSatisfaction(
return Instantiator.TransformExpr(E);
}
+ExprResult Sema::SubstConceptTemplateArguments(
+ const ConceptSpecializationExpr *CSE, const Expr *ConstraintExpr,
+ const MultiLevelTemplateArgumentList &MLTAL) {
+ TemplateInstantiator Instantiator(*this, MLTAL, SourceLocation(),
+ DeclarationName());
+ auto *ArgsAsWritten = CSE->getTemplateArgsAsWritten();
+ TemplateArgumentListInfo SubstArgs(ArgsAsWritten->getLAngleLoc(),
+ ArgsAsWritten->getRAngleLoc());
+
+ Sema::InstantiatingTemplate Inst(
+ *this, ArgsAsWritten->arguments().front().getSourceRange().getBegin(),
+ Sema::InstantiatingTemplate::ConstraintNormalization{},
+ CSE->getNamedConcept(),
+ ArgsAsWritten->arguments().front().getSourceRange());
+
+ if (Instantiator.TransformConceptTemplateArguments(
+ ArgsAsWritten->getTemplateArgs(),
+ ArgsAsWritten->getTemplateArgs() +
+ ArgsAsWritten->getNumTemplateArgs(),
+ SubstArgs))
+ return true;
+
+ llvm::SmallVector<TemplateArgument, 4> NewArgList;
+ NewArgList.reserve(SubstArgs.arguments().size());
+ for (const auto &ArgLoc : SubstArgs.arguments())
+ NewArgList.push_back(ArgLoc.getArgument());
+
+ MultiLevelTemplateArgumentList MLTALForConstraint =
+ getTemplateInstantiationArgs(
+ CSE->getNamedConcept(),
+ CSE->getNamedConcept()->getLexicalDeclContext(),
+ /*Final=*/false,
+ /*Innermost=*/NewArgList,
+ /*RelativeToPrimary=*/true,
+ /*Pattern=*/nullptr,
+ /*ForConstraintInstantiation=*/true);
+
+ struct ConstraintExprTransformer : TreeTransform<ConstraintExprTransformer> {
+ using Base = TreeTransform<ConstraintExprTransformer>;
+ MultiLevelTemplateArgumentList &MLTAL;
+
+ ConstraintExprTransformer(Sema &SemaRef,
+ MultiLevelTemplateArgumentList &MLTAL)
+ : TreeTransform(SemaRef), MLTAL(MLTAL) {}
+
+ ExprResult TransformExpr(Expr *E) {
+ if (!E)
+ return E;
+ switch (E->getStmtClass()) {
+ case Stmt::BinaryOperatorClass:
+ case Stmt::ConceptSpecializationExprClass:
+ case Stmt::ParenExprClass:
+ case Stmt::UnresolvedLookupExprClass:
+ return Base::TransformExpr(E);
+ default:
+ break;
+ }
+ return E;
+ }
+
+ ExprResult TransformBinaryOperator(BinaryOperator *E) {
+ if (!(E->getOpcode() == BinaryOperatorKind::BO_LAnd ||
+ E->getOpcode() == BinaryOperatorKind::BO_LOr))
+ return E;
+
+ ExprResult LHS = TransformExpr(E->getLHS());
+ ExprResult RHS = TransformExpr(E->getRHS());
+
+ if (LHS.get() == E->getLHS() && RHS.get() == E->getRHS())
+ return E;
+
+ return BinaryOperator::Create(SemaRef.Context, LHS.get(), RHS.get(),
+ E->getOpcode(), SemaRef.Context.BoolTy,
+ VK_PRValue, OK_Ordinary,
+ E->getOperatorLoc(), FPOptionsOverride{});
+ }
+
+ bool TransformTemplateArgument(const TemplateArgumentLoc &Input,
+ TemplateArgumentLoc &Output,
+ bool Uneval = false) {
+ if (Input.getArgument().isConceptOrConceptTemplateParameter())
+ return Base::TransformTemplateArgument(Input, Output, Uneval);
+
+ Output = Input;
+ return false;
+ }
+
+ ExprResult TransformUnresolvedLookupExpr(UnresolvedLookupExpr *E,
+ bool IsAddressOfOperand = false) {
+ if (E->isConceptReference()) {
+ ExprResult Res = SemaRef.SubstExpr(E, MLTAL);
+ return Res;
+ }
+ return E;
+ }
+ };
+
+ ConstraintExprTransformer Transformer(*this, MLTALForConstraint);
+ ExprResult Res =
+ Transformer.TransformExpr(const_cast<Expr *>(ConstraintExpr));
+ return Res;
+}
+
ExprResult Sema::SubstInitializer(Expr *Init,
const MultiLevelTemplateArgumentList &TemplateArgs,
bool CXXDirectInit) {
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 86896abc1f775..97c6f7066cff9 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -694,6 +694,12 @@ class TreeTransform {
TemplateArgumentListInfo &Outputs,
bool Uneval = false);
+ template <typename InputIterator>
+ bool TransformConceptTemplateArguments(InputIterator First,
+ InputIterator Last,
+ TemplateArgumentListInfo &Outputs,
+ bool Uneval = false);
+
/// Checks if the argument pack from \p In will need to be expanded and does
/// the necessary prework.
/// Whether the expansion is needed is captured in Info.Expand.
@@ -5192,6 +5198,46 @@ bool TreeTransform<Derived>::TransformTemplateArguments(
return false;
}
+template <typename Derived>
+template <typename InputIterator>
+bool TreeTransform<Derived>::TransformConceptTemplateArguments(
+ InputIterator First, InputIterator Last, TemplateArgumentListInfo &Outputs,
+ bool Uneval) {
+
+ auto isNonDependentConcept = [](const TemplateArgument &Arg) {
+ return !Arg.isDependent() && Arg.isConceptOrConceptTemplateParameter();
+ };
+
+ for (; First != Last; ++First) {
+ TemplateArgumentLoc Out;
+ TemplateArgumentLoc In = *First;
+
+ if (In.getArgument().getKind() == TemplateArgument::Pack) {
+ typedef TemplateArgumentLocInventIterator<Derived,
+ TemplateArgument::pack_iterator>
+ PackLocIterator;
+ if (TransformConceptTemplateArguments(
+ PackLocIterator(*this, In.getArgument().pack_begin()),
+ PackLocIterator(*this, In.getArgument().pack_end()), Outputs,
+ Uneval))
+ return true;
+ continue;
+ }
+
+ if (!isNonDependentConcept(In.getArgument())) {
+ Outputs.addArgument(In);
+ continue;
+ }
+
+ if (getDerived().TransformTemplateArgument(In, Out, Uneval))
+ return true;
+
+ Outputs.addArgument(Out);
+ }
+
+ return false;
+}
+
// FIXME: Find ways to reduce code duplication for pack expansions.
template <typename Derived>
bool TreeTransform<Derived>::PreparePackForExpansion(TemplateArgumentLoc In,
diff --git a/clang/test/SemaCXX/cxx2c-template-template-param.cpp b/clang/test/SemaCXX/cxx2c-template-template-param.cpp
index 4ad3fd95039cd..77f872d31d923 100644
--- a/clang/test/SemaCXX/cxx2c-template-template-param.cpp
+++ b/clang/test/SemaCXX/cxx2c-template-template-param.cpp
@@ -350,3 +350,87 @@ template <A<concept missing<int>> T> // expected-error {{expected expression}} \
// expected-error {{expected unqualified-id}}
auto f();
}
+
+namespace concept_arg_normalization {
+
+template <typename T,
+ template <typename...> concept C1>
+concept one = (C1<T>); // #concept-arg-one
+
+template <typename T>
+concept A = true; // #concept-arg-A
+
+template <typename T>
+concept BetterA = A<T> && true;
+
+template <typename T>
+concept B = true; // #concept-arg-B
+
+template <typename T>
+concept False = false; // #concept-arg-False
+
+template <typename T>
+requires one<T, A>
+void f1(T){} // #concept-arg-f1-1
+
+template <typename T>
+requires one<T, B>
+void f1(T){} // #concept-arg-f1-2
+
+template <typename T>
+requires one<T, A>
+void f2(T){}
+
+template <typename T>
+requires one<T, BetterA>
+void f2(T){}
+
+
+template <template <typename> concept CT>
+requires one<int, A>
+void f3(){} // #concept-arg-f3-1
+
+template <template <typename> concept CT>
+requires one<int, CT>
+void f3(){} // #concept-arg-f3-2
+
+template <typename T>
+requires one<T, False> void f4(T){} // #concept-arg-f4
+
+
+void test() {
+ f1(0);
+ // expected-error@-1 {{call to 'f1' is ambiguous}}
+ // expected-note@#concept-arg-f1-1{{candidate function [with T = int]}}
+ // expected-note@#concept-arg-f1-2{{candidate function [with T = int]}}
+ // expected-note@#concept-arg-A {{similar constraint expressions not considered equivalent}}
+ // expected-note@#concept-arg-B {{similar constraint expression here}}
+ f2(0);
+
+ f3<BetterA>();
+ // expected-error@-1 {{call to 'f3' is ambiguous}}
+ // expected-note@#concept-arg-f3-1 {{candidate function [with CT = concept_arg_normalization::BetterA]}}
+ // expected-note@#concept-arg-f3-2 {{candidate function [with CT = concept_arg_normalization::BetterA]}}
+
+static_assert(one<int, A>);
+static_assert(one<int, False>);
+// expected-error@-1 {{static assertion failed}} \
+// expected-note@-1 {{because 'one<int, False>' evaluated to false}}
+// expected-note@#concept-arg-one {{because 'int' does not satisfy 'False'}}
+// expected-note@#concept-arg-False {{because 'false' evaluated to false}}
+
+f4(0);
+// expected-error@-1 {{no matching function for call to 'f4'}}
+// expected-note@#concept-arg-f4 {{candidate template ignored: constraints not satisfied [with T = int]}}
+// expected-note@#concept-arg-f4 {{because 'one<int, False>'}}
+// expected-note@#concept-arg-one {{because 'int' does not satisfy 'False'}}
+// expected-note@#concept-arg-False {{because 'false' evaluated to false}}
+
+
+template <typename T, template <typename...> concept C1>
+concept TestBinary = T::a || C1<T>;
+static_assert(TestBinary<int, A>);
+
+}
+
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple nits, else LGTM.
const ConceptSpecializationExpr *ConstraintExpr, | ||
ConstraintSatisfaction &Satisfaction) { | ||
|
||
ExprResult Res = SubstituteConceptsInConstrainExpression( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be doing something with Res
? Is this expression used anywhere now?
const MultiLevelTemplateArgumentList &MLTAL) { | ||
TemplateInstantiator Instantiator(*this, MLTAL, SourceLocation(), | ||
DeclarationName()); | ||
auto *ArgsAsWritten = CSE->getTemplateArgsAsWritten(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here too.
bool Uneval = false); | ||
|
||
template <typename InputIterator> | ||
bool TransformConceptTemplateArguments(InputIterator First, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we take a range instead of iterators?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather we stay consistent with the other TransformTemplateArguments functions - we could do a subsequent cleanup
/*Pattern=*/nullptr, | ||
/*ForConstraintInstantiation=*/true); | ||
|
||
struct ConstraintExprTransformer : TreeTransform<ConstraintExprTransformer> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have an explanation as to why we need a TreeTransform here, and it would be better to have an example too.
ExprResult TransformUnresolvedLookupExpr(UnresolvedLookupExpr *E, | ||
bool IsAddressOfOperand = false) { | ||
if (E->isConceptReference()) { | ||
ExprResult Res = SemaRef.SubstExpr(E, MLTAL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to continue transform of substituted expressions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't imagine a scenario?
|
||
ExprResult Res = | ||
SubstituteConceptsInConstrainExpression(S, D, CSE, SubstIndex); | ||
if (!Res.isUsable()) | ||
return nullptr; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that a substitution in this function will risk resulting in an exponential time complexity, unless we can ensure that the substitution in SubstituteConceptsInConstrainExpression is not deeply recursive. (i.e. at most 1 level substitution)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup - but we only do that if we find concepts template arguments, so I am not concerned
return false; | ||
} | ||
|
||
template <typename Derived> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about TransformNonDependentTemplateArguments? (we can have a default IsNonDependentConcept and a client defined implementation)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal is to only transform Arguments that are concept
SomeConcept<SomeType, SomeValue, SomeConceptName>
^- only transform that
Everything else goes into the mapping instead
bool Uneval) { | ||
|
||
auto isNonDependentConcept = [](const TemplateArgument &Arg) { | ||
return !Arg.isDependent() && Arg.isConceptOrConceptTemplateParameter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit confusing with the name non-dependent concept...
Can we reference something from standard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to isNonDependentConceptArgument and added standard quote https://eel.is/c++draft/temp.constr.normal#1.4.2.sentence-1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/17645 Here is the relevant piece of the build log for the reference
|
This is
https://eel.is/c++draft/temp.constr.normal#1.4
And continues the implementation of P2841R7 (C++26).
No changelog, we will add an entry for P2841R7 closer to the next release, depending on the state of avancement.