[clang] [Clang][Sema] Extract ellipsis location from CXXFoldExpr for reattaching constraints on NTTPs (PR #78080)

Younan Zhang via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 14 00:45:17 PST 2024


https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/78080

>From 7c0b5ffe4b377f198308b976eb726138837145c4 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sun, 14 Jan 2024 12:21:33 +0800
Subject: [PATCH 1/2] [Clang][Sema] Extract ellipsis location from CXXFoldExpr
 for reattaching constraints on NTTPs

We build up a CXXFoldExpr for immediately declared constraints as
per C++20 [temp.param]/4. This is done by
`formImmediatelyDeclaredConstraint` where an EllipsisLoc is essential
to determine whether this is a pack.

On the other hand, when attempting to instantiate a class template,
member templates might not be instantiated immediately, so we leave
them intact. For function templates with NTTPs,  we reattach
constraints if possible so that they can be evaluated later. To
properly form that, we attempted to extract an ellipsis location if
the param per se was a parameter pack. Unfortunately, for the following
NTTP case, we seemingly failed to handle:

```cpp
template <Constraint auto... Pack>
void member();
```

The NTTPD Pack is neither an ExpandedParameterPack nor a PackExpansion
(its type does not expand anything). As a result, we end up losing
track of the constraints on packs, although we have them inside the
associated CXXFoldExpr.

This patch fixes that by extracting the ellipsis location out of the
previous constraint expression. This closes
https://github.com/llvm/llvm-project/issues/63837.
---
 clang/docs/ReleaseNotes.rst                   |  3 +++
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  | 21 ++++++++++++-------
 clang/test/SemaTemplate/concepts.cpp          | 21 +++++++++++++++++++
 3 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 154412144ef97a..04b2fac2c74713 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -857,6 +857,9 @@ Bug Fixes to C++ Support
   (`#64607 <https://github.com/llvm/llvm-project/issues/64607>`_)
   (`#64086 <https://github.com/llvm/llvm-project/issues/64086>`_)
 
+- Fixed a crash where we lost uninstantiated constraints on placeholder NTTP packs. Fixes:
+  (`#63837 <https://github.com/llvm/llvm-project/issues/63837>`_)
+
 - Fixed a regression where clang forgets how to substitute into constraints on template-template
   parameters. Fixes: 
   (`#57410 <https://github.com/llvm/llvm-project/issues/57410>`_) and
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index d768bb72e07c09..208a7b120c419b 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3056,16 +3056,21 @@ Decl *TemplateDeclInstantiator::VisitNonTypeTemplateParmDecl(
         D->getPosition(), D->getIdentifier(), T, D->isParameterPack(), DI);
 
   if (AutoTypeLoc AutoLoc = DI->getTypeLoc().getContainedAutoTypeLoc())
-    if (AutoLoc.isConstrained())
+    if (AutoLoc.isConstrained()) {
+      SourceLocation EllipsisLoc;
+      if (IsExpandedParameterPack)
+        EllipsisLoc =
+            DI->getTypeLoc().getAs<PackExpansionTypeLoc>().getEllipsisLoc();
+      else if (auto *Constraint = dyn_cast_if_present<CXXFoldExpr>(
+                   D->getPlaceholderTypeConstraint()))
+        EllipsisLoc = Constraint->getEllipsisLoc();
       // Note: We attach the uninstantiated constriant here, so that it can be
-      // instantiated relative to the top level, like all our other constraints.
-      if (SemaRef.AttachTypeConstraint(
-              AutoLoc, Param, D,
-              IsExpandedParameterPack
-                ? DI->getTypeLoc().getAs<PackExpansionTypeLoc>()
-                    .getEllipsisLoc()
-                : SourceLocation()))
+      // instantiated relative to the top level, like all our other
+      // constraints.
+      if (SemaRef.AttachTypeConstraint(AutoLoc, /*NewConstrainedParm=*/Param,
+                                       /*OrigConstrainedParm=*/D, EllipsisLoc))
         Invalid = true;
+    }
 
   Param->setAccess(AS_public);
   Param->setImplicit(D->isImplicit());
diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp
index e98ebcc9203a43..bac209a28da912 100644
--- a/clang/test/SemaTemplate/concepts.cpp
+++ b/clang/test/SemaTemplate/concepts.cpp
@@ -1064,3 +1064,24 @@ void cand(T t)
 
 void test() { cand(42); }
 }
+
+namespace GH63837 {
+
+template<class> concept IsFoo = true;
+
+template<class> struct Struct {
+  template<IsFoo auto... xs>
+  void foo() {}
+
+  template<auto... xs> requires (... && IsFoo<decltype(xs)>)
+  void bar() {}
+
+  template<IsFoo auto... xs>
+  static inline int field = 0;
+};
+
+template void Struct<void>::foo<>();
+template void Struct<void>::bar<>();
+template int Struct<void>::field<1, 2>;
+
+}

>From 5f1d090f414bcb42a931fde557844ddfd37c375d Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sun, 14 Jan 2024 16:44:41 +0800
Subject: [PATCH 2/2] Remove trailing whitespaces

---
 clang/docs/ReleaseNotes.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 04b2fac2c74713..3b7868e7403815 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -861,7 +861,7 @@ Bug Fixes to C++ Support
   (`#63837 <https://github.com/llvm/llvm-project/issues/63837>`_)
 
 - Fixed a regression where clang forgets how to substitute into constraints on template-template
-  parameters. Fixes: 
+  parameters. Fixes:
   (`#57410 <https://github.com/llvm/llvm-project/issues/57410>`_) and
   (`#76604 <https://github.com/llvm/llvm-project/issues/57410>`_)
 



More information about the cfe-commits mailing list