| Message ID | 20180604154458.GD23949@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | C++ PATCH for c++/85976, ICE with USING_DECL in cp_tree_equal | expand |
On Mon, Jun 4, 2018 at 11:44 AM, Marek Polacek <polacek@redhat.com> wrote: > I've had no luck in reducing the testcase in this PR, creduce won't get even > past the initial passes, and reducing by hand didn't get me very far, either. > > But the problem seems to be merely that we're not handling USING_DECLs in > cp_tree_equal, and we can get there via comp_template_arguments. In this case > we have two USING_DECLs with different full names. > > So this patch just adds the USING_DECL case, similarly to e.g. > https://gcc.gnu.org/ml/gcc-patches/2012-10/msg00799.html > > Bootstrapped/regtested on x86_64-linux, ok for trunk/8? I verified manually > that this fixes the testcase from the PR. Hmm, do these USING_DECLs have DECL_DEPENDENT_P set? What do they represent? In the case of dependent USING_DECL I'd think we want to compare the scope and name rather than just return false. I think we do want a reduced testcase. Maybe add the needs-reduction tag if you're having trouble reducing it yourself? Jason
On Mon, Jun 04, 2018 at 01:28:01PM -0400, Jason Merrill wrote: > On Mon, Jun 4, 2018 at 11:44 AM, Marek Polacek <polacek@redhat.com> wrote: > > I've had no luck in reducing the testcase in this PR, creduce won't get even > > past the initial passes, and reducing by hand didn't get me very far, either. > > > > But the problem seems to be merely that we're not handling USING_DECLs in > > cp_tree_equal, and we can get there via comp_template_arguments. In this case > > we have two USING_DECLs with different full names. > > > > So this patch just adds the USING_DECL case, similarly to e.g. > > https://gcc.gnu.org/ml/gcc-patches/2012-10/msg00799.html > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk/8? I verified manually > > that this fixes the testcase from the PR. > > Hmm, do these USING_DECLs have DECL_DEPENDENT_P set? What do they > represent? In the case of dependent USING_DECL I'd think we want to > compare the scope and name rather than just return false. They represent a using-declaration in this testcase. Yep, they're DECL_DEPENDENT_P. So let's check their scope and name. > I think we do want a reduced testcase. Maybe add the needs-reduction > tag if you're having trouble reducing it yourself? I got lucky: I managed to remove some random unnecessary code which then allowed creduce to do its job, the result is pretty nice! Bootstrapped/regtested on x86_64-linux, ok for trunk/8? 2018-06-04 Marek Polacek <polacek@redhat.com> PR c++/85976 * tree.c (cp_tree_equal): Handle USING_DECL. * g++.dg/cpp0x/alias-decl-64.C: New test. diff --git gcc/cp/tree.c gcc/cp/tree.c index c5b6e9689b6..bbbda7e98b6 100644 --- gcc/cp/tree.c +++ gcc/cp/tree.c @@ -3878,6 +3878,14 @@ cp_tree_equal (tree t1, tree t2) DEFERRED_NOEXCEPT_ARGS (t2))); break; + case USING_DECL: + if (DECL_DEPENDENT_P (t1) && DECL_DEPENDENT_P (t2)) + return (cp_tree_equal (USING_DECL_SCOPE (t1), + USING_DECL_SCOPE (t2)) + && cp_tree_equal (DECL_NAME (t1), + DECL_NAME (t2))); + return false; + default: break; } diff --git gcc/testsuite/g++.dg/cpp0x/alias-decl-64.C gcc/testsuite/g++.dg/cpp0x/alias-decl-64.C index e69de29bb2d..019eb269750 100644 --- gcc/testsuite/g++.dg/cpp0x/alias-decl-64.C +++ gcc/testsuite/g++.dg/cpp0x/alias-decl-64.C @@ -0,0 +1,15 @@ +// PR c++/85976 +// { dg-do compile { target c++11 } } + +template <int> class A; +template <typename> class B; +template <typename> struct C; +template <typename P_expr> class D { + using B<typename P_expr::T_numtype>::rank_; + void operator()(typename C<A<rank_>>::i); +}; + +template <typename P_expr> class F { + using B<typename P_expr::T_numtype>::rank_; + void operator()(typename C<A<rank_>>::i); +}; Marek
On Mon, Jun 4, 2018 at 11:03 PM, Marek Polacek <polacek@redhat.com> wrote: > On Mon, Jun 04, 2018 at 01:28:01PM -0400, Jason Merrill wrote: >> On Mon, Jun 4, 2018 at 11:44 AM, Marek Polacek <polacek@redhat.com> wrote: >> > I've had no luck in reducing the testcase in this PR, creduce won't get even >> > past the initial passes, and reducing by hand didn't get me very far, either. >> > >> > But the problem seems to be merely that we're not handling USING_DECLs in >> > cp_tree_equal, and we can get there via comp_template_arguments. In this case >> > we have two USING_DECLs with different full names. >> > >> > So this patch just adds the USING_DECL case, similarly to e.g. >> > https://gcc.gnu.org/ml/gcc-patches/2012-10/msg00799.html >> > >> > Bootstrapped/regtested on x86_64-linux, ok for trunk/8? I verified manually >> > that this fixes the testcase from the PR. >> >> Hmm, do these USING_DECLs have DECL_DEPENDENT_P set? What do they >> represent? In the case of dependent USING_DECL I'd think we want to >> compare the scope and name rather than just return false. > > They represent a using-declaration in this testcase. > > Yep, they're DECL_DEPENDENT_P. So let's check their scope and name. > >> I think we do want a reduced testcase. Maybe add the needs-reduction >> tag if you're having trouble reducing it yourself? > > I got lucky: I managed to remove some random unnecessary code which then > allowed creduce to do its job, the result is pretty nice! > > Bootstrapped/regtested on x86_64-linux, ok for trunk/8? > > 2018-06-04 Marek Polacek <polacek@redhat.com> > > PR c++/85976 > * tree.c (cp_tree_equal): Handle USING_DECL. > > * g++.dg/cpp0x/alias-decl-64.C: New test. > > diff --git gcc/cp/tree.c gcc/cp/tree.c > index c5b6e9689b6..bbbda7e98b6 100644 > --- gcc/cp/tree.c > +++ gcc/cp/tree.c > @@ -3878,6 +3878,14 @@ cp_tree_equal (tree t1, tree t2) > DEFERRED_NOEXCEPT_ARGS (t2))); > break; > > + case USING_DECL: > + if (DECL_DEPENDENT_P (t1) && DECL_DEPENDENT_P (t2)) > + return (cp_tree_equal (USING_DECL_SCOPE (t1), > + USING_DECL_SCOPE (t2)) > + && cp_tree_equal (DECL_NAME (t1), > + DECL_NAME (t2))); > + return false; > + > default: > break; > } > diff --git gcc/testsuite/g++.dg/cpp0x/alias-decl-64.C gcc/testsuite/g++.dg/cpp0x/alias-decl-64.C > index e69de29bb2d..019eb269750 100644 > --- gcc/testsuite/g++.dg/cpp0x/alias-decl-64.C > +++ gcc/testsuite/g++.dg/cpp0x/alias-decl-64.C > @@ -0,0 +1,15 @@ > +// PR c++/85976 > +// { dg-do compile { target c++11 } } > + > +template <int> class A; > +template <typename> class B; > +template <typename> struct C; > +template <typename P_expr> class D { > + using B<typename P_expr::T_numtype>::rank_; > + void operator()(typename C<A<rank_>>::i); > +}; > + > +template <typename P_expr> class F { > + using B<typename P_expr::T_numtype>::rank_; > + void operator()(typename C<A<rank_>>::i); > +}; Hmm, this testcase looks a bit over-reduced; these templates can never be instantiated, so they are ill-formed (no diagnostic required). But I suppose that's not too much of an issue. The patch is OK. Jason
--- gcc/cp/tree.c +++ gcc/cp/tree.c @@ -3752,6 +3752,7 @@ cp_tree_equal (tree t1, tree t2) case TEMPLATE_DECL: case IDENTIFIER_NODE: case SSA_NAME: + case USING_DECL: return false; case BASELINK: