[Clang] Correctly propagate type aliases' unexpanded flags up to lambda

[zyn0217]

We should have been checking desugar() for the type of the right-hand side of a typedef declaration, instead of using getCanonicalType(), which points to the end of the type alias chain.

Fixes #122417

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

We should have been checking desugar() for the type of the right-hand side of a typedef declaration, instead of using getCanonicalType(), which points to the end of the type alias chain.

Fixes #122417

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1-1)
• (modified) clang/lib/Sema/TreeTransform.h (+1-1)
• (modified) clang/test/SemaCXX/fold_lambda_with_variadics.cpp (+6)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 07a1a4195427d8..a5da6c9c88328c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -810,7 +810,7 @@ Bug Fixes to C++ Support
   module imports in those situations. (#GH60336)
 - Fix init-capture packs having a size of one before being instantiated. (#GH63677)
 - Clang now preserves the unexpanded flag in a lambda transform used for pack expansion. (#GH56852), (#GH85667),
-  (#GH99877).
+  (#GH99877), (#GH122417).
 - Fixed a bug when diagnosing ambiguous explicit specializations of constrained member functions.
 - Fixed an assertion failure when selecting a function from an overload set that includes a
   specialization of a conversion function template.
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 4a3c739ecbeab8..79df5cf4f2bb56 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -8496,7 +8496,7 @@ TreeTransform<Derived>::TransformDeclStmt(DeclStmt *S) {
             getSema()
                 .getASTContext()
                 .getTypeDeclType(TD)
-                .getCanonicalType()
+                .getSingleStepDesugaredType(getSema().getASTContext())
                 ->containsUnexpandedParameterPack();
 
       if (auto *VD = dyn_cast<VarDecl>(Transformed))
diff --git a/clang/test/SemaCXX/fold_lambda_with_variadics.cpp b/clang/test/SemaCXX/fold_lambda_with_variadics.cpp
index 2257a4c2d975a8..69572bea3664a7 100644
--- a/clang/test/SemaCXX/fold_lambda_with_variadics.cpp
+++ b/clang/test/SemaCXX/fold_lambda_with_variadics.cpp
@@ -7,6 +7,8 @@ struct identity {
   using type = T;
 };
 
+template <class> using ElementType = int;
+
 template <class = void> void f() {
 
   static_assert([]<class... Is>(Is... x) {
@@ -47,6 +49,10 @@ template <class = void> void f() {
     }(), ...);
   }(1, 2);
 
+  []<class... Is>(Is...) {
+    ([] { using T = ElementType<Is>; }(), ...);
+  }(1);
+
   [](auto ...y) {
     ([y] { }(), ...);
   }();

[cor3ntin]

LGTM, but I'd love @mizvekov to double check

[cor3ntin]

What's up with that formatting? 😄

[zyn0217]

Nothing wrong with that, this is just another case falling into that entry - we should have fixed this one in that PR :)

[shafik]

Out of curiosity were applying the getCanonicalType to T and getting int while we really wanted ElementType<Is>?

[zyn0217]

Yes, that's exactly why the issue arises.

[cor3ntin]

LGTM again, feel free to merge

[mizvekov]

I think the change is ok, but I don't think this fixes the underlying issue. There is something generally wrong with type aliases which don't use all of their parameters, and this needs more thought.

[zyn0217]

There is something generally wrong with type aliases which don't use all of their parameters, and this needs more thought.

The current approach - bubbling up the containsUnexpanded flag within a transform - is admittedly not ideal. However, it's the only way to make Clang correctly handle nested packs, within lambdas.

I remembered @cor3ntin once told me Richard had an idea of completely refactoring the pack model as in #9395 (comment). If implemented, we could get rid of this 'workaround'. Maybe we could explore that (probably starting from lambdas?) in the future.