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

login
register
mail settings
Submitter Marek Polacek
Date April 14, 2014, 5:32 p.m.
Message ID <20140414173232.GB20332@redhat.com>
Download mbox | patch
Permalink /patch/339023/
State New
Headers show

Comments

Marek Polacek - April 14, 2014, 5:32 p.m.
On Mon, Apr 14, 2014 at 05:26:03PM +0200, Marc Glisse wrote:
> If val is NULL, it seems you still end up looking at its TREE_CODE?
> (don't know if that can happen)
 
I didn't hit that while testing, but it looks weird, so fixed.

> In C++, default_conversion probably handles IDENTIFIER_NODE just
> fine, but having different code for the 2 languages would not be
> nice, right?

Right.  Incidentally, C's default_conversion can handle CONST_DECLs
too, so I tweaked get_attrib_value routine a bit.  Is it any better?

Tested x86_64.

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

	PR c/50459
c-family/
	* c-common.c (get_attrib_value): New function.
	(get_priority): Call get_attrib_value.
	(check_user_alignment): Likewise.
	(handle_alloc_size_attribute): Likewise.
	(handle_alloc_align_attribute): Likewise.
	(handle_assume_aligned_attribute): Likewise.
	(handle_vector_size_attribute): Likewise.
	(handle_sentinel_attribute): Likewise.
testsuite/
	* c-c++-common/pr50459.c: New test.
	* g++.dg/ext/pr50459.C: New test.


	Marek
Richard Henderson - April 16, 2014, 9:35 p.m.
On 04/14/2014 10:32 AM, Marek Polacek wrote:
> +      if (TREE_CODE (val) != IDENTIFIER_NODE
> +	  && TREE_CODE (val) != FUNCTION_DECL)
> +	val = default_conversion (val);
> +      else if (TREE_CODE (val) == IDENTIFIER_NODE)
> +	{
> +	  tree t = lookup_name (val);
> +	  if (t && TREE_CODE (t) == CONST_DECL)
> +	    val = default_conversion (t);
> +	}

In addition to Jason's comment, a general style point:

  if (X != A && X != B)
    ...
  else if (X == A)
    ...

should be written

  if (X == A)
    ...
  else if (X != B)
    ...

As a general rule, positive tests are easier to reason with than negative tests.


r~

Patch

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 1d56bc0..d627462 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -6448,6 +6448,28 @@  attribute_takes_identifier_p (const_tree attr_id)
     return targetm.attribute_takes_identifier_p (attr_id);
 }
 
+/* Extract the integer constant from an enum value and/or perform default
+   promotions.  This function is used for getting a value from an attribute
+   argument.  */
+
+static tree
+get_attrib_value (tree val)
+{
+  if (val)
+    {
+      if (TREE_CODE (val) != IDENTIFIER_NODE
+	  && TREE_CODE (val) != FUNCTION_DECL)
+	val = default_conversion (val);
+      else if (TREE_CODE (val) == IDENTIFIER_NODE)
+	{
+	  tree t = lookup_name (val);
+	  if (t && TREE_CODE (t) == CONST_DECL)
+	    val = default_conversion (t);
+	}
+    }
+  return val;
+}
+
 /* Attribute handlers common to C front ends.  */
 
 /* Handle a "packed" attribute; arguments as in
@@ -7030,11 +7052,10 @@  get_priority (tree args, bool is_destructor)
     }
 
   arg = TREE_VALUE (args);
-  if (TREE_CODE (arg) == IDENTIFIER_NODE)
-    goto invalid;
   if (arg == error_mark_node)
     return DEFAULT_INIT_PRIORITY;
-  arg = default_conversion (arg);
+  arg = get_attrib_value (arg);
+
   if (!tree_fits_shwi_p (arg)
       || !INTEGRAL_TYPE_P (TREE_TYPE (arg)))
     goto invalid;
@@ -7418,6 +7439,8 @@  check_user_alignment (const_tree align, bool allow_zero)
 {
   int i;
 
+  align = get_attrib_value (CONST_CAST_TREE (align));
+
   if (TREE_CODE (align) != INTEGER_CST
       || !INTEGRAL_TYPE_P (TREE_TYPE (align)))
     {
@@ -8037,11 +8060,7 @@  handle_alloc_size_attribute (tree *node, tree ARG_UNUSED (name), tree args,
   unsigned arg_count = type_num_arguments (*node);
   for (; args; args = TREE_CHAIN (args))
     {
-      tree position = TREE_VALUE (args);
-      if (position && TREE_CODE (position) != IDENTIFIER_NODE
-	  && TREE_CODE (position) != FUNCTION_DECL)
-	position = default_conversion (position);
-
+      tree position = get_attrib_value (TREE_VALUE (args));
       if (!tree_fits_uhwi_p (position)
 	  || !arg_count
 	  || !IN_RANGE (tree_to_uhwi (position), 1, arg_count))
@@ -8063,10 +8082,7 @@  handle_alloc_align_attribute (tree *node, tree, tree args, int,
 			      bool *no_add_attrs)
 {
   unsigned arg_count = type_num_arguments (*node);
-  tree position = TREE_VALUE (args);
-  if (position && TREE_CODE (position) != IDENTIFIER_NODE)
-    position = default_conversion (position);
-
+  tree position = get_attrib_value (TREE_VALUE (args));
   if (!tree_fits_uhwi_p (position)
       || !arg_count
       || !IN_RANGE (tree_to_uhwi (position), 1, arg_count))
@@ -8088,11 +8104,7 @@  handle_assume_aligned_attribute (tree *, tree, tree args, int,
 {
   for (; args; args = TREE_CHAIN (args))
     {
-      tree position = TREE_VALUE (args);
-      if (position && TREE_CODE (position) != IDENTIFIER_NODE
-	  && TREE_CODE (position) != FUNCTION_DECL)
-	position = default_conversion (position);
-
+      tree position = get_attrib_value (TREE_VALUE (args));
       if (TREE_CODE (position) != INTEGER_CST)
 	{
 	  warning (OPT_Wattributes,
@@ -8532,10 +8544,7 @@  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)
-    size = default_conversion (size);
-
+  size = get_attrib_value (TREE_VALUE (args));
   if (!tree_fits_uhwi_p (size))
     {
       warning (OPT_Wattributes, "%qE attribute ignored", name);
@@ -8943,8 +8952,7 @@  handle_sentinel_attribute (tree *node, tree name, tree args,
 
   if (args)
     {
-      tree position = TREE_VALUE (args);
-
+      tree position = get_attrib_value (TREE_VALUE (args));
       if (TREE_CODE (position) != INTEGER_CST)
 	{
 	  warning (OPT_Wattributes,
diff --git gcc/testsuite/c-c++-common/pr50459.c gcc/testsuite/c-c++-common/pr50459.c
index e69de29..f837b63 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;
diff --git gcc/testsuite/g++.dg/ext/pr50459.C gcc/testsuite/g++.dg/ext/pr50459.C
index e69de29..8acc299 100644
--- gcc/testsuite/g++.dg/ext/pr50459.C
+++ gcc/testsuite/g++.dg/ext/pr50459.C
@@ -0,0 +1,14 @@ 
+// PR c/50459
+// { dg-do compile }
+// { dg-options "-Wall -Wextra" }
+
+const int 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;