[C++] Improve start_function and grokmethod locations

Message ID eba28653-62f3-7a00-f712-c45341ddf6ec@oracle.com
State New
Headers show
Series
  • [C++] Improve start_function and grokmethod locations
Related show

Commit Message

Paolo Carlini Aug. 6, 2019, 12:28 p.m.
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.

Comments

Jason Merrill Aug. 8, 2019, 2:51 p.m. | #1
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
Paolo Carlini Aug. 9, 2019, 11:46 a.m. | #2
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" }
 };
Jason Merrill Aug. 13, 2019, 2:51 p.m. | #3
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

Patch

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 274141)
+++ cp/decl.c	(working copy)
@@ -15774,9 +15774,10 @@  start_function (cp_decl_specifier_seq *declspecs,
     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)
+  if (TREE_CODE (decl1) != FUNCTION_DECL)
     {
-      error ("invalid function declaration");
+      error_at (DECL_SOURCE_LOCATION (decl1),
+		"invalid function declaration");
       return false;
     }
 
@@ -16433,9 +16434,10 @@  grokmethod (cp_decl_specifier_seq *declspecs,
   if (fndecl == error_mark_node)
     return error_mark_node;
 
-  if (fndecl == NULL || TREE_CODE (fndecl) != FUNCTION_DECL)
+  if (TREE_CODE (fndecl) != FUNCTION_DECL)
     {
-      error ("invalid member function declaration");
+      error_at (DECL_SOURCE_LOCATION (fndecl),
+		"invalid member function declaration");
       return error_mark_node;
     }
 
Index: testsuite/g++.dg/parse/typedef9.C
===================================================================
--- testsuite/g++.dg/parse/typedef9.C	(revision 274140)
+++ 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: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" }
 };