c-family: copy attribute diagnostic fixes [PR113262]

Message ID ZZ0JQX+D1CeEH9aB@tucnak
State Unresolved
Headers
Series c-family: copy attribute diagnostic fixes [PR113262] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Jan. 9, 2024, 8:52 a.m. UTC
  Hi!

The copy attributes is allowed on decls as well as types and even has
checks whether decl (set to *node) is DECL_P or TYPE_P, but for diagnostics
unconditionally uses DECL_SOURCE_LOCATION (decl), which obviously only works
if it applies to a decl.

The following patch fixes that, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?

2024-01-09  Jakub Jelinek  <jakub@redhat.com>

	PR c/113262
	* c-attribs.cc (handle_copy_attribute): Don't use
	DECL_SOURCE_LOCATION (decl) if decl is not DECL_P, use input_location
	instead.  Formatting fixes.

	* gcc.dg/pr113262.c: New test.


	Jakub
  

Comments

Marek Polacek Jan. 9, 2024, 2:28 p.m. UTC | #1
On Tue, Jan 09, 2024 at 09:52:17AM +0100, Jakub Jelinek wrote:
> Hi!
> 
> The copy attributes is allowed on decls as well as types and even has
> checks whether decl (set to *node) is DECL_P or TYPE_P, but for diagnostics
> unconditionally uses DECL_SOURCE_LOCATION (decl), which obviously only works
> if it applies to a decl.
> 
> The following patch fixes that, bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk?

Ok, thanks!
 
> 2024-01-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/113262
> 	* c-attribs.cc (handle_copy_attribute): Don't use
> 	DECL_SOURCE_LOCATION (decl) if decl is not DECL_P, use input_location
> 	instead.  Formatting fixes.
> 
> 	* gcc.dg/pr113262.c: New test.
> 
> --- gcc/c-family/c-attribs.cc.jj	2024-01-03 12:07:02.020736256 +0100
> +++ gcc/c-family/c-attribs.cc	2024-01-08 22:10:04.789616664 +0100
> @@ -3143,13 +3143,14 @@ handle_copy_attribute (tree *node, tree
>    if (ref == error_mark_node)
>      return NULL_TREE;
>  
> +  location_t loc = input_location;
> +  if (DECL_P (decl))
> +    loc = DECL_SOURCE_LOCATION (decl);
>    if (TREE_CODE (ref) == STRING_CST)
>      {
>        /* Explicitly handle this case since using a string literal
>  	 as an argument is a likely mistake.  */
> -      error_at (DECL_SOURCE_LOCATION (decl),
> -		"%qE attribute argument cannot be a string",
> -		name);
> +      error_at (loc, "%qE attribute argument cannot be a string", name);
>        return NULL_TREE;
>      }
>  
> @@ -3160,10 +3161,8 @@ handle_copy_attribute (tree *node, tree
>        /* Similar to the string case, since some function attributes
>  	 accept literal numbers as arguments (e.g., alloc_size or
>  	 nonnull) using one here is a likely mistake.  */
> -      error_at (DECL_SOURCE_LOCATION (decl),
> -		"%qE attribute argument cannot be a constant arithmetic "
> -		"expression",
> -		name);
> +      error_at (loc, "%qE attribute argument cannot be a constant arithmetic "
> +		"expression", name);
>        return NULL_TREE;
>      }
>  
> @@ -3171,12 +3170,11 @@ handle_copy_attribute (tree *node, tree
>      {
>        /* Another possible mistake (but indirect self-references aren't
>  	 and diagnosed and shouldn't be).  */
> -      if (warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
> +      if (warning_at (loc, OPT_Wattributes,
>  		      "%qE attribute ignored on a redeclaration "
> -		      "of the referenced symbol",
> -		      name))
> -	inform (DECL_SOURCE_LOCATION (node[1]),
> -		"previous declaration here");
> +		      "of the referenced symbol", name)
> +	  && DECL_P (node[1]))
> +	inform (DECL_SOURCE_LOCATION (node[1]), "previous declaration here");
>        return NULL_TREE;
>      }
>  
> @@ -3196,7 +3194,8 @@ handle_copy_attribute (tree *node, tree
>  	ref = TREE_OPERAND (ref, 1);
>        else
>  	break;
> -    } while (!DECL_P (ref));
> +    }
> +  while (!DECL_P (ref));
>  
>    /* For object pointer expressions, consider those to be requests
>       to copy from their type, such as in:
> @@ -3228,8 +3227,7 @@ handle_copy_attribute (tree *node, tree
>  	     to a variable, or variable attributes to a function.  */
>  	  if (warning (OPT_Wattributes,
>  		       "%qE attribute ignored on a declaration of "
> -		       "a different kind than referenced symbol",
> -		       name)
> +		       "a different kind than referenced symbol", name)
>  	      && DECL_P (ref))
>  	    inform (DECL_SOURCE_LOCATION (ref),
>  		    "symbol %qD referenced by %qD declared here", ref, decl);
> @@ -3279,9 +3277,7 @@ handle_copy_attribute (tree *node, tree
>      }
>    else if (!TYPE_P (decl))
>      {
> -      error_at (DECL_SOURCE_LOCATION (decl),
> -		"%qE attribute must apply to a declaration",
> -		name);
> +      error_at (loc, "%qE attribute must apply to a declaration", name);
>        return NULL_TREE;
>      }
>  
> --- gcc/testsuite/gcc.dg/pr113262.c.jj	2024-01-08 22:19:07.414588762 +0100
> +++ gcc/testsuite/gcc.dg/pr113262.c	2024-01-08 22:18:51.327815573 +0100
> @@ -0,0 +1,6 @@
> +/* PR c/113262 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +int [[gnu::copy ("")]] a;	/* { dg-error "'copy' attribute argument cannot be a string" } */
> +
> 
> 	Jakub
> 

