Re: [C++ Patch] PR 70635 ([4.9/5/6 Regression] ICE on (and rejects) valid code...)

[Paolo Carlini <paolo dot carlini at oracle dot com>]

Hi again,

On 13/04/2016 16:42, Jason Merrill wrote:
> On 04/13/2016 03:36 AM, Paolo Carlini wrote:
> > Hi,
> >
> > On 12/04/2016 15:50, Jason Merrill wrote:
> > > On 04/12/2016 09:25 AM, Paolo Carlini wrote:
> > > > in this regression we have an infinite recursion affecting the
> > > > same_type_p call at parser.c:25125 which I added in the patch for
> > > > c++/38313. The issue is that for, eg, the testcase at issue, we are
> > > > passing a TYPENAME_TYPE to same_type_p. I think we can simply 
> > > > handle the
> > > > problem by checking first that we have a CLASS_TYPE_P as TREE_TYPE
> > > > (type_decl). Tested x86_64-linux.
> > >
> > > I don't see how this can be considered valid code; the typenames are
> > > indeed mutually recursive, with no real underlying type anywhere.
> > > clang and EDG both reject the testcase if the template is
> > > instantiated. Please add an instantiation to the testcase. And we
> > > should fix the recursion issue in structural_comptypes.
>
> ...but your patch is OK; even if it doesn't fix the real problem, it 
> isn't wrong.  Only the testcase should be adjusted.
>
> > Ok. But then, if I understand correctly, our user-visible behavior
> > should not change much: accept the testcase (modulo the -fpermissive
> > detail) without an instantiation and reject it when the template is
> > actually instantiated. In other terms, exactly what EDG and clang also
> > do.
>
> The testcase without an instantiation is ill-formed, no diagnostic 
> required, so we shouldn't test for accepting it, we should only test 
> for rejecting the variant with an instantiation.
Agreed, now I really get your point: in the future certainly it would 
not be wrong to somehow provide diagnostic for a testcase without the 
instantiation too. A testcase with the instantiation is robust.

As it happens, anyway, the below exceedingly trivial patch avoids the 
ICE in resolve_typename_type and appears to pass testing (is already 
past g++.dg/tm/tm.exp...). Should I apply it instead together with the 
amended testcase? Weird that nobody noticed the pair of typos before!?!

Thanks,
Paolo.

//////////////////////

Attachment: p
Description: Text document