diff mbox

[C++] PR 58650

Message ID 549070AE.50808@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Dec. 16, 2014, 5:49 p.m. UTC 
Hi,

On 12/16/2014 04:10 PM, Jason Merrill wrote:
> On 12/16/2014 05:40 AM, Paolo Carlini wrote:
>> In better detail: grokdeclarator is called, via grokfield, by
>> cp_parser_member_declaration. The latter stores the friendship
>> information in a friend_p local flag, which remains true when
>> grokdeclarator returns.
>
> Maybe check function_declarator_p in cp_parser_member_declaration?
I see what you mean: try to somehow realize that grokdeclarator issued 
an error and we are in error recovery. What about directly addressing 
NSDMIs, the specific case at issue, thus the below? A bit ad-hoc-ish but 
on the other hand should be lighter than calling function_declarator_p, 
etc, to figure out...

Well, I'd like to add a consideration in favor of the simplistic 
solution involving error_mark_node which I proposed earlier today: it 
has the advantage that we end up with consistent diagnostic for

struct A
{
     friend int i;

     A() { i = 1; }
};

and

struct B
{
     friend int i = 0;

     B() { i = 1; }
};

as far as the constructor is concerned: that is, 'i' is diagnosed as 
undeclared in both cases. This is, apparently, what both current clang 
and edg do. All the other solutions I figured out so far, so to speak 
"recover too much" in case of NSDMIs, that is beyond what we do for 
normal data members: no diagnostic is produced for the constructor in B.

Thanks,
Paolo.

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

Comments

Jason Merrill Dec. 16, 2014, 6:18 p.m. UTC | #1 
On 12/16/2014 12:49 PM, Paolo Carlini wrote:
> I see what you mean: try to somehow realize that grokdeclarator issued
> an error and we are in error recovery. What about directly addressing
> NSDMIs, the specific case at issue, thus the below? A bit ad-hoc-ish but
> on the other hand should be lighter than calling function_declarator_p,
> etc, to figure out...

Well, since only functions can be friends at this point, since types 
take a different path, maybe change the if (!friend_p) just above to
if (!friend_p || TREE_CODE (decl) != FUNCTION_DECL)?

Jason
diff mbox

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 218777)
+++ cp/parser.c	(working copy)
@@ -21077,8 +21077,16 @@  cp_parser_member_declaration (cp_parser* parser)
 	      else if (TREE_CODE (decl) == FIELD_DECL
 		       && !DECL_C_BIT_FIELD (decl)
 		       && DECL_INITIAL (decl))
-		/* Add DECL to the queue of NSDMI to be parsed later.  */
-		vec_safe_push (unparsed_nsdmis, decl);
+		{
+
+		  if (friend_p)
+		    /* Explicitly include NSDMIs for error recovery purposes:
+		       avoid crashing later if one is wrongly declared friend
+		       (c++/58650).  */
+		    finish_member_declaration (decl);
+		  /* Add DECL to the queue of NSDMI to be parsed later.  */
+		  vec_safe_push (unparsed_nsdmis, decl);
+		}
 	    }
 
 	  if (assume_semicolon)
Index: testsuite/g++.dg/parse/friend12.C
===================================================================
--- testsuite/g++.dg/parse/friend12.C	(revision 0)
+++ testsuite/g++.dg/parse/friend12.C	(working copy)
@@ -0,0 +1,7 @@ 
+// PR c++/58650
+
+struct A
+{
+  friend int i = 0;  // { dg-error "cannot be declared friend" }
+// { dg-error "non-static data member" "" { target { ! c++11 } } 5 }
+};