[clang] 342e649 - [Sema] Fix assertion failure when instantiating requires expression
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 23 07:20:51 PDT 2022
Author: Ilya Biryukov
Date: 2022-06-23T16:20:30+02:00
New Revision: 342e64979afe0b3859462397c4a8abba6faa9de0
URL: https://github.com/llvm/llvm-project/commit/342e64979afe0b3859462397c4a8abba6faa9de0
DIFF: https://github.com/llvm/llvm-project/commit/342e64979afe0b3859462397c4a8abba6faa9de0.diff
LOG: [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
Added:
clang/test/SemaTemplate/concepts-PR54629.cpp
Modified:
clang/lib/Sema/SemaConcept.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
Removed:
################################################################################
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index c78b0df6ff488..239e5dc4394c3 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<Expr *>(AtomicExpr));
+ [this](const Expr *AtomicExpr) -> ExprResult {
+ // We only do this to immitate lvalue-to-rvalue conversion.
+ return PerformContextuallyConvertToBool(const_cast<Expr *>(AtomicExpr));
});
}
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 8e59c449ae656..0187cb3d12dd8 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 0000000000000..cb63c2e134ed5
--- /dev/null
+++ b/clang/test/SemaTemplate/concepts-PR54629.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+template <class T>
+struct A {
+ void primary();
+};
+
+template <class T>
+ requires requires(T &t) { requires sizeof(t) > 4; }
+struct A<T> {
+ void specialization1();
+};
+
+template <class T>
+ requires requires(T &t) { requires sizeof(t) > 8; }
+struct A<T> {
+ void specialization2();
+};
+
+int main() {
+ A<char>().primary();
+ A<char[5]>().specialization1();
+ A<char[16]>(); // expected-error {{ambiguous partial specialization}}
+ // expected-note at 10 {{partial specialization matches [with T = char[16]}}
+ // expected-note at 16 {{partial specialization matches [with T = char[16]}}
+}
+
+// Check error messages when no overload with constraints matches.
+template <class T>
+void foo()
+ requires requires(T &t) { requires sizeof(t) < 4; }
+{}
+
+template <class T>
+void foo()
+ requires requires(T &t) { requires sizeof(t) > 4; }
+{}
+
+template <class T>
+void foo()
+ requires requires(T &t) { requires sizeof(t) > 8; }
+{}
+
+void test() {
+ foo<char[4]>();
+ // expected-error at -1 {{no matching function for call to 'foo'}}
+ // expected-note at 30 {{candidate template ignored: constraints not satisfied}}
+ // expected-note at 31 {{because 'sizeof (t) < 4' (4 < 4) evaluated to false}}
+ // expected-note at 35 {{candidate template ignored: constraints not satisfied}}
+ // expected-note at 36 {{because 'sizeof (t) > 4' (4 > 4) evaluated to false}}
+ // expected-note at 40 {{candidate template ignored: constraints not satisfied}}
+ // expected-note at 41 {{because 'sizeof (t) > 8' (4 > 8) evaluated to false}}
+
+ foo<char[16]>();
+ // expected-error at -1 {{call to 'foo' is ambiguous}}
+ // expected-note at 35 {{candidate function}}
+ // expected-note at 40 {{candidate function}}
+}
More information about the cfe-commits
mailing list