[C++] Improve start_function and grokmethod locations

Hi,

apparently this is now easy to do, likely because a while ago I made 
sure that we consistently have meaningful locations for TYPE_DECLs too.

(I went through grokdeclarator and confirmed that when the third 
argument is FUNCDEF or MEMFUNCDEF it cannot return NULL_TREE)

Tested x86_64-linux.

Thanks, Paolo.

////////////////////////
/cp
2019-08-06  Paolo Carlini  <paolo.carlini@oracle.com>

	* decl.c (start_function): Use DECL_SOURCE_LOCATION.
	(grokmethod): Likewise.

/testsuite
2019-08-06  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/parse/typedef9.C: Test locations too.

On 8/6/19 8:28 AM, Paolo Carlini wrote:
> apparently this is now easy to do, likely because a while ago I made 
> sure that we consistently have meaningful locations for TYPE_DECLs too.
> 
> (I went through grokdeclarator and confirmed that when the third 
> argument is FUNCDEF or MEMFUNCDEF it cannot return NULL_TREE)

> -typedef void foo () {}	// { dg-error "invalid function declaration" }
> +typedef void foo () {}	// { dg-error "14:invalid function declaration" }
>  struct S
>  {
> -  typedef int bar (void) { return 0; } // { dg-error "invalid member function declaration" }
> +  typedef int bar (void) { return 0; }  // { dg-error "15:invalid member function declaration" }

Maybe we could give a more specific diagnostic in grokdeclarator; 
perhaps under

>   if (typedef_p && decl_context != TYPENAME)

complain and return error_mark_node in FUNCDEF or MEMFUNCDEF context.

Jason

Hi,

On 08/08/19 16:51, Jason Merrill wrote:
> On 8/6/19 8:28 AM, Paolo Carlini wrote:
>> apparently this is now easy to do, likely because a while ago I made 
>> sure that we consistently have meaningful locations for TYPE_DECLs too.
>>
>> (I went through grokdeclarator and confirmed that when the third 
>> argument is FUNCDEF or MEMFUNCDEF it cannot return NULL_TREE)
>
>> -typedef void foo () {}    // { dg-error "invalid function 
>> declaration" }
>> +typedef void foo () {}    // { dg-error "14:invalid function 
>> declaration" }
>>  struct S
>>  {
>> -  typedef int bar (void) { return 0; } // { dg-error "invalid member 
>> function declaration" }
>> +  typedef int bar (void) { return 0; }  // { dg-error "15:invalid 
>> member function declaration" }
>
> Maybe we could give a more specific diagnostic in grokdeclarator; 
> perhaps under
>
>>   if (typedef_p && decl_context != TYPENAME)
>
> complain and return error_mark_node in FUNCDEF or MEMFUNCDEF context.

Yes, I briefly considered that myself, I only stopped because 
grokdeclarator seemed big enough already ;)

Anyway, I tested on x86_64-linux the below. Not 100% sure about the 
wording, but we have something similar a few lines below. Also, probably 
a single error_at both for functions and member functions would be good 
enough (but it would be a specificity regression).

Thanks, Paolo.

////////////////////////
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 274220)
+++ cp/decl.c	(working copy)
@@ -12165,6 +12165,17 @@ grokdeclarator (const cp_declarator *declarator,
       bool alias_p = decl_spec_seq_has_spec_p (declspecs, ds_alias);
       tree decl;
 
+      if (funcdef_flag)
+	{
+	  if (decl_context == NORMAL)
+	    error_at (id_loc,
+		      "typedef may not be a function definition");
+	  else
+	    error_at (id_loc,
+		      "typedef may not be a member function definition");
+	  return error_mark_node;
+	}
+
       /* This declaration:
 
 	   typedef void f(int) const;
@@ -15775,13 +15786,6 @@ start_function (cp_decl_specifier_seq *declspecs,
   invoke_plugin_callbacks (PLUGIN_START_PARSE_FUNCTION, decl1);
   if (decl1 == error_mark_node)
     return false;
-  /* If the declarator is not suitable for a function definition,
-     cause a syntax error.  */
-  if (decl1 == NULL_TREE || TREE_CODE (decl1) != FUNCTION_DECL)
-    {
-      error ("invalid function declaration");
-      return false;
-    }
 
   if (DECL_MAIN_P (decl1))
     /* main must return int.  grokfndecl should have corrected it
@@ -16436,12 +16440,6 @@ grokmethod (cp_decl_specifier_seq *declspecs,
   if (fndecl == error_mark_node)
     return error_mark_node;
 
-  if (fndecl == NULL || TREE_CODE (fndecl) != FUNCTION_DECL)
-    {
-      error ("invalid member function declaration");
-      return error_mark_node;
-    }
-
   if (attrlist)
     cplus_decl_attributes (&fndecl, attrlist, 0);
 
Index: testsuite/g++.dg/parse/typedef9.C
===================================================================
--- testsuite/g++.dg/parse/typedef9.C	(revision 274218)
+++ testsuite/g++.dg/parse/typedef9.C	(working copy)
@@ -1,8 +1,8 @@
 // PR c++/38794
 // { dg-do compile }
 
-typedef void foo () {}	// { dg-error "invalid function declaration" }
+typedef void foo () {}	// { dg-error "14:typedef may not be a function definition" }
 struct S
 {
-  typedef int bar (void) { return 0; } // { dg-error "invalid member function declaration" }
+  typedef int bar (void) { return 0; } // { dg-error "15:typedef may not be a member function definition" }
 };

On 8/9/19 7:46 AM, Paolo Carlini wrote:
> +	    error_at (id_loc,
> +		      "typedef may not be a function definition");

We might use the location of the typedef keyword.  OK either way.

Jason