Marek
  
Jeff Law Jan. 9, 2024, 6:17 p.m. UTC | #2
On 1/9/24 01:52, Jakub Jelinek wrote:
> Hi!
> 
> The copy attributes is allowed on decls as well as types and even has
> checks whether decl (set to *node) is DECL_P or TYPE_P, but for diagnostics
> unconditionally uses DECL_SOURCE_LOCATION (decl), which obviously only works
> if it applies to a decl.
> 
> The following patch fixes that, bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk?
> 
> 2024-01-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/113262
> 	* c-attribs.cc (handle_copy_attribute): Don't use
> 	DECL_SOURCE_LOCATION (decl) if decl is not DECL_P, use input_location
> 	instead.  Formatting fixes.
> 
> 	* gcc.dg/pr113262.c: New test.
ok
Jeff
  
Jason Merrill Jan. 15, 2024, 10:43 p.m. UTC | #3
On 1/9/24 03:52, Jakub Jelinek wrote:
> Hi!
> 
> The copy attributes is allowed on decls as well as types and even has
> checks whether decl (set to *node) is DECL_P or TYPE_P, but for diagnostics
> unconditionally uses DECL_SOURCE_LOCATION (decl), which obviously only works
> if it applies to a decl.

In the C++ front-end location_of checks to see if the type has a 
TYPE_MAIN_DECL to get a location from, you might do that if !DECL_P?  OK 
either way.

