diff mbox

[C] Make attributes accept enum values (PR c/50459)

Message ID 20140417160050.GG20332@redhat.com
State New
Headers show

Commit Message

Marek Polacek April 17, 2014, 4 p.m. UTC 
On Wed, Apr 16, 2014 at 01:40:22PM -0400, Jason Merrill wrote:
> On 04/15/2014 03:56 PM, Marek Polacek wrote:
> >The testsuite doesn't hit this code with C++, but does hit this code
> >with C.  The thing is, if we have e.g.
> >enum { A = 128 };
> >void *fn1 (void) __attribute__((assume_aligned (A)));
> >then handle_assume_aligned_attribute walks the attribute arguments
> >and gets the argument via TREE_VALUE.  If this argument is an enum
> >value, then for C the argument is identifier_node that contains const_decl,
> 
> Ah.  Then I think the C parser should be fixed to check
> attribute_takes_identifier_p and look up the argument if false.

Ok, thanks, I didn't know about attribute_takes_identifier_p.  Should be done
in the following.  Regtested/bootstrapped on x86_64-linux.  Ok now?

2014-04-17  Marek Polacek  <polacek@redhat.com>

	PR c/50459
c-family/
	* c-common.c (handle_aligned_attribute): Don't call default_conversion
	on FUNCTION_DECLs.
	(handle_vector_size_attribute): Likewise.
	(handle_sentinel_attribute): Call default_conversion and allow even
	integral types as an argument.  
c/
	* c-parser.c (c_parser_attributes): If the attribute doesn't take an
	identifier, call lookup_name for arguments. 
testsuite/
	* c-c++-common/pr50459.c: New test.


	Marek

Comments

Jason Merrill April 19, 2014, 1:56 p.m. UTC | #1 
On 04/17/2014 12:00 PM, Marek Polacek wrote:
>   		      == CPP_CLOSE_PAREN)))
>   	    {
>   	      tree arg1 = c_parser_peek_token (parser)->value;
> +	      if (!attr_takes_id_p)
> +	        {
> +		  /* This is for enum values, so that they can be used as
> +		     an attribute parameter; lookup_name will find their
> +		     CONST_DECLs.  */
> +		  tree ln = lookup_name (arg1);
> +		  if (ln)
> +		    arg1 = ln;
> +		}
>   	      c_parser_consume_token (parser);

Instead, we should add !attr_takes_id_p to the if condition immediately 
above so that we parse the arguments as an expression-list.

Jason
diff mbox

Patch

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index c0e247b..1443914 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -7539,7 +7539,8 @@  handle_aligned_attribute (tree *node, tree ARG_UNUSED (name), tree args,
   if (args)
     {
       align_expr = TREE_VALUE (args);
-      if (align_expr && TREE_CODE (align_expr) != IDENTIFIER_NODE)
+      if (align_expr && TREE_CODE (align_expr) != IDENTIFIER_NODE
+	  && TREE_CODE (align_expr) != FUNCTION_DECL)
 	align_expr = default_conversion (align_expr);
     }
   else
@@ -8533,7 +8534,8 @@  handle_vector_size_attribute (tree *node, tree name, tree args,
   *no_add_attrs = true;
 
   size = TREE_VALUE (args);
-  if (size && TREE_CODE (size) != IDENTIFIER_NODE)
+  if (size && TREE_CODE (size) != IDENTIFIER_NODE
+      && TREE_CODE (size) != FUNCTION_DECL)
     size = default_conversion (size);
 
   if (!tree_fits_uhwi_p (size))
@@ -8944,8 +8946,12 @@  handle_sentinel_attribute (tree *node, tree name, tree args,
   if (args)
     {
       tree position = TREE_VALUE (args);
+      if (position && TREE_CODE (position) != IDENTIFIER_NODE
+	  && TREE_CODE (position) != FUNCTION_DECL)
+	position = default_conversion (position);
 
-      if (TREE_CODE (position) != INTEGER_CST)
+      if (TREE_CODE (position) != INTEGER_CST
+          || !INTEGRAL_TYPE_P (TREE_TYPE (position)))
 	{
 	  warning (OPT_Wattributes,
 		   "requested position is not an integer constant");
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 5653e49..f8fe424 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -3912,6 +3912,7 @@  c_parser_attributes (c_parser *parser)
 	     || c_parser_next_token_is (parser, CPP_KEYWORD))
 	{
 	  tree attr, attr_name, attr_args;
+	  bool attr_takes_id_p;
 	  vec<tree, va_gc> *expr_list;
 	  if (c_parser_next_token_is (parser, CPP_COMMA))
 	    {
@@ -3922,6 +3923,7 @@  c_parser_attributes (c_parser *parser)
 	  attr_name = c_parser_attribute_any_word (parser);
 	  if (attr_name == NULL)
 	    break;
+	  attr_takes_id_p = attribute_takes_identifier_p (attr_name);
 	  if (is_cilkplus_vector_p (attr_name))		  
 	    {
 	      c_token *v_token = c_parser_peek_token (parser);
@@ -3950,6 +3952,15 @@  c_parser_attributes (c_parser *parser)
 		      == CPP_CLOSE_PAREN)))
 	    {
 	      tree arg1 = c_parser_peek_token (parser)->value;
+	      if (!attr_takes_id_p)
+	        {
+		  /* This is for enum values, so that they can be used as
+		     an attribute parameter; lookup_name will find their
+		     CONST_DECLs.  */
+		  tree ln = lookup_name (arg1);
+		  if (ln)
+		    arg1 = ln;
+		}
 	      c_parser_consume_token (parser);
 	      if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
 		attr_args = build_tree_list (NULL_TREE, arg1);
diff --git gcc/testsuite/c-c++-common/pr50459.c gcc/testsuite/c-c++-common/pr50459.c
index e69de29..f954b32 100644
--- gcc/testsuite/c-c++-common/pr50459.c
+++ gcc/testsuite/c-c++-common/pr50459.c
@@ -0,0 +1,14 @@ 
+/* PR c/50459 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+enum { A = 128, B = 1 };
+void *fn1 (void) __attribute__((assume_aligned (A)));
+void *fn2 (void) __attribute__((assume_aligned (A, 4)));
+void fn3 (void) __attribute__((constructor (A)));
+void fn4 (void) __attribute__((destructor (A)));
+void *fn5 (int) __attribute__((alloc_size (B)));
+void *fn6 (int) __attribute__((alloc_align (B)));
+void fn7 (const char *, ...) __attribute__ ((sentinel (B)));
+int __attribute__((vector_size (A))) a;
+int __attribute__((aligned (A))) foo;