[clang] 5c55f96 - [Clang] Don't assume unexpanded PackExpansions' size when expanding packs (#120380)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 18 21:12:05 PST 2024
Author: Younan Zhang
Date: 2024-12-19T13:12:01+08:00
New Revision: 5c55f9664f7e2f9fe29589a97bc5818d6b8d3c9c
URL: https://github.com/llvm/llvm-project/commit/5c55f9664f7e2f9fe29589a97bc5818d6b8d3c9c
DIFF: https://github.com/llvm/llvm-project/commit/5c55f9664f7e2f9fe29589a97bc5818d6b8d3c9c.diff
LOG: [Clang] Don't assume unexpanded PackExpansions' size when expanding packs (#120380)
CheckParameterPacksForExpansion() previously assumed that template
arguments don't include PackExpansion types when attempting another pack
expansion (i.e. when NumExpansions is present). However, this assumption
doesn't hold for type aliases, whose substitution might involve
unexpanded packs. This can lead to incorrect diagnostics during
substitution because the pack size is not yet determined.
To address this, this patch calculates the minimum pack size (ignoring
unexpanded PackExpansionTypes) and compares it to the previously
expanded size. If the minimum pack size is smaller, then there's still a
chance for future substitution to expand it to a correct size, so we
don't diagnose it too eagerly.
Fixes #61415
Fixes #32252
Fixes #17042
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaTemplateVariadic.cpp
clang/test/SemaTemplate/pack-deduction.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 29794f27d30057..5f91ff90634036 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -850,6 +850,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/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d67a81f8564a8e..7bd154e7da2f4c 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
diff erent "
- "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
diff erent "
- "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
diff erent "
"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..c8452db6bc9014 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,44 @@ 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;
+ 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 {{
diff erent length (2 vs. 0)}} \
+// expected-note at -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 {{
diff erent length (2 vs. at least 3)}} \
+// expected-note at -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 {{
diff erent length (2 vs. 3)}} \
+// expected-note@#any_pairs_list {{requested here}} \
+// expected-note at -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 {{
diff erent length (2 vs. 3)}} \
+// expected-note@#any_pairs_list_3 {{requested here}} \
+// expected-note at -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...>;
+}
+
+}
More information about the cfe-commits
mailing list