> The following patch fixes that, bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk?
> 
> 2024-01-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/113262
> 	* c-attribs.cc (handle_copy_attribute): Don't use
> 	DECL_SOURCE_LOCATION (decl) if decl is not DECL_P, use input_location
> 	instead.  Formatting fixes.
> 
> 	* gcc.dg/pr113262.c: New test.
> 
> --- gcc/c-family/c-attribs.cc.jj	2024-01-03 12:07:02.020736256 +0100
> +++ gcc/c-family/c-attribs.cc	2024-01-08 22:10:04.789616664 +0100
> @@ -3143,13 +3143,14 @@ handle_copy_attribute (tree *node, tree
>     if (ref == error_mark_node)
>       return NULL_TREE;
>   
> +  location_t loc = input_location;
> +  if (DECL_P (decl))
> +    loc = DECL_SOURCE_LOCATION (decl);
>     if (TREE_CODE (ref) == STRING_CST)
>       {
>         /* Explicitly handle this case since using a string literal
>   	 as an argument is a likely mistake.  */
> -      error_at (DECL_SOURCE_LOCATION (decl),
> -		"%qE attribute argument cannot be a string",
> -		name);
> +      error_at (loc, "%qE attribute argument cannot be a string", name);
>         return NULL_TREE;
>       }
>   
> @@ -3160,10 +3161,8 @@ handle_copy_attribute (tree *node, tree
>         /* Similar to the string case, since some function attributes
>   	 accept literal numbers as arguments (e.g., alloc_size or
>   	 nonnull) using one here is a likely mistake.  */
> -      error_at (DECL_SOURCE_LOCATION (decl),
> -		"%qE attribute argument cannot be a constant arithmetic "
> -		"expression",
> -		name);
> +      error_at (loc, "%qE attribute argument cannot be a constant arithmetic "
> +		"expression", name);
>         return NULL_TREE;
>       }
>   
> @@ -3171,12 +3170,11 @@ handle_copy_attribute (tree *node, tree
>       {
>         /* Another possible mistake (but indirect self-references aren't
>   	 and diagnosed and shouldn't be).  */
> -      if (warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
> +      if (warning_at (loc, OPT_Wattributes,
>   		      "%qE attribute ignored on a redeclaration "
> -		      "of the referenced symbol",
> -		      name))
> -	inform (DECL_SOURCE_LOCATION (node[1]),
> -		"previous declaration here");
> +		      "of the referenced symbol", name)
> +	  && DECL_P (node[1]))
> +	inform (DECL_SOURCE_LOCATION (node[1]), "previous declaration here");
>         return NULL_TREE;
>       }
>   
> @@ -3196,7 +3194,8 @@ handle_copy_attribute (tree *node, tree
>   	ref = TREE_OPERAND (ref, 1);
>         else
>   	break;
> -    } while (!DECL_P (ref));
> +    }
> +  while (!DECL_P (ref));
>   
>     /* For object pointer expressions, consider those to be requests
>        to copy from their type, such as in:
> @@ -3228,8 +3227,7 @@ handle_copy_attribute (tree *node, tree
>   	     to a variable, or variable attributes to a function.  */
>   	  if (warning (OPT_Wattributes,
>   		       "%qE attribute ignored on a declaration of "
> -		       "a different kind than referenced symbol",
> -		       name)
> +		       "a different kind than referenced symbol", name)
>   	      && DECL_P (ref))
>   	    inform (DECL_SOURCE_LOCATION (ref),
>   		    "symbol %qD referenced by %qD declared here", ref, decl);
> @@ -3279,9 +3277,7 @@ handle_copy_attribute (tree *node, tree
>       }
>     else if (!TYPE_P (decl))
>       {
> -      error_at (DECL_SOURCE_LOCATION (decl),
> -		"%qE attribute must apply to a declaration",
> -		name);
> +      error_at (loc, "%qE attribute must apply to a declaration", name);
>         return NULL_TREE;
>       }
>   
> --- gcc/testsuite/gcc.dg/pr113262.c.jj	2024-01-08 22:19:07.414588762 +0100
> +++ gcc/testsuite/gcc.dg/pr113262.c	2024-01-08 22:18:51.327815573 +0100
> @@ -0,0 +1,6 @@
> +/* PR c/113262 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +int [[gnu::copy ("")]] a;	/* { dg-error "'copy' attribute argument cannot be a string" } */
> +
> 
> 	Jakub
>
  

