-
Notifications
You must be signed in to change notification settings - Fork 15k
[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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we be doing something with |
||
| *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; | ||
|
|
||
|
Comment on lines
+2290
to
+2295
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 |
||
| SubNF = NormalizedConstraint::fromAssociatedConstraints( | ||
| S, CD, AssociatedConstraint(CD->getConstraintExpr(), SubstIndex)); | ||
| S, CD, AssociatedConstraint(Res.get(), SubstIndex)); | ||
|
|
||
| if (!SubNF) | ||
| return nullptr; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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()); | ||
cor3ntin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| 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()); | ||
cor3ntin marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| MultiLevelTemplateArgumentList MLTALForConstraint = | ||
| getTemplateInstantiationArgs( | ||
| CSE->getNamedConcept(), | ||
| CSE->getNamedConcept()->getLexicalDeclContext(), | ||
| /*Final=*/false, | ||
| /*Innermost=*/NewArgList, | ||
| /*RelativeToPrimary=*/true, | ||
| /*Pattern=*/nullptr, | ||
| /*ForConstraintInstantiation=*/true); | ||
|
|
||
| struct ConstraintExprTransformer : TreeTransform<ConstraintExprTransformer> { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| 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); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. I can't imagine a scenario? |
||
| 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) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -694,6 +694,12 @@ class TreeTransform { | |
| TemplateArgumentListInfo &Outputs, | ||
| bool Uneval = false); | ||
|
|
||
| template <typename InputIterator> | ||
| bool TransformConceptTemplateArguments(InputIterator First, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 |
||
| 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> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. The goal is to only transform Arguments that are concept Everything else goes into the mapping instead |
||
| 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(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 |
||
| }; | ||
|
|
||
| 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, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.