[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

[Ilya Biryukov via Phabricator via cfe-commits]

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));
       });
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D127487.435870.patch
Type: text/x-patch
Size: 3743 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220610/2ec53e12/attachment.bin>