Patch

--- gcc/c-family/c-attribs.cc.jj	2024-01-03 12:07:02.020736256 +0100
+++ gcc/c-family/c-attribs.cc	2024-01-08 22:10:04.789616664 +0100
@@ -3143,13 +3143,14 @@  handle_copy_attribute (tree *node, tree
   if (ref == error_mark_node)
     return NULL_TREE;
 
+  location_t loc = input_location;
+  if (DECL_P (decl))
+    loc = DECL_SOURCE_LOCATION (decl);
   if (TREE_CODE (ref) == STRING_CST)
     {
       /* Explicitly handle this case since using a string literal
 	 as an argument is a likely mistake.  */
-      error_at (DECL_SOURCE_LOCATION (decl),
-		"%qE attribute argument cannot be a string",
-		name);
+      error_at (loc, "%qE attribute argument cannot be a string", name);
       return NULL_TREE;
     }
 
@@ -3160,10 +3161,8 @@  handle_copy_attribute (tree *node, tree
       /* Similar to the string case, since some function attributes
 	 accept literal numbers as arguments (e.g., alloc_size or
 	 nonnull) using one here is a likely mistake.  */
-      error_at (DECL_SOURCE_LOCATION (decl),
-		"%qE attribute argument cannot be a constant arithmetic "
-		"expression",
-		name);
+      error_at (loc, "%qE attribute argument cannot be a constant arithmetic "
+		"expression", name);
       return NULL_TREE;
     }
 
@@ -3171,12 +3170,11 @@  handle_copy_attribute (tree *node, tree
     {
       /* Another possible mistake (but indirect self-references aren't
 	 and diagnosed and shouldn't be).  */
-      if (warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+      if (warning_at (loc, OPT_Wattributes,
 		      "%qE attribute ignored on a redeclaration "
-		      "of the referenced symbol",
-		      name))
-	inform (DECL_SOURCE_LOCATION (node[1]),
-		"previous declaration here");
+		      "of the referenced symbol", name)
+	  && DECL_P (node[1]))
+	inform (DECL_SOURCE_LOCATION (node[1]), "previous declaration here");
       return NULL_TREE;
     }
 
@@ -3196,7 +3194,8 @@  handle_copy_attribute (tree *node, tree
 	ref = TREE_OPERAND (ref, 1);
       else
 	break;
-    } while (!DECL_P (ref));
+    }
+  while (!DECL_P (ref));
 
   /* For object pointer expressions, consider those to be requests
      to copy from their type, such as in:
@@ -3228,8 +3227,7 @@  handle_copy_attribute (tree *node, tree
 	     to a variable, or variable attributes to a function.  */
 	  if (warning (OPT_Wattributes,
 		       "%qE attribute ignored on a declaration of "
-		       "a different kind than referenced symbol",
-		       name)
+		       "a different kind than referenced symbol", name)
 	      && DECL_P (ref))
 	    inform (DECL_SOURCE_LOCATION (ref),
 		    "symbol %qD referenced by %qD declared here", ref, decl);
@@ -3279,9 +3277,7 @@  handle_copy_attribute (tree *node, tree
     }
   else if (!TYPE_P (decl))
     {
-      error_at (DECL_SOURCE_LOCATION (decl),
-		"%qE attribute must apply to a declaration",
-		name);
+      error_at (loc, "%qE attribute must apply to a declaration", name);
       return NULL_TREE;
     }
 
--- gcc/testsuite/gcc.dg/pr113262.c.jj	2024-01-08 22:19:07.414588762 +0100
+++ gcc/testsuite/gcc.dg/pr113262.c	2024-01-08 22:18:51.327815573 +0100
@@ -0,0 +1,6 @@ 
+/* PR c/113262 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+int [[gnu::copy ("")]] a;	/* { dg-error "'copy' attribute argument cannot be a string" } */
+