[clang] 849d1b8 - [clang] Do not substitute parameter pack while retaining the pack expansion (#108197)

[via cfe-commits]

Author: Utkarsh Saxena
Date: 2024-09-12T13:21:26+02:00
New Revision: 849d1b8b1f1fc16dc28b07da358515a52b79ea81

URL: https://github.com/llvm/llvm-project/commit/849d1b8b1f1fc16dc28b07da358515a52b79ea81
DIFF: https://github.com/llvm/llvm-project/commit/849d1b8b1f1fc16dc28b07da358515a52b79ea81.diff

LOG: [clang] Do not substitute parameter pack while retaining the pack expansion (#108197)

(In reference to https://github.com/llvm/llvm-project/pull/108197/commits/5901d82ea0543074853b963f7dc9106a6fe3bcee)
Consider when `Input[I]` is a `VarDecl` with parameter pack. We would
have already expanded the pack before the code change in the loop`for
(unsigned I = 0; I != *NumExpansions; ++I) {`.

Now in `if (RetainExpansion) {`, without this change, we continue to
substitute the pack in the pattern even when we do not have meaningful
`ArgumentPackSubstitutionIndex` set.

This leads to use of an invalid pack substitution index in
`TemplateInstantiator::TransformFunctionParmPackRefExpr` in
`TransformedDecl = (*Pack)[getSema().ArgumentPackSubstitutionIndex];`

This change sets `ArgumentPackSubstitutionIndex` to `-1` while retaining
expansion to instruct `TransformFunctionParmPackRefExpr` to build
`FunctionParmPackExpr` instead of substituting the param pack.

---

There are other instances of `RetainExpansion` and IIUC, they should
also unset the `ArgumentPackSubstitutionIndex`. It would be great if
someone can verify my understanding. If this is correct then we could
instead have a `ArgumentPackSubstitutionIndexRAII` as part of
`ForgetPartiallySubstitutedPackRAII`.

EDIT: I have moved this to `ForgetPartiallySubstitutedPackRAII`.

Fixes https://github.com/llvm/llvm-project/issues/63819
Fixes https://github.com/llvm/llvm-project/issues/107560

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/TreeTransform.h
    clang/test/SemaTemplate/pack-deduction.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d4db877a823ea5..c4fa017b982bbd 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -382,6 +382,9 @@ Bug Fixes to C++ Support
 - Fix a crash when using ``source_location`` in the trailing return type of a lambda expression. (#GH67134)
 - A follow-up fix was added for (#GH61460), as the previous fix was not entirely correct. (#GH86361)
 - Fixed a crash in the typo correction of an invalid CTAD guide. (#GH107887)
+- Fixed a crash when clang tries to subtitute parameter pack while retaining the parameter
+  pack. #GH63819, #GH107560
+
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 4bbc024587915c..ff745b3385fcd9 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -113,9 +113,13 @@ class TreeTransform {
   class ForgetPartiallySubstitutedPackRAII {
     Derived &Self;
     TemplateArgument Old;
+    // Set the pack expansion index to -1 to avoid pack substitution and
+    // indicate that parameter packs should be instantiated as themselves.
+    Sema::ArgumentPackSubstitutionIndexRAII ResetPackSubstIndex;
 
   public:
-    ForgetPartiallySubstitutedPackRAII(Derived &Self) : Self(Self) {
+    ForgetPartiallySubstitutedPackRAII(Derived &Self)
+        : Self(Self), ResetPackSubstIndex(Self.getSema(), -1) {
       Old = Self.ForgetPartiallySubstitutedPack();
     }
 

diff  --git a/clang/test/SemaTemplate/pack-deduction.cpp b/clang/test/SemaTemplate/pack-deduction.cpp
index e42709820e9cff..28fb127a386441 100644
--- a/clang/test/SemaTemplate/pack-deduction.cpp
+++ b/clang/test/SemaTemplate/pack-deduction.cpp
@@ -185,3 +185,17 @@ void Run() {
   Outer<void>::Inner<0>().Test(1,1);
 }
 }
+
+namespace GH107560 {
+int bar(...);
+
+template <int> struct Int {};
+
+template <class ...T>
+constexpr auto foo(T... x) -> decltype(bar(T(x)...)) { return 10; }
+
+template <class ...T>
+constexpr auto baz(Int<foo<T>(T())>... x) -> int { return 1; }
+
+static_assert(baz<Int<1>, Int<2>, Int<3>>(Int<10>(), Int<10>(), Int<10>()) == 1, "");
+}