[clang] Reapply "[Clang] Improve diagnostics for expansion length mismatch" (PR #121044)
@@ -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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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