| Message ID | 20130403123600.GC4201@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 3, 2013 at 7:36 AM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > On the following testcase we ICE with -fcompare-debug with > --enable-checking=yes, because strip_typedefs copies args to a new TREE_VEC, > but doesn't copy over NON_DEFAULT_TEMPLATE_ARGS_COUNT. For ENABLE_CHECKING > the code requires that it is set, for !ENABLE_CHECKING it would be needed > only if NON_DEFAULT_TEMPLATE_ARGS_COUNT was already non-NULL (otherwise > GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT would use length of the TREE_VEC). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.8/4.7? > > 2013-04-03 Jakub Jelinek <jakub@redhat.com> > > PR debug/56819 > * tree.c (strip_typedefs): SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT > on new_args. > > * g++.dg/debug/pr56819.C: New test. > > --- gcc/cp/tree.c.jj 2013-04-02 20:24:34.000000000 +0200 > +++ gcc/cp/tree.c 2013-04-03 10:51:56.614548181 +0200 > @@ -1255,8 +1255,16 @@ strip_typedefs (tree t) > changed = true; > } > if (changed) > - fullname = lookup_template_function (TREE_OPERAND (fullname, 0), > - new_args); > + { > +#ifndef ENABLE_CHECKING > + if (NON_DEFAULT_TEMPLATE_ARGS_COUNT (args)) > +#endif > + SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT > + (new_args, GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args)); > + fullname > + = lookup_template_function (TREE_OPERAND (fullname, 0), > + new_args); > + } > else > ggc_free (new_args); Hmm, the resulting code does not look simpler. Why can't we always copy the stuff as opposed to playing cat-n-fish with the CPP macro ENABLE_CHECKING? -- Gaby
We should be able to just copy NON_DEFAULT_TEMPLATE_ARGS_COUNT over rather than mess with looking into and building an INTEGER_CST. Jason
On Wed, Apr 03, 2013 at 08:38:01AM -0500, Gabriel Dos Reis wrote: > > 2013-04-03 Jakub Jelinek <jakub@redhat.com> > > > > PR debug/56819 > > * tree.c (strip_typedefs): SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT > > on new_args. > > > > * g++.dg/debug/pr56819.C: New test. > > > > --- gcc/cp/tree.c.jj 2013-04-02 20:24:34.000000000 +0200 > > +++ gcc/cp/tree.c 2013-04-03 10:51:56.614548181 +0200 > > @@ -1255,8 +1255,16 @@ strip_typedefs (tree t) > > changed = true; > > } > > if (changed) > > - fullname = lookup_template_function (TREE_OPERAND (fullname, 0), > > - new_args); > > + { > > +#ifndef ENABLE_CHECKING > > + if (NON_DEFAULT_TEMPLATE_ARGS_COUNT (args)) > > +#endif > > + SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT > > + (new_args, GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args)); > > + fullname > > + = lookup_template_function (TREE_OPERAND (fullname, 0), > > + new_args); > > + } > > else > > ggc_free (new_args); > > Hmm, the resulting code does not look simpler. > Why can't we always copy the stuff as opposed to > playing cat-n-fish with the CPP macro ENABLE_CHECKING? I didn't want to slow it down in the common case (--enable-checking=release) and null NON_DEFAULT_TEMPLATE_ARGS_COUNT, in that case it would build_int_cst unnecessarily. If that isn't something we care about, why are we differentiating between ENABLE_CHECKING vs. !ENABLE_CHECKING for *NON_DEFAULT_TEMPLATE_ARGS_COUNT everywhere at all? Though, as INTEGER_CSTs should be shared, perhaps this could be just NON_DEFAULT_TEMPLATE_ARGS_COUNT (new_args) = NON_DEFAULT_TEMPLATE_ARGS_COUNT (args); and strip_typedefs_expr could be changed to do the same thing. Jakub
> Though, as INTEGER_CSTs should be shared, perhaps this could be just > NON_DEFAULT_TEMPLATE_ARGS_COUNT (new_args) > = NON_DEFAULT_TEMPLATE_ARGS_COUNT (args); Yes, thanks! > and strip_typedefs_expr could be changed to do the same thing.
--- gcc/cp/tree.c.jj 2013-04-02 20:24:34.000000000 +0200 +++ gcc/cp/tree.c 2013-04-03 10:51:56.614548181 +0200 @@ -1255,8 +1255,16 @@ strip_typedefs (tree t) changed = true; } if (changed) - fullname = lookup_template_function (TREE_OPERAND (fullname, 0), - new_args); + { +#ifndef ENABLE_CHECKING + if (NON_DEFAULT_TEMPLATE_ARGS_COUNT (args)) +#endif + SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT + (new_args, GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args)); + fullname + = lookup_template_function (TREE_OPERAND (fullname, 0), + new_args); + } else ggc_free (new_args); } --- gcc/testsuite/g++.dg/debug/pr56819.C.jj 2013-04-03 10:57:55.187410867 +0200 +++ gcc/testsuite/g++.dg/debug/pr56819.C 2013-04-03 10:56:42.000000000 +0200 @@ -0,0 +1,27 @@ +// PR debug/56819 +// { dg-do compile } +// { dg-options "-fcompare-debug" } + +template <typename> +struct A +{ + template <typename> + struct B; +}; + +template <typename> +struct C +{ + typedef int I; +}; + +template <typename T> +class D +{ + typedef A <void> E; + typedef typename T::template B <E> F; + typedef typename C <F>::I I; + A <I> foo () { return A<I> (); } +}; + +template class D <A <void> >;