On Wed, 1 Jan 2025, A J Ryan Solutions Ltd wrote:
Hi and happy new year, this patch is to fix a compiler seg-fault as encountered
in the following example:
Hi, thanks for the patch! Your fix makes sense to me, and I believe it
also fixes the testcases from PR102626 and at least some of its related
PRs.
template<class> struct Class1{};
template<auto...Tn> class Class2;
template<typename...V,Class1<V>...Un> class Class2<Un...>
{
public: void apply(){}
};
Class1<bool> class1_bool;
Class1<char> class1_char;
int main()
{
Class2<class1_bool,class1_char> class2;
class2.apply();
}
where it ends up populating the V argument in the instantiated template at the
class2 declaration with the unknown placeholder and the segfault is when it
later tries to get the name for an incompletely
resolved type.
Here's a more reduced example that's fixed by your patch:
template<auto> struct A { };
template<class... Ts, Ts... Vs>
void f(A<Vs>...);
int main() {
f(A<0>{}, A<1u>{}, A<2l>{}); // OK, Ts deduced to {int,unsigned,long}
}
A related PR that isn't and shouldn't be fixed by this patch is PR84796
where the type parameter pack is declared at a different level than
the non-type parameter pack, e.g.:
template<auto...>
struct A { };
template<class... Ts>
struct B {
template<Ts... Vs>
static void f(A<Vs...>);
};
int main() {
A<1, true, 'c'> a;
B<int, bool, char>::f(a);
}
Before your patch we would ICE on this valid testcase, after we now reject it
with a bogus error:
<stdin>: In function ‘int main()’:
<stdin>:12:24: error: no matching function for call to ‘B<int, bool, char>::f(A<1,
true, 'c'>&)’
<stdin>:12:24: note: there is 1 candidate
<stdin>:7:15: note: candidate 1: ‘template<Ts ...Vs> static void B<Ts>::f(A<Vs
...>) [with Ts ...Vs = {Vs ...}; Ts = {int, bool, char}]’
<stdin>:7:15: note: template argument deduction/substitution failed:
<stdin>:12:24: note: ‘Vs’ is not equivalent to ‘1’
Which is no worse than ICEing, I suppose :)
The incorrect processing happens in unify_pack_expansion where it is calling
unify_one_argument
here(https://gcc.gnu.org/git?p=gcc.git;a=blob;f=gcc/cp/pt.cc;h=0ffa0b53e26a87df3ed2c7202446c68d2800db64;hb=HEAD#l24718)
for each argument of the argument pack and recursively unifying the inner
arguments. In
this example the inner argument against V (bool/char) is set in targs however
it is not seen as a pack argument in unify_pack_expansion because in the
unfixed code it does not reach and link this associated
pack and later on when processing the collected arguments to substitute it in
in instantiate_class_template the argument has not been set as an argument pack
in targs and it doesn't match the the template
specialisation parameter which is a parameter pack and it falls back to the
unknown placeholder.
The parameter packs are originally linked in make_pack_expansion
here(https://gcc.gnu.org/git?p=gcc.git;a=blob;f=gcc/cp/pt.cc;h=0ffa0b53e26a87df3ed2c7202446c68d2800db64;hb=HEAD#l4246)
and the change is in
this tree walk so that in the example above V is chained to Un and after
calling unify_one_argument it loops over both packs and extracts their
associated value set in targs and puts it into it's pack
argument collection.
Regards
Adam Ryan
Subject: [PATCH] Add tree walk case to obtain A pack in
<typename...A,Class<A>...B>.
For non-type parameter packs when unifying the arguments in unify_pack_expansion
it iterates over the associated packs of a param so that when it recursivly
unifies the param with the arguments it knows which targs have been populated
with parameter pack arguments that it can then collect up. This change adds a
tree walk so that in the example above it reaches ...A and adds it to the
associated packs for ...B and therefore knows it will have been set in targs
in unify_pack_expansion and processes it as per other pack arguments.
PR gcc/118265
PR c++/118265 rather
A ChangeLog entry is missing from the commit message as per
https://gcc.gnu.org/contribute.html#patches
Here's an example ChangeLog we can use for this patch:
gcc/cp/ChangeLog:
* pt.cc (find_parameter_packs_r) <case TEMPLATE_PARM_INDEX>:
Walk into the type of a parameter pack.
Signed-off-by: Adam J Ryan <gcc.gnu....@ajryansolutions.co.uk>
---
gcc/cp/pt.cc | 5 +++++
gcc/testsuite/g++.dg/pr118265.C | 17 +++++++++++++++++
2 files changed, 22 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/pr118265.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 0ffa0b53e26..22f5d4b1875 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -4012,6 +4012,11 @@ find_parameter_packs_r (tree *tp, int *walk_subtrees,
void* data)
&find_parameter_packs_r, ppd, ppd->visited);
return NULL_TREE;
+ case TEMPLATE_PARM_INDEX:
+ if (parameter_pack_p)
+ WALK_SUBTREE ( TREE_TYPE(t) );
There should be a space before the ( starting a function call not
after (as per the GNU coding style), so this should be
"WALK_SUBTREE (TREE_TYPE (t));"
+ return NULL_TREE;
+
case DECL_EXPR:
{
tree decl = DECL_EXPR_DECL (t);
diff --git a/gcc/testsuite/g++.dg/pr118265.C b/gcc/testsuite/g++.dg/pr118265.C
new file mode 100644
index 00000000000..00bda1b5b48
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr118265.C
@@ -0,0 +1,17 @@
+// { dg-do compile { target c++20 } }
+template<class> struct Class1{};
+
+template<auto...Tn> class Class2;
+template<typename...V,Class1<V>...Un> class Class2<Un...>
+{
+ public: void apply(){}
+};
+
+Class1<bool> class1_bool;
+Class1<char> class1_char;
+
+int main()
+{
+ Class2<class1_bool,class1_char> class2;
+ class2.apply();
+}
\ No newline at end of file
--
2.43.0
