From 342e64979afe0b3859462397c4a8abba6faa9de0 Mon Sep 17 00:00:00 2001 From: Ilya Biryukov Date: Thu, 23 Jun 2022 15:52:16 +0200 Subject: [PATCH] [Sema] Fix assertion failure when instantiating requires expression 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. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D127487 --- clang/lib/Sema/SemaConcept.cpp | 5 ++- clang/lib/Sema/SemaTemplateInstantiate.cpp | 28 +++++++++++++- clang/test/SemaTemplate/concepts-PR54629.cpp | 58 ++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 clang/test/SemaTemplate/concepts-PR54629.cpp diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index c78b0df..239e5dc 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -348,8 +348,9 @@ bool Sema::CheckConstraintSatisfaction(const Expr *ConstraintExpr, ConstraintSatisfaction &Satisfaction) { return calculateConstraintSatisfaction( *this, ConstraintExpr, Satisfaction, - [](const Expr *AtomicExpr) -> ExprResult { - return ExprResult(const_cast(AtomicExpr)); + [this](const Expr *AtomicExpr) -> ExprResult { + // We only do this to immitate lvalue-to-rvalue conversion. + return PerformContextuallyConvertToBool(const_cast(AtomicExpr)); }); } diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 8e59c44..0187cb3 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/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" @@ -2022,6 +2024,7 @@ TemplateInstantiator::TransformNestedRequirement( Req->getConstraintExpr()->getSourceRange()); ExprResult TransConstraint; + ConstraintSatisfaction Satisfaction; TemplateDeductionInfo Info(Req->getConstraintExpr()->getBeginLoc()); { EnterExpressionEvaluationContext ContextRAII( @@ -2033,6 +2036,25 @@ TemplateInstantiator::TransformNestedRequirement( if (ConstrInst.isInvalid()) return nullptr; TransConstraint = TransformExpr(Req->getConstraintExpr()); + if (!TransConstraint.isInvalid()) { + bool CheckSucceeded = + SemaRef.CheckConstraintExpression(TransConstraint.get()); + (void)CheckSucceeded; + assert(CheckSucceeded || 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()) { + bool CheckFailed = SemaRef.CheckConstraintSatisfaction( + TransConstraint.get(), Satisfaction); + (void)CheckFailed; + assert(!CheckFailed || 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) { @@ -2040,7 +2062,11 @@ TemplateInstantiator::TransformNestedRequirement( 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); } diff --git a/clang/test/SemaTemplate/concepts-PR54629.cpp b/clang/test/SemaTemplate/concepts-PR54629.cpp new file mode 100644 index 0000000..cb63c2e --- /dev/null +++ b/clang/test/SemaTemplate/concepts-PR54629.cpp @@ -0,0 +1,58 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s + +template +struct A { + void primary(); +}; + +template + requires requires(T &t) { requires sizeof(t) > 4; } +struct A { + void specialization1(); +}; + +template + requires requires(T &t) { requires sizeof(t) > 8; } +struct A { + void specialization2(); +}; + +int main() { + A().primary(); + A().specialization1(); + A(); // expected-error {{ambiguous partial specialization}} + // expected-note@10 {{partial specialization matches [with T = char[16]}} + // expected-note@16 {{partial specialization matches [with T = char[16]}} +} + +// Check error messages when no overload with constraints matches. +template +void foo() + requires requires(T &t) { requires sizeof(t) < 4; } +{} + +template +void foo() + requires requires(T &t) { requires sizeof(t) > 4; } +{} + +template +void foo() + requires requires(T &t) { requires sizeof(t) > 8; } +{} + +void test() { + foo(); + // expected-error@-1 {{no matching function for call to 'foo'}} + // expected-note@30 {{candidate template ignored: constraints not satisfied}} + // expected-note@31 {{because 'sizeof (t) < 4' (4 < 4) evaluated to false}} + // expected-note@35 {{candidate template ignored: constraints not satisfied}} + // expected-note@36 {{because 'sizeof (t) > 4' (4 > 4) evaluated to false}} + // expected-note@40 {{candidate template ignored: constraints not satisfied}} + // expected-note@41 {{because 'sizeof (t) > 8' (4 > 8) evaluated to false}} + + foo(); + // expected-error@-1 {{call to 'foo' is ambiguous}} + // expected-note@35 {{candidate function}} + // expected-note@40 {{candidate function}} +} -- 2.7.4