| Message ID | 570CF748.9040806@oracle.com |
|---|---|
| State | New |
| Headers | show |
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. Jason
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. 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. Thus same_type_p should be fixed to simply return false in such recursive cases without ICEing and nothing more, right? I'm not sure I will be able to do that in time for 6.0 but I'm going to try... Thanks, Paolo.
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. > Thus same_type_p should be fixed to simply return false in such > recursive cases without ICEing and nothing more, right? I'm not sure I > will be able to do that in time for 6.0 but I'm going to try... resolve_typename_type should give up in such cases. Jason
Index: cp/parser.c =================================================================== --- cp/parser.c (revision 234874) +++ cp/parser.c (working copy) @@ -25118,8 +25122,9 @@ cp_parser_constructor_declarator_p (cp_parser *par constructor_p = (!cp_parser_error_occurred (parser) && (outside_class_specifier_p || type_decl == error_mark_node - || same_type_p (current_class_type, - TREE_TYPE (type_decl)))); + || (CLASS_TYPE_P (TREE_TYPE (type_decl)) + && same_type_p (current_class_type, + TREE_TYPE (type_decl))))); /* If we're still considering a constructor, we have to see a `(', to begin the parameter-declaration-clause, followed by either a Index: testsuite/g++.dg/parse/pr70635.C =================================================================== --- testsuite/g++.dg/parse/pr70635.C (revision 0) +++ testsuite/g++.dg/parse/pr70635.C (working copy) @@ -0,0 +1,23 @@ +// PR c++/70635 +// { dg-options "-fpermissive -w" } + +template < typename T > +struct A +{ + struct B; + typedef typename B::type type; +}; + +template < typename T > +struct A < T >::B +{ + typedef typename A < type >::type type; + type Foo (); +}; + +template < typename T > +typename A < T >::B::type +A < T >::B::Foo () +{ + return 0; +}