https://github.com/zyn0217 updated
https://github.com/llvm/llvm-project/pull/120380
>From 3dd62b0dc3d913b76072c4dc0a7c0d172fe9d529 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7...@gmail.com>
Date: Wed, 18 Dec 2024 16:22:15 +0800
Subject: [PATCH 1/2] [Clang] Don't assume unexpanded PackExpansions' size when
expanding packs
---
.../clang/Basic/DiagnosticSemaKinds.td | 4 +-
clang/lib/Sema/SemaTemplateVariadic.cpp | 36 +++++++++--
clang/test/SemaTemplate/pack-deduction.cpp | 59 +++++++++++++++++++
3 files changed, 91 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 811265151fa0da..762e01dbaa857c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5863,10 +5863,10 @@ def err_pack_expansion_without_parameter_packs : Error<
"pack expansion does not contain any unexpanded parameter packs">;
def err_pack_expansion_length_conflict : Error<
"pack expansion contains parameter packs %0 and %1 that have different "
- "lengths (%2 vs. %3)">;
+ "lengths (%2 vs. %select{|at least }3%4))">;
def err_pack_expansion_length_conflict_multilevel : Error<
"pack expansion contains parameter pack %0 that has a different "
- "length (%1 vs. %2) from outer parameter packs">;
+ "length (%1 vs. %select{|at least }2%3) from outer parameter packs">;
def err_pack_expansion_length_conflict_partial : Error<
"pack expansion contains parameter pack %0 that has a different "
"length (at least %1 vs. %2) from outer parameter packs">;
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 88a21240e1c800..2f1dc3d7efd626 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -780,7 +780,7 @@ bool Sema::CheckParameterPacksForExpansion(
}
// Determine the size of this argument pack.
- unsigned NewPackSize;
+ unsigned NewPackSize, PendingPackExpansionSize = 0;
if (IsVarDeclPack) {
// Figure out whether we're instantiating to an argument pack or not.
typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
@@ -808,7 +808,25 @@ bool Sema::CheckParameterPacksForExpansion(
}
// Determine the size of the argument pack.
- NewPackSize = TemplateArgs(Depth, Index).pack_size();
+ ArrayRef<TemplateArgument> Pack =
+ TemplateArgs(Depth, Index).getPackAsArray();
+ NewPackSize = Pack.size();
+ PendingPackExpansionSize =
+ llvm::count_if(Pack, [](const TemplateArgument &TA) {
+ if (!TA.isPackExpansion())
+ return false;
+
+ if (TA.getKind() == TemplateArgument::Type)
+ return !TA.getAsType()
+ ->getAs<PackExpansionType>()
+ ->getNumExpansions();
+
+ if (TA.getKind() == TemplateArgument::Expression)
+ return !cast<PackExpansionExpr>(TA.getAsExpr())
+ ->getNumExpansions();
+
+ return !TA.getNumTemplateExpansions();
+ });
}
// C++0x [temp.arg.explicit]p9:
@@ -831,7 +849,7 @@ bool Sema::CheckParameterPacksForExpansion(
}
if (!NumExpansions) {
- // The is the first pack we've seen for which we have an argument.
+ // This is the first pack we've seen for which we have an argument.
// Record it.
NumExpansions = NewPackSize;
FirstPack.first = Name;
@@ -841,17 +859,23 @@ bool Sema::CheckParameterPacksForExpansion(
}
if (NewPackSize != *NumExpansions) {
+ unsigned LeastNewPackSize = NewPackSize - PendingPackExpansionSize;
+ if (PendingPackExpansionSize && LeastNewPackSize <= *NumExpansions) {
+ 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 << NewPackSize
+ << 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 << NewPackSize
- << SourceRange(ParmPack.second);
+ << Name << *NumExpansions << (LeastNewPackSize != NewPackSize)
+ << LeastNewPackSize << SourceRange(ParmPack.second);
return true;
}
}
diff --git a/clang/test/SemaTemplate/pack-deduction.cpp
b/clang/test/SemaTemplate/pack-deduction.cpp
index 28fb127a386441..b3104609994a4e 100644
--- a/clang/test/SemaTemplate/pack-deduction.cpp
+++ b/clang/test/SemaTemplate/pack-deduction.cpp
@@ -199,3 +199,62 @@ constexpr auto baz(Int<foo<T>(T())>... x) -> int { return
1; }
static_assert(baz<Int<1>, Int<2>, Int<3>>(Int<10>(), Int<10>(), Int<10>()) ==
1, "");
}
+
+namespace GH17042 {
+
+template <class... Ts> struct X {
+ template <class... Us> using Y = X<void(Ts, Us)...>; // #GH17042_Y
+};
+
+template <class... T>
+using any_pairs_list = X<int, int>::Y<T...>; // #any_pairs_list
+
+template <class... T>
+using any_pairs_list_2 = X<int, int>::Y<>;
+// expected-error@#GH17042_Y {{different length (2 vs. 0)}} \
+// expected-note@-1 {{requested here}}
+
+template <class A, class B, class... P>
+using any_pairs_list_3 = X<int, int>::Y<A, B, P...>; // #any_pairs_list_3
+
+template <class A, class B, class C, class... P>
+using any_pairs_list_4 = X<int, int>::Y<A, B, C, P...>;
+// expected-error@#GH17042_Y {{different length (2 vs. at least 3)}} \
+// expected-note@-1 {{requested here}}
+
+static_assert(__is_same(any_pairs_list<char, char>, X<void(int, char),
void(int, char)>), "");
+
+static_assert(!__is_same(any_pairs_list<char, char, char>, X<void(int, char),
void(int, char)>), "");
+// expected-error@#GH17042_Y {{different length (2 vs. 3)}} \
+// expected-note@#any_pairs_list {{requested here}} \
+// expected-note@-1 {{requested here}}
+
+static_assert(__is_same(any_pairs_list_3<char, char>, X<void(int, char),
void(int, char)>), "");
+
+static_assert(!__is_same(any_pairs_list_3<char, char, float>, X<void(int,
char), void(int, char)>), "");
+// expected-error@#GH17042_Y {{different length (2 vs. 3)}} \
+// expected-note@#any_pairs_list_3 {{requested here}} \
+// expected-note@-1 {{requested here}}
+
+namespace TemplateTemplateParameters {
+template <class... T> struct C {};
+
+template <class T, template <class> class... Args1> struct Ttp {
+ template <template <class> class... Args2>
+ using B = C<void(Args1<T>, Args2<T>)...>;
+};
+template <class> struct D {};
+
+template <template <class> class... Args>
+using Alias = Ttp<int, D, D>::B<Args...>;
+}
+
+namespace NTTP {
+template <int... Args1> struct Nttp {
+ template <int... Args2> using B = Nttp<(Args1 + Args2)...>;
+};
+
+template <int... Args> using Alias = Nttp<1, 2, 3>::B<Args...>;
+}
+
+}
>From b2d554d09ec55cd750c3f59a0e91d14685441c88 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7...@gmail.com>
Date: Wed, 18 Dec 2024 17:59:09 +0800
Subject: [PATCH 2/2] add a release note and comments
---
clang/docs/ReleaseNotes.rst | 1 +
clang/lib/Sema/SemaTemplateVariadic.cpp | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index befa411e882b4c..10ed4cf3c964a1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -825,6 +825,7 @@ Bug Fixes to C++ Support
- Clang no longer rejects deleting a pointer of incomplete enumeration type.
(#GH99278)
- Fixed recognition of ``std::initializer_list`` when it's surrounded with
``extern "C++"`` and exported
out of a module (which is the case e.g. in MSVC's implementation of ``std``
module). (#GH118218)
+- Fixed a pack expansion issue in checking unexpanded parameter sizes.
(#GH17042)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 2f1dc3d7efd626..c8452db6bc9014 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -859,6 +859,27 @@ bool Sema::CheckParameterPacksForExpansion(
}
if (NewPackSize != *NumExpansions) {
+ // In some cases, we might be handling packs with unexpanded template
+ // arguments. For example, this can occur when substituting into a type
+ // alias declaration that uses its injected template parameters as
+ // arguments:
+ //
+ // template <class... Outer> struct S {
+ // template <class... Inner> using Alias = S<void(Outer, Inner)...>;
+ // };
+ //
+ // Consider an instantiation attempt like 'S<int>::Alias<Pack...>', where
+ // Pack comes from another template parameter. 'S<int>' is first
+ // instantiated, expanding the outer pack 'Outer' to <int>. The alias
+ // declaration is accordingly substituted, leaving the template arguments
+ // as unexpanded
+ // '<Pack...>'.
+ //
+ // Since we have no idea of the size of '<Pack...>' 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) {
ShouldExpand = false;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
