ilya-biryukov created this revision. ilya-biryukov added reviewers: erichkeane, saar.raz. Herald added a project: All. ilya-biryukov requested review of this revision. Herald added a project: clang.
Fixes #54629. The crash is is caused by the double template instantiation. See the added test. Here is what happens: - Template arguments for the partial specialization get instantiated. - This causes instantitation into the corrensponding requires expression. - `TemplateInsantiator` correctly handles instantiation of parameters inside `RequiresExprBody` and instantiates the constraint expression inside the `NestedRequirement`. - To build the substituted `NestedRequirement`, `TemplateInsantiator` calls `Sema::BuildNestedRequirement` calls `CheckConstraintSatisfaction`, which results in another template instantiation (with empty template arguments). This seem to be an implementation detail to handle constraint satisfaction and is not required by the standard. - The recursive template instantiation tries to find the parameter inside `RequiresExprBody` and fails with the corresponding assertion. Note that this only happens as both instantiations happen with the class partial template specialization set as `Sema.CurContext`, which is considered a dependent `DeclContext`. To fix the assertion, avoid doing the recursive template instantiation and instead evaluate resulting expressions in-place. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D127487 Files: clang/lib/Sema/SemaConcept.cpp clang/lib/Sema/SemaTemplateInstantiate.cpp clang/test/SemaTemplate/concepts-PR54629.cpp Index: clang/test/SemaTemplate/concepts-PR54629.cpp =================================================================== --- /dev/null +++ clang/test/SemaTemplate/concepts-PR54629.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s +// expected-no-diagnostics +template <class T> +struct A{}; + +template <class T> requires requires (T t) { requires sizeof(t) > 4; } +struct A<T> {}; + +int main() { + A<int> a; +} + Index: clang/lib/Sema/SemaTemplateInstantiate.cpp =================================================================== --- clang/lib/Sema/SemaTemplateInstantiate.cpp +++ clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -10,12 +10,14 @@ //===----------------------------------------------------------------------===/ #include "TreeTransform.h" +#include "clang/AST/ASTConcept.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ASTLambda.h" #include "clang/AST/ASTMutationListener.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" +#include "clang/AST/ExprConcepts.h" #include "clang/AST/PrettyDeclStackTrace.h" #include "clang/AST/TypeVisitor.h" #include "clang/Basic/LangOptions.h" @@ -2023,6 +2025,7 @@ Req->getConstraintExpr()->getSourceRange()); ExprResult TransConstraint; + ConstraintSatisfaction Satisfaction; TemplateDeductionInfo Info(Req->getConstraintExpr()->getBeginLoc()); { EnterExpressionEvaluationContext ContextRAII( @@ -2034,6 +2037,21 @@ if (ConstrInst.isInvalid()) return nullptr; TransConstraint = TransformExpr(Req->getConstraintExpr()); + if (!TransConstraint.isInvalid() && + !SemaRef.CheckConstraintExpression(TransConstraint.get())) { + assert(Trap.hasErrorOccurred() && "CheckConstraintExpression failed, but " + "did not produce a SFINAE error"); + } + // Use version of CheckConstraintSatisfaction that does no substitutions. + if (!TransConstraint.isInvalid() && + !TransConstraint.get()->isInstantiationDependent() && + !Trap.hasErrorOccurred()) { + if (SemaRef.CheckConstraintSatisfaction(TransConstraint.get(), + Satisfaction)) { + assert(Trap.hasErrorOccurred() && "CheckConstraintSatisfaction failed, " + "but did not produce a SFINAE error"); + } + } if (TransConstraint.isInvalid() || Trap.hasErrorOccurred()) return RebuildNestedRequirement(createSubstDiag(SemaRef, Info, [&] (llvm::raw_ostream& OS) { @@ -2041,7 +2059,11 @@ SemaRef.getPrintingPolicy()); })); } - return RebuildNestedRequirement(TransConstraint.get()); + if (TransConstraint.get()->isInstantiationDependent()) + return new (SemaRef.Context) + concepts::NestedRequirement(TransConstraint.get()); + return new (SemaRef.Context) concepts::NestedRequirement( + SemaRef.Context, TransConstraint.get(), Satisfaction); } Index: clang/lib/Sema/SemaConcept.cpp =================================================================== --- clang/lib/Sema/SemaConcept.cpp +++ clang/lib/Sema/SemaConcept.cpp @@ -348,8 +348,9 @@ ConstraintSatisfaction &Satisfaction) { return calculateConstraintSatisfaction( *this, ConstraintExpr, Satisfaction, - [](const Expr *AtomicExpr) -> ExprResult { - return ExprResult(const_cast<Expr *>(AtomicExpr)); + [this](const Expr *AtomicExpr) -> ExprResult { + // We only do this to immitate lvalue-to-rvalue conversion. + return PerformContextuallyConvertToBool(const_cast<Expr*>(AtomicExpr)); }); }
Index: clang/test/SemaTemplate/concepts-PR54629.cpp =================================================================== --- /dev/null +++ clang/test/SemaTemplate/concepts-PR54629.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s +// expected-no-diagnostics +template <class T> +struct A{}; + +template <class T> requires requires (T t) { requires sizeof(t) > 4; } +struct A<T> {}; + +int main() { + A<int> a; +} + Index: clang/lib/Sema/SemaTemplateInstantiate.cpp =================================================================== --- clang/lib/Sema/SemaTemplateInstantiate.cpp +++ clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -10,12 +10,14 @@ //===----------------------------------------------------------------------===/ #include "TreeTransform.h" +#include "clang/AST/ASTConcept.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ASTLambda.h" #include "clang/AST/ASTMutationListener.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" +#include "clang/AST/ExprConcepts.h" #include "clang/AST/PrettyDeclStackTrace.h" #include "clang/AST/TypeVisitor.h" #include "clang/Basic/LangOptions.h" @@ -2023,6 +2025,7 @@ Req->getConstraintExpr()->getSourceRange()); ExprResult TransConstraint; + ConstraintSatisfaction Satisfaction; TemplateDeductionInfo Info(Req->getConstraintExpr()->getBeginLoc()); { EnterExpressionEvaluationContext ContextRAII( @@ -2034,6 +2037,21 @@ if (ConstrInst.isInvalid()) return nullptr; TransConstraint = TransformExpr(Req->getConstraintExpr()); + if (!TransConstraint.isInvalid() && + !SemaRef.CheckConstraintExpression(TransConstraint.get())) { + assert(Trap.hasErrorOccurred() && "CheckConstraintExpression failed, but " + "did not produce a SFINAE error"); + } + // Use version of CheckConstraintSatisfaction that does no substitutions. + if (!TransConstraint.isInvalid() && + !TransConstraint.get()->isInstantiationDependent() && + !Trap.hasErrorOccurred()) { + if (SemaRef.CheckConstraintSatisfaction(TransConstraint.get(), + Satisfaction)) { + assert(Trap.hasErrorOccurred() && "CheckConstraintSatisfaction failed, " + "but did not produce a SFINAE error"); + } + } if (TransConstraint.isInvalid() || Trap.hasErrorOccurred()) return RebuildNestedRequirement(createSubstDiag(SemaRef, Info, [&] (llvm::raw_ostream& OS) { @@ -2041,7 +2059,11 @@ SemaRef.getPrintingPolicy()); })); } - return RebuildNestedRequirement(TransConstraint.get()); + if (TransConstraint.get()->isInstantiationDependent()) + return new (SemaRef.Context) + concepts::NestedRequirement(TransConstraint.get()); + return new (SemaRef.Context) concepts::NestedRequirement( + SemaRef.Context, TransConstraint.get(), Satisfaction); } Index: clang/lib/Sema/SemaConcept.cpp =================================================================== --- clang/lib/Sema/SemaConcept.cpp +++ clang/lib/Sema/SemaConcept.cpp @@ -348,8 +348,9 @@ ConstraintSatisfaction &Satisfaction) { return calculateConstraintSatisfaction( *this, ConstraintExpr, Satisfaction, - [](const Expr *AtomicExpr) -> ExprResult { - return ExprResult(const_cast<Expr *>(AtomicExpr)); + [this](const Expr *AtomicExpr) -> ExprResult { + // We only do this to immitate lvalue-to-rvalue conversion. + return PerformContextuallyConvertToBool(const_cast<Expr*>(AtomicExpr)); }); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits