[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-04-05 Thread via cfe-commits


@@ -38,7 +38,7 @@ class CollectUnexpandedParameterPacksVisitor
   unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-bool ContainsIntermediatePacks = false;
+  bool ContainsIntermediatePacks = false;

cor3ntin wrote:

We gain nothing by hiding that behind NDEBUG; we can afford a boolean on the 
stack

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-04-04 Thread Younan Zhang via cfe-commits

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

>From 77537d523bc164a86b46e83651500a4b37c0c3bf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 24 Dec 2024 13:06:44 +0800
Subject: [PATCH 1/8] Reapply "[Clang] Improve diagnostics for expansion length
 mismatch"

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

Co-authored-by: Matheus Izvekov 
---
 clang/include/clang/Sema/Sema.h   |   8 +-
 clang/include/clang/Sema/SemaInternal.h   |   2 +-
 clang/lib/AST/ExprCXX.cpp |   2 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   2 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp  |   7 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 340 +-
 clang/lib/Sema/TreeTransform.h|   1 +
 .../CXX/temp/temp.decls/temp.variadic/p5.cpp  |  50 +++
 .../SemaTemplate/cxx1z-fold-expressions.cpp   |   2 +-
 clang/test/SemaTemplate/pack-deduction.cpp|  51 ++-
 10 files changed, 285 insertions(+), 180 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9ac26d8728446..34f99b8adf467 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,9 +232,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 95874077050a9..acf6c8146d70d 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index c8d61e2cf3f26..afa3db950284b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..756937f44e948 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17589,7 +17589,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front()->getDepth();
   for (UnexpandedParameterPack &U : Unexpanded) {
 if (getDepthAndIndex(U).first >= FriendDeclDepth) {
-  auto *ND = dyn_cast(U.first);
+  auto *ND = dyn_cast(U.first);
   if (!ND)
 ND = cast(U.first)->getDecl();
   Diag(U.second, diag::friend_template_decl_malformed_pack_expansion)
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index e6ec4a7178e81..6af842121ae79 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -874,8 +874,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (isa(U.first))
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index d9256dbd07d7a..d6c71ecec13b6 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,7 +38,7 @@ class CollectUnexpandedParameterPacksVisitor
   unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-bool ContainsIntermediatePacks = false;
+  bool ContainsIntermediatePacks = false;
 #endif
 
 void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation(

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-31 Thread via cfe-commits

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- 
clang/include/clang/Sema/Sema.h clang/include/clang/Sema/SemaInternal.h 
clang/lib/AST/ExprCXX.cpp clang/lib/Sema/SemaConcept.cpp 
clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaTemplateDeduction.cpp 
clang/lib/Sema/SemaTemplateVariadic.cpp clang/lib/Sema/TreeTransform.h 
clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp 
clang/test/SemaTemplate/cxx1z-fold-expressions.cpp 
clang/test/SemaTemplate/pack-deduction.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 14f65d76c..035523864 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -39,21 +39,21 @@ class CollectUnexpandedParameterPacksVisitor
 
   bool ContainsIntermediatePacks = false;
 
-void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation()) {
-  if (auto *VD = dyn_cast(ND)) {
-// For now, the only problematic case is a generic lambda's templated
-// call operator, so we don't need to look for all the other ways we
-// could have reached a dependent parameter pack.
-auto *FD = dyn_cast(VD->getDeclContext());
-auto *FTD = FD ? FD->getDescribedFunctionTemplate() : nullptr;
-if (FTD && FTD->getTemplateParameters()->getDepth() >= DepthLimit)
-  return;
-  } else if (ND->isTemplateParameterPack() &&
- getDepthAndIndex(ND).first >= DepthLimit) {
+  void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation()) {
+if (auto *VD = dyn_cast(ND)) {
+  // For now, the only problematic case is a generic lambda's templated
+  // call operator, so we don't need to look for all the other ways we
+  // could have reached a dependent parameter pack.
+  auto *FD = dyn_cast(VD->getDeclContext());
+  auto *FTD = FD ? FD->getDescribedFunctionTemplate() : nullptr;
+  if (FTD && FTD->getTemplateParameters()->getDepth() >= DepthLimit)
 return;
-  }
+} else if (ND->isTemplateParameterPack() &&
+   getDepthAndIndex(ND).first >= DepthLimit) {
+  return;
+}
 
-  Unexpanded.push_back({ND, Loc});
+Unexpanded.push_back({ND, Loc});
 }
 
 void addUnexpanded(const TemplateTypeParmType *T,

``




https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-31 Thread Younan Zhang via cfe-commits

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

>From 77537d523bc164a86b46e83651500a4b37c0c3bf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 24 Dec 2024 13:06:44 +0800
Subject: [PATCH 1/8] Reapply "[Clang] Improve diagnostics for expansion length
 mismatch"

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

Co-authored-by: Matheus Izvekov 
---
 clang/include/clang/Sema/Sema.h   |   8 +-
 clang/include/clang/Sema/SemaInternal.h   |   2 +-
 clang/lib/AST/ExprCXX.cpp |   2 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   2 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp  |   7 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 340 +-
 clang/lib/Sema/TreeTransform.h|   1 +
 .../CXX/temp/temp.decls/temp.variadic/p5.cpp  |  50 +++
 .../SemaTemplate/cxx1z-fold-expressions.cpp   |   2 +-
 clang/test/SemaTemplate/pack-deduction.cpp|  51 ++-
 10 files changed, 285 insertions(+), 180 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9ac26d8728446..34f99b8adf467 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,9 +232,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 95874077050a9..acf6c8146d70d 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index c8d61e2cf3f26..afa3db950284b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..756937f44e948 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17589,7 +17589,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front()->getDepth();
   for (UnexpandedParameterPack &U : Unexpanded) {
 if (getDepthAndIndex(U).first >= FriendDeclDepth) {
-  auto *ND = dyn_cast(U.first);
+  auto *ND = dyn_cast(U.first);
   if (!ND)
 ND = cast(U.first)->getDecl();
   Diag(U.second, diag::friend_template_decl_malformed_pack_expansion)
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index e6ec4a7178e81..6af842121ae79 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -874,8 +874,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (isa(U.first))
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index d9256dbd07d7a..d6c71ecec13b6 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,7 +38,7 @@ class CollectUnexpandedParameterPacksVisitor
   unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-bool ContainsIntermediatePacks = false;
+  bool ContainsIntermediatePacks = false;
 #endif
 
 void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation(

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-31 Thread Younan Zhang via cfe-commits

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

>From 77537d523bc164a86b46e83651500a4b37c0c3bf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 24 Dec 2024 13:06:44 +0800
Subject: [PATCH 1/9] Reapply "[Clang] Improve diagnostics for expansion length
 mismatch"

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

Co-authored-by: Matheus Izvekov 
---
 clang/include/clang/Sema/Sema.h   |   8 +-
 clang/include/clang/Sema/SemaInternal.h   |   2 +-
 clang/lib/AST/ExprCXX.cpp |   2 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   2 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp  |   7 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 340 +-
 clang/lib/Sema/TreeTransform.h|   1 +
 .../CXX/temp/temp.decls/temp.variadic/p5.cpp  |  50 +++
 .../SemaTemplate/cxx1z-fold-expressions.cpp   |   2 +-
 clang/test/SemaTemplate/pack-deduction.cpp|  51 ++-
 10 files changed, 285 insertions(+), 180 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9ac26d8728446..34f99b8adf467 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,9 +232,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 95874077050a9..acf6c8146d70d 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index c8d61e2cf3f26..afa3db950284b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..756937f44e948 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17589,7 +17589,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front()->getDepth();
   for (UnexpandedParameterPack &U : Unexpanded) {
 if (getDepthAndIndex(U).first >= FriendDeclDepth) {
-  auto *ND = dyn_cast(U.first);
+  auto *ND = dyn_cast(U.first);
   if (!ND)
 ND = cast(U.first)->getDecl();
   Diag(U.second, diag::friend_template_decl_malformed_pack_expansion)
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index e6ec4a7178e81..6af842121ae79 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -874,8 +874,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (isa(U.first))
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index d9256dbd07d7a..d6c71ecec13b6 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,7 +38,7 @@ class CollectUnexpandedParameterPacksVisitor
   unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-bool ContainsIntermediatePacks = false;
+  bool ContainsIntermediatePacks = false;
 #endif
 
 void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation(

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-31 Thread Younan Zhang via cfe-commits

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

>From 77537d523bc164a86b46e83651500a4b37c0c3bf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 24 Dec 2024 13:06:44 +0800
Subject: [PATCH 1/7] Reapply "[Clang] Improve diagnostics for expansion length
 mismatch"

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

Co-authored-by: Matheus Izvekov 
---
 clang/include/clang/Sema/Sema.h   |   8 +-
 clang/include/clang/Sema/SemaInternal.h   |   2 +-
 clang/lib/AST/ExprCXX.cpp |   2 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   2 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp  |   7 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 340 +-
 clang/lib/Sema/TreeTransform.h|   1 +
 .../CXX/temp/temp.decls/temp.variadic/p5.cpp  |  50 +++
 .../SemaTemplate/cxx1z-fold-expressions.cpp   |   2 +-
 clang/test/SemaTemplate/pack-deduction.cpp|  51 ++-
 10 files changed, 285 insertions(+), 180 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9ac26d8728446..34f99b8adf467 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,9 +232,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 95874077050a9..acf6c8146d70d 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index c8d61e2cf3f26..afa3db950284b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..756937f44e948 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17589,7 +17589,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front()->getDepth();
   for (UnexpandedParameterPack &U : Unexpanded) {
 if (getDepthAndIndex(U).first >= FriendDeclDepth) {
-  auto *ND = dyn_cast(U.first);
+  auto *ND = dyn_cast(U.first);
   if (!ND)
 ND = cast(U.first)->getDecl();
   Diag(U.second, diag::friend_template_decl_malformed_pack_expansion)
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index e6ec4a7178e81..6af842121ae79 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -874,8 +874,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (isa(U.first))
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index d9256dbd07d7a..d6c71ecec13b6 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,7 +38,7 @@ class CollectUnexpandedParameterPacksVisitor
   unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-bool ContainsIntermediatePacks = false;
+  bool ContainsIntermediatePacks = false;
 #endif
 
 void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation(

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-31 Thread Younan Zhang via cfe-commits

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

>From 77537d523bc164a86b46e83651500a4b37c0c3bf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 24 Dec 2024 13:06:44 +0800
Subject: [PATCH 1/6] Reapply "[Clang] Improve diagnostics for expansion length
 mismatch"

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

Co-authored-by: Matheus Izvekov 
---
 clang/include/clang/Sema/Sema.h   |   8 +-
 clang/include/clang/Sema/SemaInternal.h   |   2 +-
 clang/lib/AST/ExprCXX.cpp |   2 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   2 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp  |   7 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 340 +-
 clang/lib/Sema/TreeTransform.h|   1 +
 .../CXX/temp/temp.decls/temp.variadic/p5.cpp  |  50 +++
 .../SemaTemplate/cxx1z-fold-expressions.cpp   |   2 +-
 clang/test/SemaTemplate/pack-deduction.cpp|  51 ++-
 10 files changed, 285 insertions(+), 180 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9ac26d8728446..34f99b8adf467 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,9 +232,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 95874077050a9..acf6c8146d70d 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index c8d61e2cf3f26..afa3db950284b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..756937f44e948 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17589,7 +17589,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front()->getDepth();
   for (UnexpandedParameterPack &U : Unexpanded) {
 if (getDepthAndIndex(U).first >= FriendDeclDepth) {
-  auto *ND = dyn_cast(U.first);
+  auto *ND = dyn_cast(U.first);
   if (!ND)
 ND = cast(U.first)->getDecl();
   Diag(U.second, diag::friend_template_decl_malformed_pack_expansion)
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index e6ec4a7178e81..6af842121ae79 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -874,8 +874,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (isa(U.first))
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index d9256dbd07d7a..d6c71ecec13b6 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,7 +38,7 @@ class CollectUnexpandedParameterPacksVisitor
   unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-bool ContainsIntermediatePacks = false;
+  bool ContainsIntermediatePacks = false;
 #endif
 
 void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation(

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-25 Thread via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);
 Decl *B = cast(*Instantiation);
 Expr *BindingExpr = cast(B)->getBinding();
-BindingPack = cast_if_present(BindingExpr);
+auto *BindingPack = cast_if_present(BindingExpr);
 if (!BindingPack) {
   ShouldExpand = false;
   continue;
 }
+NewPackSize = BindingPack->getNumExpansions();
   } else
-std::tie(Depth, Index) = getDepthAndIndex(ND);
-
-  Name = ND->getIdentifier();
+Pos = getDepthAndIndex(ND);
+} else if (const auto *TTP = dyn_cast(P)) {
+  Pos = {TTP->getDepth(), TTP->getIndex()};
+  ND = TTP->getDecl();
+  // FIXME: We either should have some fallback for canonical TTP, or
+  //never have canonical TTP here.
+} else if (const auto *STP =
+   dyn_cast(P)) {
+  NewPackSize = STP->getNumArgs();
+  PendingPackExpansionSize = llvm::count_if(
+  STP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = STP->getReplacedParameter();
+} else {
+  const auto *SEP = cast(P);
+  NewPackSize = SEP->getArgumentPack().pack_size();
+  PendingPackExpansionSize = llvm::count_if(
+  SEP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = SEP->getParameterPack();

cor3ntin wrote:

That makes a lot of sense. Can you add a comment along these lines?

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-25 Thread via cfe-commits


@@ -874,8 +874,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];

cor3ntin wrote:

I'd like to see a comment here (this is sufficiently confusing)

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-25 Thread via cfe-commits


@@ -888,31 +896,47 @@ bool Sema::CheckParameterPacksForExpansion(
   // Pack comes from another template parameter. 'S' is first
   // instantiated, expanding the outer pack 'Outer' to . The alias
   // declaration is accordingly substituted, leaving the template arguments
-  // as unexpanded
-  // ''.
+  // as unexpanded ''.
   //
   // Since we have no idea of the size of '' until its expansion,
   // we shouldn't assume its pack size for validation. However if we are
   // certain that there are extra arguments beyond unexpanded packs, in
   // which case the pack size is already larger than the previous 
expansion,
   // we can complain that before instantiation.
-  unsigned LeastNewPackSize = NewPackSize - PendingPackExpansionSize;
-  if (PendingPackExpansionSize && LeastNewPackSize <= *NumExpansions) {
+  if (PendingPackExpansionSize && LeastNewPackSize <= *CurNumExpansions) {
 ShouldExpand = false;
 continue;
   }
   // C++0x [temp.variadic]p5:
   //   All of the parameter packs expanded by a pack expansion shall have
   //   the same number of arguments specified.
-  if (HaveFirstPack)
-Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict)
-<< FirstPack.first << Name << *NumExpansions
-<< (LeastNewPackSize != NewPackSize) << LeastNewPackSize
-<< SourceRange(FirstPack.second) << SourceRange(ParmPack.second);
-  else
-Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_multilevel)
-<< Name << *NumExpansions << (LeastNewPackSize != NewPackSize)
-<< LeastNewPackSize << SourceRange(ParmPack.second);
+  Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict)
+  << FirstPack.first << Name << *CurNumExpansions
+  << (LeastNewPackSize != NewPackSize) << LeastNewPackSize
+  << SourceRange(FirstPack.second) << SourceRange(Loc);
+  return true;
+}
+  }
+
+  // We have tried our best in the for loop to find out which outer pack
+  // expansion has a different length than the current one (by checking those
+  // Subst* nodes). However, if we fail to determine that, we'll have to
+  // complain the difference in a vague manner.
+  if (NumExpansions && CurNumExpansions &&
+  *NumExpansions != *CurNumExpansions) {
+// If the current pack expansion contains any unresolved packs, and since
+// they might eventually expand to match the outer expansion, we don't
+// complain it too early.

cor3ntin wrote:

```suggestion
// diagnose it now.
```

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-25 Thread via cfe-commits


@@ -888,31 +896,47 @@ bool Sema::CheckParameterPacksForExpansion(
   // Pack comes from another template parameter. 'S' is first
   // instantiated, expanding the outer pack 'Outer' to . The alias
   // declaration is accordingly substituted, leaving the template arguments
-  // as unexpanded
-  // ''.
+  // as unexpanded ''.
   //
   // Since we have no idea of the size of '' until its expansion,
   // we shouldn't assume its pack size for validation. However if we are
   // certain that there are extra arguments beyond unexpanded packs, in
   // which case the pack size is already larger than the previous 
expansion,
   // we can complain that before instantiation.
-  unsigned LeastNewPackSize = NewPackSize - PendingPackExpansionSize;
-  if (PendingPackExpansionSize && LeastNewPackSize <= *NumExpansions) {
+  if (PendingPackExpansionSize && LeastNewPackSize <= *CurNumExpansions) {
 ShouldExpand = false;
 continue;
   }
   // C++0x [temp.variadic]p5:
   //   All of the parameter packs expanded by a pack expansion shall have
   //   the same number of arguments specified.
-  if (HaveFirstPack)
-Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict)
-<< FirstPack.first << Name << *NumExpansions
-<< (LeastNewPackSize != NewPackSize) << LeastNewPackSize
-<< SourceRange(FirstPack.second) << SourceRange(ParmPack.second);
-  else
-Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_multilevel)
-<< Name << *NumExpansions << (LeastNewPackSize != NewPackSize)
-<< LeastNewPackSize << SourceRange(ParmPack.second);
+  Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict)
+  << FirstPack.first << Name << *CurNumExpansions
+  << (LeastNewPackSize != NewPackSize) << LeastNewPackSize
+  << SourceRange(FirstPack.second) << SourceRange(Loc);
+  return true;
+}
+  }
+
+  // We have tried our best in the for loop to find out which outer pack
+  // expansion has a different length than the current one (by checking those
+  // Subst* nodes). However, if we fail to determine that, we'll have to
+  // complain the difference in a vague manner.

cor3ntin wrote:

```suggestion
  // We have tried our best in the for loop to find out which outer pack
  // expansion has a different length than the current one (by checking those
  // Subst* nodes).  Here we display a more generic diagnostic if we could not 
find the outer pack.
```

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-25 Thread via cfe-commits


@@ -756,125 +766,123 @@ bool Sema::CheckParameterPacksForExpansion(
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;

cor3ntin wrote:

Can we add comments?

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-24 Thread Matheus Izvekov via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);
 Decl *B = cast(*Instantiation);
 Expr *BindingExpr = cast(B)->getBinding();
-BindingPack = cast_if_present(BindingExpr);
+auto *BindingPack = cast_if_present(BindingExpr);
 if (!BindingPack) {
   ShouldExpand = false;
   continue;
 }
+NewPackSize = BindingPack->getNumExpansions();
   } else
-std::tie(Depth, Index) = getDepthAndIndex(ND);
-
-  Name = ND->getIdentifier();
+Pos = getDepthAndIndex(ND);
+} else if (const auto *TTP = dyn_cast(P)) {
+  Pos = {TTP->getDepth(), TTP->getIndex()};
+  ND = TTP->getDecl();
+  // FIXME: We either should have some fallback for canonical TTP, or
+  //never have canonical TTP here.
+} else if (const auto *STP =
+   dyn_cast(P)) {
+  NewPackSize = STP->getNumArgs();
+  PendingPackExpansionSize = llvm::count_if(
+  STP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = STP->getReplacedParameter();
+} else {
+  const auto *SEP = cast(P);
+  NewPackSize = SEP->getArgumentPack().pack_size();
+  PendingPackExpansionSize = llvm::count_if(
+  SEP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = SEP->getParameterPack();
 }
 
-// Determine the size of this argument pack.
-unsigned NewPackSize, PendingPackExpansionSize = 0;
-if (IsVarDeclPack) {
-  // Figure out whether we're instantiating to an argument pack or not.
-  llvm::PointerUnion *Instantiation =
-  CurrentInstantiationScope->findInstantiationOf(
-  cast(ParmPack.first));
-  if (isa(*Instantiation)) {
-// We could expand this function parameter pack.
-NewPackSize = cast(*Instantiation)->size();
-  } else {
-// We can't expand this function parameter pack, so we can't expand
-// the pack expansion.
-ShouldExpand = false;
-continue;
-  }
-} else if (BindingPack) {
-  NewPackSize = BindingPack->getNumExpansions();
-} else {
+if (Pos) {
   // If we don't have a template argument at this depth/index, then we
   // cannot expand the pack expansion. Make a note of this, but we still
   // want to check any parameter

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-23 Thread Younan Zhang via cfe-commits

zyn0217 wrote:

@cor3ntin do you have any other concerns? thanks

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-21 Thread Younan Zhang via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);
 Decl *B = cast(*Instantiation);
 Expr *BindingExpr = cast(B)->getBinding();
-BindingPack = cast_if_present(BindingExpr);
+auto *BindingPack = cast_if_present(BindingExpr);
 if (!BindingPack) {
   ShouldExpand = false;
   continue;
 }
+NewPackSize = BindingPack->getNumExpansions();
   } else
-std::tie(Depth, Index) = getDepthAndIndex(ND);
-
-  Name = ND->getIdentifier();
+Pos = getDepthAndIndex(ND);
+} else if (const auto *TTP = dyn_cast(P)) {
+  Pos = {TTP->getDepth(), TTP->getIndex()};
+  ND = TTP->getDecl();
+  // FIXME: We either should have some fallback for canonical TTP, or
+  //never have canonical TTP here.
+} else if (const auto *STP =
+   dyn_cast(P)) {
+  NewPackSize = STP->getNumArgs();
+  PendingPackExpansionSize = llvm::count_if(
+  STP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = STP->getReplacedParameter();
+} else {
+  const auto *SEP = cast(P);
+  NewPackSize = SEP->getArgumentPack().pack_size();
+  PendingPackExpansionSize = llvm::count_if(
+  SEP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = SEP->getParameterPack();
 }
 
-// Determine the size of this argument pack.
-unsigned NewPackSize, PendingPackExpansionSize = 0;
-if (IsVarDeclPack) {
-  // Figure out whether we're instantiating to an argument pack or not.
-  llvm::PointerUnion *Instantiation =
-  CurrentInstantiationScope->findInstantiationOf(
-  cast(ParmPack.first));
-  if (isa(*Instantiation)) {
-// We could expand this function parameter pack.
-NewPackSize = cast(*Instantiation)->size();
-  } else {
-// We can't expand this function parameter pack, so we can't expand
-// the pack expansion.
-ShouldExpand = false;
-continue;
-  }
-} else if (BindingPack) {
-  NewPackSize = BindingPack->getNumExpansions();
-} else {
+if (Pos) {
   // If we don't have a template argument at this depth/index, then we
   // cannot expand the pack expansion. Make a note of this, but we still
   // want to check any parameter

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-14 Thread Younan Zhang via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);
 Decl *B = cast(*Instantiation);
 Expr *BindingExpr = cast(B)->getBinding();
-BindingPack = cast_if_present(BindingExpr);
+auto *BindingPack = cast_if_present(BindingExpr);
 if (!BindingPack) {
   ShouldExpand = false;
   continue;
 }
+NewPackSize = BindingPack->getNumExpansions();
   } else
-std::tie(Depth, Index) = getDepthAndIndex(ND);
-
-  Name = ND->getIdentifier();
+Pos = getDepthAndIndex(ND);
+} else if (const auto *TTP = dyn_cast(P)) {
+  Pos = {TTP->getDepth(), TTP->getIndex()};
+  ND = TTP->getDecl();
+  // FIXME: We either should have some fallback for canonical TTP, or
+  //never have canonical TTP here.
+} else if (const auto *STP =
+   dyn_cast(P)) {
+  NewPackSize = STP->getNumArgs();
+  PendingPackExpansionSize = llvm::count_if(
+  STP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = STP->getReplacedParameter();
+} else {
+  const auto *SEP = cast(P);
+  NewPackSize = SEP->getArgumentPack().pack_size();
+  PendingPackExpansionSize = llvm::count_if(
+  SEP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = SEP->getParameterPack();
 }
 
-// Determine the size of this argument pack.
-unsigned NewPackSize, PendingPackExpansionSize = 0;
-if (IsVarDeclPack) {
-  // Figure out whether we're instantiating to an argument pack or not.
-  llvm::PointerUnion *Instantiation =
-  CurrentInstantiationScope->findInstantiationOf(
-  cast(ParmPack.first));
-  if (isa(*Instantiation)) {
-// We could expand this function parameter pack.
-NewPackSize = cast(*Instantiation)->size();
-  } else {
-// We can't expand this function parameter pack, so we can't expand
-// the pack expansion.
-ShouldExpand = false;
-continue;
-  }
-} else if (BindingPack) {
-  NewPackSize = BindingPack->getNumExpansions();
-} else {
+if (Pos) {
   // If we don't have a template argument at this depth/index, then we
   // cannot expand the pack expansion. Make a note of this, but we still
   // want to check any parameter

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread Younan Zhang via cfe-commits

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

>From 77537d523bc164a86b46e83651500a4b37c0c3bf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 24 Dec 2024 13:06:44 +0800
Subject: [PATCH 1/4] Reapply "[Clang] Improve diagnostics for expansion length
 mismatch"

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

Co-authored-by: Matheus Izvekov 
---
 clang/include/clang/Sema/Sema.h   |   8 +-
 clang/include/clang/Sema/SemaInternal.h   |   2 +-
 clang/lib/AST/ExprCXX.cpp |   2 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   2 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp  |   7 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 340 +-
 clang/lib/Sema/TreeTransform.h|   1 +
 .../CXX/temp/temp.decls/temp.variadic/p5.cpp  |  50 +++
 .../SemaTemplate/cxx1z-fold-expressions.cpp   |   2 +-
 clang/test/SemaTemplate/pack-deduction.cpp|  51 ++-
 10 files changed, 285 insertions(+), 180 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9ac26d8728446..34f99b8adf467 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,9 +232,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 95874077050a9..acf6c8146d70d 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index c8d61e2cf3f26..afa3db950284b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..756937f44e948 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17589,7 +17589,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front()->getDepth();
   for (UnexpandedParameterPack &U : Unexpanded) {
 if (getDepthAndIndex(U).first >= FriendDeclDepth) {
-  auto *ND = dyn_cast(U.first);
+  auto *ND = dyn_cast(U.first);
   if (!ND)
 ND = cast(U.first)->getDecl();
   Diag(U.second, diag::friend_template_decl_malformed_pack_expansion)
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index e6ec4a7178e81..6af842121ae79 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -874,8 +874,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (isa(U.first))
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index d9256dbd07d7a..d6c71ecec13b6 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,7 +38,7 @@ class CollectUnexpandedParameterPacksVisitor
   unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-bool ContainsIntermediatePacks = false;
+  bool ContainsIntermediatePacks = false;
 #endif
 
 void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation(

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread Younan Zhang via cfe-commits

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

>From 77537d523bc164a86b46e83651500a4b37c0c3bf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 24 Dec 2024 13:06:44 +0800
Subject: [PATCH 1/3] Reapply "[Clang] Improve diagnostics for expansion length
 mismatch"

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

Co-authored-by: Matheus Izvekov 
---
 clang/include/clang/Sema/Sema.h   |   8 +-
 clang/include/clang/Sema/SemaInternal.h   |   2 +-
 clang/lib/AST/ExprCXX.cpp |   2 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   2 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp  |   7 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 340 +-
 clang/lib/Sema/TreeTransform.h|   1 +
 .../CXX/temp/temp.decls/temp.variadic/p5.cpp  |  50 +++
 .../SemaTemplate/cxx1z-fold-expressions.cpp   |   2 +-
 clang/test/SemaTemplate/pack-deduction.cpp|  51 ++-
 10 files changed, 285 insertions(+), 180 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9ac26d8728446..34f99b8adf467 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,9 +232,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 95874077050a9..acf6c8146d70d 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index c8d61e2cf3f26..afa3db950284b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..756937f44e948 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17589,7 +17589,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front()->getDepth();
   for (UnexpandedParameterPack &U : Unexpanded) {
 if (getDepthAndIndex(U).first >= FriendDeclDepth) {
-  auto *ND = dyn_cast(U.first);
+  auto *ND = dyn_cast(U.first);
   if (!ND)
 ND = cast(U.first)->getDecl();
   Diag(U.second, diag::friend_template_decl_malformed_pack_expansion)
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index e6ec4a7178e81..6af842121ae79 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -874,8 +874,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (isa(U.first))
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index d9256dbd07d7a..d6c71ecec13b6 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,7 +38,7 @@ class CollectUnexpandedParameterPacksVisitor
   unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-bool ContainsIntermediatePacks = false;
+  bool ContainsIntermediatePacks = false;
 #endif
 
 void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation(

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);
 Decl *B = cast(*Instantiation);
 Expr *BindingExpr = cast(B)->getBinding();
-BindingPack = cast_if_present(BindingExpr);
+auto *BindingPack = cast_if_present(BindingExpr);
 if (!BindingPack) {
   ShouldExpand = false;
   continue;
 }
+NewPackSize = BindingPack->getNumExpansions();
   } else
-std::tie(Depth, Index) = getDepthAndIndex(ND);
-
-  Name = ND->getIdentifier();
+Pos = getDepthAndIndex(ND);
+} else if (const auto *TTP = dyn_cast(P)) {
+  Pos = {TTP->getDepth(), TTP->getIndex()};
+  ND = TTP->getDecl();
+  // FIXME: We either should have some fallback for canonical TTP, or
+  //never have canonical TTP here.
+} else if (const auto *STP =
+   dyn_cast(P)) {
+  NewPackSize = STP->getNumArgs();
+  PendingPackExpansionSize = llvm::count_if(
+  STP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = STP->getReplacedParameter();
+} else {
+  const auto *SEP = cast(P);
+  NewPackSize = SEP->getArgumentPack().pack_size();
+  PendingPackExpansionSize = llvm::count_if(
+  SEP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = SEP->getParameterPack();
 }
 
-// Determine the size of this argument pack.
-unsigned NewPackSize, PendingPackExpansionSize = 0;
-if (IsVarDeclPack) {
-  // Figure out whether we're instantiating to an argument pack or not.
-  llvm::PointerUnion *Instantiation =
-  CurrentInstantiationScope->findInstantiationOf(
-  cast(ParmPack.first));
-  if (isa(*Instantiation)) {
-// We could expand this function parameter pack.
-NewPackSize = cast(*Instantiation)->size();
-  } else {
-// We can't expand this function parameter pack, so we can't expand
-// the pack expansion.
-ShouldExpand = false;
-continue;
-  }
-} else if (BindingPack) {
-  NewPackSize = BindingPack->getNumExpansions();
-} else {
+if (Pos) {
   // If we don't have a template argument at this depth/index, then we
   // cannot expand the pack expansion. Make a note of this, but we still
   // want to check any parameter

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);
 Decl *B = cast(*Instantiation);
 Expr *BindingExpr = cast(B)->getBinding();
-BindingPack = cast_if_present(BindingExpr);
+auto *BindingPack = cast_if_present(BindingExpr);
 if (!BindingPack) {
   ShouldExpand = false;
   continue;
 }
+NewPackSize = BindingPack->getNumExpansions();
   } else
-std::tie(Depth, Index) = getDepthAndIndex(ND);
-
-  Name = ND->getIdentifier();
+Pos = getDepthAndIndex(ND);
+} else if (const auto *TTP = dyn_cast(P)) {
+  Pos = {TTP->getDepth(), TTP->getIndex()};
+  ND = TTP->getDecl();
+  // FIXME: We either should have some fallback for canonical TTP, or
+  //never have canonical TTP here.
+} else if (const auto *STP =
+   dyn_cast(P)) {
+  NewPackSize = STP->getNumArgs();
+  PendingPackExpansionSize = llvm::count_if(
+  STP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = STP->getReplacedParameter();
+} else {
+  const auto *SEP = cast(P);
+  NewPackSize = SEP->getArgumentPack().pack_size();
+  PendingPackExpansionSize = llvm::count_if(
+  SEP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = SEP->getParameterPack();
 }
 
-// Determine the size of this argument pack.
-unsigned NewPackSize, PendingPackExpansionSize = 0;
-if (IsVarDeclPack) {
-  // Figure out whether we're instantiating to an argument pack or not.
-  llvm::PointerUnion *Instantiation =
-  CurrentInstantiationScope->findInstantiationOf(
-  cast(ParmPack.first));
-  if (isa(*Instantiation)) {
-// We could expand this function parameter pack.
-NewPackSize = cast(*Instantiation)->size();
-  } else {
-// We can't expand this function parameter pack, so we can't expand
-// the pack expansion.
-ShouldExpand = false;
-continue;
-  }
-} else if (BindingPack) {
-  NewPackSize = BindingPack->getNumExpansions();
-} else {
+if (Pos) {
   // If we don't have a template argument at this depth/index, then we
   // cannot expand the pack expansion. Make a note of this, but we still
   // want to check any parameter

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread via cfe-commits


@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 

cor3ntin wrote:

We should add support for SubstTemplateTypeParmPackType / 
SubstNonTypeTemplateParmPackExpr here

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread Younan Zhang via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);
 Decl *B = cast(*Instantiation);
 Expr *BindingExpr = cast(B)->getBinding();
-BindingPack = cast_if_present(BindingExpr);
+auto *BindingPack = cast_if_present(BindingExpr);
 if (!BindingPack) {
   ShouldExpand = false;
   continue;
 }
+NewPackSize = BindingPack->getNumExpansions();
   } else
-std::tie(Depth, Index) = getDepthAndIndex(ND);
-
-  Name = ND->getIdentifier();
+Pos = getDepthAndIndex(ND);
+} else if (const auto *TTP = dyn_cast(P)) {
+  Pos = {TTP->getDepth(), TTP->getIndex()};
+  ND = TTP->getDecl();
+  // FIXME: We either should have some fallback for canonical TTP, or
+  //never have canonical TTP here.
+} else if (const auto *STP =
+   dyn_cast(P)) {
+  NewPackSize = STP->getNumArgs();
+  PendingPackExpansionSize = llvm::count_if(
+  STP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = STP->getReplacedParameter();
+} else {
+  const auto *SEP = cast(P);
+  NewPackSize = SEP->getArgumentPack().pack_size();
+  PendingPackExpansionSize = llvm::count_if(
+  SEP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = SEP->getParameterPack();
 }
 
-// Determine the size of this argument pack.
-unsigned NewPackSize, PendingPackExpansionSize = 0;
-if (IsVarDeclPack) {
-  // Figure out whether we're instantiating to an argument pack or not.
-  llvm::PointerUnion *Instantiation =
-  CurrentInstantiationScope->findInstantiationOf(
-  cast(ParmPack.first));
-  if (isa(*Instantiation)) {
-// We could expand this function parameter pack.
-NewPackSize = cast(*Instantiation)->size();
-  } else {
-// We can't expand this function parameter pack, so we can't expand
-// the pack expansion.
-ShouldExpand = false;
-continue;
-  }
-} else if (BindingPack) {
-  NewPackSize = BindingPack->getNumExpansions();
-} else {
+if (Pos) {
   // If we don't have a template argument at this depth/index, then we
   // cannot expand the pack expansion. Make a note of this, but we still
   // want to check any parameter

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread Younan Zhang via cfe-commits


@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 

zyn0217 wrote:

I think the users of this function shouldn't expect a 
`SubstTemplateTypeParmPackType/SubstNonTypeTemplateParmPackExpr` anyway. (See 
the explanation below) I'll add some asserts.

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread Younan Zhang via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);

zyn0217 wrote:

`findInstantiationOf()` has an assert already

```cpp
  // If we didn't find the decl, then we either have a sema bug, or we have a
  // forward reference to a label declaration.  Return null to indicate that
  // we have an uninstantiated label.
  assert(isa(D) && "declaration not instantiated in this scope");
```

Even if it returned a null, it would still be guarded by the `cast` 
next to it.

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread Younan Zhang via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);
 Decl *B = cast(*Instantiation);
 Expr *BindingExpr = cast(B)->getBinding();
-BindingPack = cast_if_present(BindingExpr);
+auto *BindingPack = cast_if_present(BindingExpr);
 if (!BindingPack) {
   ShouldExpand = false;
   continue;
 }
+NewPackSize = BindingPack->getNumExpansions();
   } else
-std::tie(Depth, Index) = getDepthAndIndex(ND);
-
-  Name = ND->getIdentifier();
+Pos = getDepthAndIndex(ND);
+} else if (const auto *TTP = dyn_cast(P)) {
+  Pos = {TTP->getDepth(), TTP->getIndex()};
+  ND = TTP->getDecl();
+  // FIXME: We either should have some fallback for canonical TTP, or
+  //never have canonical TTP here.
+} else if (const auto *STP =
+   dyn_cast(P)) {
+  NewPackSize = STP->getNumArgs();
+  PendingPackExpansionSize = llvm::count_if(
+  STP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = STP->getReplacedParameter();
+} else {
+  const auto *SEP = cast(P);
+  NewPackSize = SEP->getArgumentPack().pack_size();
+  PendingPackExpansionSize = llvm::count_if(
+  SEP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = SEP->getParameterPack();

zyn0217 wrote:

The function `CheckParameterPacksForExpansion` serves two purposes (though 
combining them is really confusing):

1. Given a (multi-level) template argument list, it determines the length to 
which an unexpanded pack would expand, if possible.
2. If we encounter a node that has already been expanded (with its previous 
expansion size tracked in NumExpansions), it checks whether the length of packs 
expanded in this round matches that from the previous round. If they differ, 
complain.

Now that I mentioned 'last round', it's likely that in this round we'll 
encounter some `Subst*` nodes created previously because they haven't been 
expanded yet (but substituted!). This is exactly what this patch aims for: to 
recover the identifiers contained in those `Subst*` nodes - which correspond to 
the parameters that were *substituted but not expanded* in the previous round - 
so that we can improve our diagnostics by identifying which parameters conflict 

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);
 Decl *B = cast(*Instantiation);
 Expr *BindingExpr = cast(B)->getBinding();
-BindingPack = cast_if_present(BindingExpr);
+auto *BindingPack = cast_if_present(BindingExpr);
 if (!BindingPack) {
   ShouldExpand = false;
   continue;
 }
+NewPackSize = BindingPack->getNumExpansions();
   } else
-std::tie(Depth, Index) = getDepthAndIndex(ND);
-
-  Name = ND->getIdentifier();
+Pos = getDepthAndIndex(ND);
+} else if (const auto *TTP = dyn_cast(P)) {
+  Pos = {TTP->getDepth(), TTP->getIndex()};
+  ND = TTP->getDecl();
+  // FIXME: We either should have some fallback for canonical TTP, or
+  //never have canonical TTP here.
+} else if (const auto *STP =
+   dyn_cast(P)) {
+  NewPackSize = STP->getNumArgs();
+  PendingPackExpansionSize = llvm::count_if(
+  STP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = STP->getReplacedParameter();
+} else {
+  const auto *SEP = cast(P);
+  NewPackSize = SEP->getArgumentPack().pack_size();
+  PendingPackExpansionSize = llvm::count_if(
+  SEP->getArgumentPack().getPackAsArray(), IsUnexpandedPackExpansion);
+  ND = SEP->getParameterPack();

cor3ntin wrote:

Why are we not setting Pos here?

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread via cfe-commits


@@ -888,31 +890,40 @@ bool Sema::CheckParameterPacksForExpansion(
   // Pack comes from another template parameter. 'S' is first
   // instantiated, expanding the outer pack 'Outer' to . The alias
   // declaration is accordingly substituted, leaving the template arguments
-  // as unexpanded
-  // ''.
+  // as unexpanded ''.
   //
   // Since we have no idea of the size of '' until its expansion,
   // we shouldn't assume its pack size for validation. However if we are
   // certain that there are extra arguments beyond unexpanded packs, in
   // which case the pack size is already larger than the previous 
expansion,
   // we can complain that before instantiation.
-  unsigned LeastNewPackSize = NewPackSize - PendingPackExpansionSize;
-  if (PendingPackExpansionSize && LeastNewPackSize <= *NumExpansions) {
+  if (PendingPackExpansionSize && LeastNewPackSize <= *CurNumExpansions) {
 ShouldExpand = false;
 continue;
   }
   // C++0x [temp.variadic]p5:
   //   All of the parameter packs expanded by a pack expansion shall have
   //   the same number of arguments specified.
-  if (HaveFirstPack)
-Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict)
-<< FirstPack.first << Name << *NumExpansions
-<< (LeastNewPackSize != NewPackSize) << LeastNewPackSize
-<< SourceRange(FirstPack.second) << SourceRange(ParmPack.second);
-  else
-Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_multilevel)
-<< Name << *NumExpansions << (LeastNewPackSize != NewPackSize)
-<< LeastNewPackSize << SourceRange(ParmPack.second);
+  Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict)
+  << FirstPack.first << Name << *CurNumExpansions
+  << (LeastNewPackSize != NewPackSize) << LeastNewPackSize
+  << SourceRange(FirstPack.second) << SourceRange(Loc);
+  return true;
+}
+  }
+
+  if (NumExpansions && CurNumExpansions &&
+  *NumExpansions != *CurNumExpansions) {
+if (CurMaximumOfLeastExpansions &&
+*CurMaximumOfLeastExpansions <= *NumExpansions) {
+  ShouldExpand = false;

cor3ntin wrote:

Can you add a comment?

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}

cor3ntin wrote:

Can you leave a comment here? This is a bit confusing.
Actually, I think having it as a lambda, like in the current code, might be 
clearer.

It's not just that they are unexpended, they are also unresolvable at that 
point.

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-13 Thread via cfe-commits


@@ -749,132 +759,124 @@ ExprResult Sema::CheckPackExpansion(Expr *Pattern, 
SourceLocation EllipsisLoc,
 PackExpansionExpr(Context.DependentTy, Pattern, EllipsisLoc, 
NumExpansions);
 }
 
+static bool IsUnexpandedPackExpansion(const TemplateArgument &TA) {
+  if (!TA.isPackExpansion())
+return false;
+
+  if (TA.getKind() == TemplateArgument::Type)
+return !TA.getAsType()->getAs()->getNumExpansions();
+
+  if (TA.getKind() == TemplateArgument::Expression)
+return !cast(TA.getAsExpr())->getNumExpansions();
+
+  return !TA.getNumTemplateExpansions();
+}
+
 bool Sema::CheckParameterPacksForExpansion(
 SourceLocation EllipsisLoc, SourceRange PatternRange,
 ArrayRef Unexpanded,
 const MultiLevelTemplateArgumentList &TemplateArgs, bool &ShouldExpand,
 bool &RetainExpansion, std::optional &NumExpansions) {
   ShouldExpand = true;
   RetainExpansion = false;
-  std::pair FirstPack;
-  bool HaveFirstPack = false;
-  std::optional NumPartialExpansions;
-  SourceLocation PartiallySubstitutedPackLoc;
-  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
+  std::pair FirstPack;
+  std::optional> PartialExpansion;
+  std::optional CurNumExpansions, CurMaximumOfLeastExpansions;
 
-  for (UnexpandedParameterPack ParmPack : Unexpanded) {
+  for (auto [P, Loc] : Unexpanded) {
 // Compute the depth and index for this parameter pack.
-unsigned Depth = 0, Index = 0;
-IdentifierInfo *Name;
-bool IsVarDeclPack = false;
-FunctionParmPackExpr *BindingPack = nullptr;
-
-if (const TemplateTypeParmType *TTP =
-ParmPack.first.dyn_cast()) {
-  Depth = TTP->getDepth();
-  Index = TTP->getIndex();
-  Name = TTP->getIdentifier();
-} else {
-  NamedDecl *ND = cast(ParmPack.first);
-  if (isa(ND))
-IsVarDeclPack = true;
-  else if (isa(ND)) {
+std::optional> Pos;
+unsigned NewPackSize, PendingPackExpansionSize = 0;
+const auto *ND = dyn_cast_if_present(P);
+if (ND) {
+  if (isa(ND)) {
+auto *DAP = dyn_cast(
+*CurrentInstantiationScope->findInstantiationOf(ND));
+if (!DAP) {
+  // We can't expand this function parameter pack, so we can't expand
+  // the pack expansion.
+  ShouldExpand = false;
+  continue;
+}
+NewPackSize = DAP->size();
+  } else if (isa(ND)) {
 // Find the instantiated BindingDecl and check it for a resolved pack.
-llvm::PointerUnion *Instantiation =
-CurrentInstantiationScope->findInstantiationOf(ND);
+llvm::PointerUnion
+*Instantiation = 
CurrentInstantiationScope->findInstantiationOf(ND);

cor3ntin wrote:

Pre-existing but do we need an assert here? Can it be null?

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-12 Thread Younan Zhang via cfe-commits

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

>From 77537d523bc164a86b46e83651500a4b37c0c3bf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 24 Dec 2024 13:06:44 +0800
Subject: [PATCH 1/2] Reapply "[Clang] Improve diagnostics for expansion length
 mismatch"

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

Co-authored-by: Matheus Izvekov 
---
 clang/include/clang/Sema/Sema.h   |   8 +-
 clang/include/clang/Sema/SemaInternal.h   |   2 +-
 clang/lib/AST/ExprCXX.cpp |   2 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   2 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp  |   7 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 340 +-
 clang/lib/Sema/TreeTransform.h|   1 +
 .../CXX/temp/temp.decls/temp.variadic/p5.cpp  |  50 +++
 .../SemaTemplate/cxx1z-fold-expressions.cpp   |   2 +-
 clang/test/SemaTemplate/pack-deduction.cpp|  51 ++-
 10 files changed, 285 insertions(+), 180 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9ac26d8728446..34f99b8adf467 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,9 +232,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 95874077050a9..acf6c8146d70d 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index c8d61e2cf3f26..afa3db950284b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..756937f44e948 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17589,7 +17589,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front()->getDepth();
   for (UnexpandedParameterPack &U : Unexpanded) {
 if (getDepthAndIndex(U).first >= FriendDeclDepth) {
-  auto *ND = dyn_cast(U.first);
+  auto *ND = dyn_cast(U.first);
   if (!ND)
 ND = cast(U.first)->getDecl();
   Diag(U.second, diag::friend_template_decl_malformed_pack_expansion)
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index e6ec4a7178e81..6af842121ae79 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -874,8 +874,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (isa(U.first))
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index d9256dbd07d7a..d6c71ecec13b6 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,7 +38,7 @@ class CollectUnexpandedParameterPacksVisitor
   unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-bool ContainsIntermediatePacks = false;
+  bool ContainsIntermediatePacks = false;
 #endif
 
 void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation(

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-12 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)


Changes

This patch reapplies 3a0309c5 "[Clang] Improve diagnostics for expansion length 
mismatch" and b8a1b698 "[Clang] fix missing initialization of original number 
of expansions".

Both were reverted before the Clang 16 release due to a regression uncovered by 
GH58452.

I believe this regression can now be avoided by my recent patch 5c55f966, which 
defers the overeager diagnostics until the `PackExpansionTypes` are actually 
expanded. Therefore, it should be safe to reapply these two patches, as they do 
help improve diagnostics for packs.

Moreover, I also pulled in Richard's test cases for 
[PR16668](https://bugs.llvm.org/show_bug.cgi?id=16668#c2) which, unfortunately 
uncovers yet another rejects-valid case which I don't think urgent to fix at 
this point. However, part of this patch - specifically teaching pack-expansion 
functions to recognize `Subst*` packs - is helpful (and essential) to solve 
that bug. For now, that portion is blocked with `#if 0` in the test 
`clang/test/SemaTemplate/pack-deduction.cpp`.

This reverts commit acecf68c and adopts the patch to the current 
`CheckParameterPacksForExpansion`, while also following the ongoing migration 
away from `PointerIntPairs::{is,get}` and their friends.

Below is the original commit message, by @mizvekov who will be credited 
as the co-author.

When checking parameter packs for expansion, instead of basing the diagnostic 
for
length mismatch for outer parameters only on the known number of expansions,
we should also analyze SubstTemplateTypeParmPackType and 
SubstNonTypeTemplateParmPackExpr
for unexpanded packs, so we can emit a diagnostic pointing to a concrete
outer parameter.

When expanding undeclared function parameters, we should initialize
the original number of expansions, if known, before trying to expand
them, otherwise a length mismatch with an outer pack might not be
diagnosed.

Fixes #56094

---

Patch is 31.75 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/121044.diff


10 Files Affected:

- (modified) clang/include/clang/Sema/Sema.h (+5-3) 
- (modified) clang/include/clang/Sema/SemaInternal.h (+1-1) 
- (modified) clang/lib/AST/ExprCXX.cpp (+1-1) 
- (modified) clang/lib/Sema/SemaDeclCXX.cpp (+1-1) 
- (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+5-2) 
- (modified) clang/lib/Sema/SemaTemplateVariadic.cpp (+176-164) 
- (modified) clang/lib/Sema/TreeTransform.h (+1) 
- (modified) clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp (+50) 
- (modified) clang/test/SemaTemplate/cxx1z-fold-expressions.cpp (+1-1) 
- (modified) clang/test/SemaTemplate/pack-deduction.cpp (+44-7) 


``diff
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9ac26d8728446..34f99b8adf467 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,9 +232,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 95874077050a9..acf6c8146d70d 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index c8d61e2cf3f26..afa3db950284b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..756937f44e948 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17589,7 +17589,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-12 Thread Younan Zhang via cfe-commits

https://github.com/zyn0217 ready_for_review 
https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-03-12 Thread Younan Zhang via cfe-commits

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

>From 77537d523bc164a86b46e83651500a4b37c0c3bf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 24 Dec 2024 13:06:44 +0800
Subject: [PATCH] Reapply "[Clang] Improve diagnostics for expansion length
 mismatch"

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

Co-authored-by: Matheus Izvekov 
---
 clang/include/clang/Sema/Sema.h   |   8 +-
 clang/include/clang/Sema/SemaInternal.h   |   2 +-
 clang/lib/AST/ExprCXX.cpp |   2 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   2 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp  |   7 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 340 +-
 clang/lib/Sema/TreeTransform.h|   1 +
 .../CXX/temp/temp.decls/temp.variadic/p5.cpp  |  50 +++
 .../SemaTemplate/cxx1z-fold-expressions.cpp   |   2 +-
 clang/test/SemaTemplate/pack-deduction.cpp|  51 ++-
 10 files changed, 285 insertions(+), 180 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9ac26d8728446..34f99b8adf467 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -232,9 +232,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 95874077050a9..acf6c8146d70d 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -75,7 +75,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index c8d61e2cf3f26..afa3db950284b 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 96aac7871db1e..756937f44e948 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17589,7 +17589,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front()->getDepth();
   for (UnexpandedParameterPack &U : Unexpanded) {
 if (getDepthAndIndex(U).first >= FriendDeclDepth) {
-  auto *ND = dyn_cast(U.first);
+  auto *ND = dyn_cast(U.first);
   if (!ND)
 ND = cast(U.first)->getDecl();
   Diag(U.second, diag::friend_template_decl_malformed_pack_expansion)
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index e6ec4a7178e81..6af842121ae79 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -874,8 +874,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (isa(U.first))
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index d9256dbd07d7a..d6c71ecec13b6 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,7 +38,7 @@ class CollectUnexpandedParameterPacksVisitor
   unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-bool ContainsIntermediatePacks = false;
+  bool ContainsIntermediatePacks = false;
 #endif
 
 void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation()) {

[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-01-07 Thread Younan Zhang via cfe-commits

zyn0217 wrote:

(I plan to merge this after the branch 20 cut to avoid any potential revert 
churn. So it will stay in draft for now)

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-01-07 Thread Matheus Izvekov via cfe-commits

https://github.com/mizvekov approved this pull request.

LGTM, though I would have preferred to keep the patches separate, it's less of 
a revert risk.

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2025-01-02 Thread Erich Keane via cfe-commits

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2024-12-24 Thread Matheus Izvekov via cfe-commits

mizvekov wrote:

Thanks! I think I completely missed the fact these were both reverted.

https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2024-12-24 Thread Younan Zhang via cfe-commits

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/121044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)

2024-12-24 Thread Younan Zhang via cfe-commits

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/121044

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

>From aa008450981a80a2d26e687df33f10038aa50a9c Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 24 Dec 2024 13:06:44 +0800
Subject: [PATCH] Reapply "[Clang] Improve diagnostics for expansion length
 mismatch"

... and "[Clang] fix missing initialization of original number of expansions"

This reverts commit acecf68c8b7c3c625cfa00f00f8ddc8f15baae44.

Co-authored-by: Matheus Izvekov 
---
 clang/include/clang/Sema/Sema.h   |   8 +-
 clang/include/clang/Sema/SemaInternal.h   |   4 +-
 clang/lib/AST/ExprCXX.cpp |   2 +-
 clang/lib/Sema/SemaDeclCXX.cpp|   2 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp  |   7 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 318 +-
 clang/lib/Sema/TreeTransform.h|   1 +
 .../CXX/temp/temp.decls/temp.variadic/p5.cpp  |  50 +++
 .../SemaTemplate/cxx1z-fold-expressions.cpp   |   2 +-
 clang/test/SemaTemplate/pack-deduction.cpp|  51 ++-
 10 files changed, 275 insertions(+), 170 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5ee7ea48cc983c..4a726ef809551e 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -230,9 +230,11 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation>
-UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.
diff --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 27cda71989726d..acf6c8146d70d4 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -72,10 +72,10 @@ inline std::pair getDepthAndIndex(const 
NamedDecl *ND) {
 /// Retrieve the depth and index of an unexpanded parameter pack.
 inline std::pair
 getDepthAndIndex(UnexpandedParameterPack UPP) {
-  if (const auto *TTP = UPP.first.dyn_cast())
+  if (const auto *TTP = dyn_cast(UPP.first))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(cast(UPP.first));
+  return getDepthAndIndex(cast(UPP.first));
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index fc09d24fc30cb4..715b819bfb96bb 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1734,7 +1734,7 @@ PackIndexingExpr *PackIndexingExpr::Create(
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast(getPackIdExpression()); D) {
 NamedDecl *ND = dyn_cast(D->getDecl());
-assert(ND && "exected a named decl");
+assert(ND && "expected a named decl");
 return ND;
   }
   assert(false && "invalid declaration kind in pack indexing expression");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index c5a72cf812ebc9..bf5f31c3a5ca4c 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17496,7 +17496,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
   unsigned FriendDeclDepth = TempParamLists.front()->getDepth();
   for (UnexpandedParameterPack &U : Unexpanded) {
 if (getDepthAndIndex(U).first >= FriendDeclDepth) {
-  auto *ND = U.first.dyn_cast();
+  auto *ND = dyn_cast(U.first);
   if (!ND)
 ND = cast(U.first)->getDecl();
   Diag(U.second, diag::friend_template_decl_malformed_pack_expansion)
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index fad20b37a7d9a2..1b541aa665a71a 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -868,8 +868,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (isa(U.first))
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index c8452db6bc9014..b9e52080d62a96 100644
--- a/clang/lib/Sema/SemaTemplateVariadic