[clang] [clang][Sema] Avoid guessing unexpanded packs' size in getFullyPackExpandedSize (PR #87768)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

<details>
<summary>Changes</summary>

There has been an optimization for `SizeOfPackExprs` since c5452ed9, in which
we overlooked a case where the template arguments were not yet
formed into a `PackExpansionType` at the token annotation stage. This
led to a problem in that a template involving such expressions may
lose its nature of being dependent, causing some false-positive
diagnostics.

Fixes https://github.com/llvm/llvm-project/issues/84220

---
Full diff: https://github.com/llvm/llvm-project/pull/87768.diff


3 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+1) 
- (modified) clang/lib/Sema/SemaTemplateVariadic.cpp (+11) 
- (modified) clang/test/SemaTemplate/alias-templates.cpp (+23) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 28e8ddb3c41c3e..4501bf3e8fe99f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -504,6 +504,7 @@ Bug Fixes to C++ Support
 - Fix crash when inheriting from a cv-qualified type. Fixes:
   (`#35603 <https://github.com/llvm/llvm-project/issues/35603>`_)
 - Fix a crash when the using enum declaration uses an anonymous enumeration. Fixes (#GH86790).
+- Handled an edge case in ``getFullyPackExpandedSize`` so that we now avoid a false-positive diagnostic. (#GH84220)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 903fbfd18e779c..4909414c0c78d4 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -1243,6 +1243,17 @@ std::optional<unsigned> Sema::getFullyPackExpandedSize(TemplateArgument Arg) {
     // expanded this pack expansion into the enclosing pack if we could.
     if (Elem.isPackExpansion())
       return std::nullopt;
+    // Don't guess the size of unexpanded packs. The pack within a template
+    // argument may have yet to be of a PackExpansion type before we see the
+    // ellipsis in the annotation stage.
+    //
+    // This doesn't mean we would invalidate the optimization: Arg can be an
+    // unexpanded pack regardless of Elem's dependence. For instance,
+    // A TemplateArgument that contains either a SubstTemplateTypeParmPackType
+    // or SubstNonTypeTemplateParmPackExpr is always considered Unexpanded, but
+    // the underlying TemplateArgument thereof may not.
+    if (Elem.containsUnexpandedParameterPack())
+      return std::nullopt;
   }
   return Pack.pack_size();
 }
diff --git a/clang/test/SemaTemplate/alias-templates.cpp b/clang/test/SemaTemplate/alias-templates.cpp
index 8d7cc6118610a0..ab5cad72faf1b7 100644
--- a/clang/test/SemaTemplate/alias-templates.cpp
+++ b/clang/test/SemaTemplate/alias-templates.cpp
@@ -236,6 +236,29 @@ namespace PR14858 {
   void test_q(int (&a)[5]) { Q<B, B, B>().f<B, B>(&a); }
 }
 
+namespace PR84220 {
+
+template <class...> class list {};
+
+template <int> struct foo_impl {
+  template <class> using f = int;
+};
+
+template <class... xs>
+using foo = typename foo_impl<sizeof...(xs)>::template f<xs...>;
+
+// We call getFullyPackExpandedSize at the annotation stage
+// before parsing the ellipsis next to the foo<xs>. This happens before
+// a PackExpansionType is formed for foo<xs>.
+// getFullyPackExpandedSize shouldn't determine the value here. Otherwise,
+// foo_impl<sizeof...(xs)> would lose its dependency despite the template
+// arguments being unsubstituted.
+template <class... xs> using test = list<foo<xs>...>;
+
+test<int> a;
+
+}
+
 namespace redecl {
   template<typename> using A = int;
   template<typename = void> using A = int;

``````````

</details>


https://github.com/llvm/llvm-project/pull/87768