PING Re: RFA (c-common): PATCH for c++/36625 (misinterpreting name in attribute argument)

[Jason Merrill <jason at redhat dot com>]

On 04/09/2010 04:30 PM, Jason Merrill wrote:
> In 36625, we assume that an identifier that appears as the first
> argument to an attribute is always intended as a plain name as for
> attribute (mode). But this isn't the case for all attributes, which can
> cause problems in templates when people try to use a template non-type
> parameter as an attribute argument.
>
> This patch adds a predicate in c-common to distinguish between
> attributes that want a plain name and attributes that want an
> expression, then uses it to decide how to parse the name.
>
> Tested x86_64-pc-linux-gnu. Are the c-common bits OK for trunk?

Ping.

commit 56526ab556def415ab97b88e0920a80ef4b606c0
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Jan 20 00:50:33 2010 -0500

    	PR c++/36625
    	* c-common.c (attribute_takes_identifier_p): New fn.
    	* c-common.h: Declare it.
    cp/
    	* parser.c (cp_parser_parenthesized_expression_list): Change
    	is_attribute_list parm to int to indicate whether or not to
    	handle initial identifier specially.
    	(cp_parser_attribute_list): Use attribute_takes_identifier_p.

diff --git a/gcc/c-common.c b/gcc/c-common.c
index 5772b83..8d9990b 100644
--- a/gcc/c-common.c
+++ b/gcc/c-common.c
@@ -5803,6 +5803,17 @@ c_init_attributes (void)
 #undef DEF_ATTR_TREE_LIST
 }
 
+/* Returns TRUE iff the attribute indicated by ATTR_ID takes a plain
+   identifier as an argument, so the front end shouldn't look it up.  */
+
+bool
+attribute_takes_identifier_p (tree attr_id)
+{
+  return (is_attribute_p ("mode", attr_id)
+	  || is_attribute_p ("format", attr_id)
+	  || is_attribute_p ("cleanup", attr_id));
+}
+
 /* Attribute handlers common to C front ends.  */
 
 /* Handle a "packed" attribute; arguments as in
diff --git a/gcc/c-common.h b/gcc/c-common.h
index 6f8d832..6ed3849 100644
--- a/gcc/c-common.h
+++ b/gcc/c-common.h
@@ -822,6 +822,7 @@ extern void check_function_format (tree, int, tree *);
 extern void set_Wformat (int);
 extern tree handle_format_attribute (tree *, tree, tree, int, bool *);
 extern tree handle_format_arg_attribute (tree *, tree, tree, int, bool *);
+extern bool attribute_takes_identifier_p (tree);
 extern int c_common_handle_option (size_t code, const char *arg, int value);
 extern bool c_common_missing_argument (const char *opt, size_t code);
 extern tree c_common_type_for_mode (enum machine_mode, int);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index e25c280..8af22c0 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -1596,7 +1596,9 @@ static tree cp_parser_postfix_open_square_expression
 static tree cp_parser_postfix_dot_deref_expression
   (cp_parser *, enum cpp_ttype, tree, bool, cp_id_kind *, location_t);
 static VEC(tree,gc) *cp_parser_parenthesized_expression_list
-  (cp_parser *, bool, bool, bool, bool *);
+  (cp_parser *, int, bool, bool, bool *);
+/* Values for the second parameter of cp_parser_parenthesized_expression_list.  */
+enum { non_attr = 0, normal_attr = 1, id_attr = 2 };
 static void cp_parser_pseudo_destructor_name
   (cp_parser *, tree *, tree *);
 static tree cp_parser_unary_expression
@@ -4806,7 +4808,7 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p,
 		parser->integral_constant_expression_p = false;
 	      }
 	    args = (cp_parser_parenthesized_expression_list
-		    (parser, /*is_attribute_list=*/false,
+		    (parser, non_attr,
 		     /*cast_p=*/false, /*allow_expansion_p=*/true,
 		     /*non_constant_p=*/NULL));
 	    if (is_builtin_constant_p)
@@ -5244,20 +5246,22 @@ cp_parser_postfix_dot_deref_expression (cp_parser *parser,
    Returns a vector of trees.  Each element is a representation of an
    assignment-expression.  NULL is returned if the ( and or ) are
    missing.  An empty, but allocated, vector is returned on no
-   expressions.  The parentheses are eaten.  IS_ATTRIBUTE_LIST is true
-   if this is really an attribute list being parsed.  If
+   expressions.  The parentheses are eaten.  IS_ATTRIBUTE_LIST is id_attr
+   if we are parsing an attribute list for an attribute that wants a
+   plain identifier argument, normal_attr for an attribute that wants
+   an expression, or non_attr if we aren't parsing an attribute list.  If
    NON_CONSTANT_P is non-NULL, *NON_CONSTANT_P indicates whether or
    not all of the expressions in the list were constant.  */
 
 static VEC(tree,gc) *
 cp_parser_parenthesized_expression_list (cp_parser* parser,
-					 bool is_attribute_list,
+					 int is_attribute_list,
 					 bool cast_p,
                                          bool allow_expansion_p,
 					 bool *non_constant_p)
 {
   VEC(tree,gc) *expression_list;
-  bool fold_expr_p = is_attribute_list;
+  bool fold_expr_p = is_attribute_list != non_attr;
   tree identifier = NULL_TREE;
   bool saved_greater_than_is_operator_p;
 
@@ -5284,7 +5288,7 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
 
 	/* At the beginning of attribute lists, check to see if the
 	   next token is an identifier.  */
-	if (is_attribute_list
+	if (is_attribute_list == id_attr
 	    && cp_lexer_peek_token (parser->lexer)->type == CPP_NAME)
 	  {
 	    cp_token *token;
@@ -5345,7 +5349,7 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
 
 	/* After the first item, attribute lists look the same as
 	   expression lists.  */
-	is_attribute_list = false;
+	is_attribute_list = non_attr;
 
       get_comma:;
 	/* If the next token isn't a `,', then we are done.  */
@@ -5820,7 +5824,8 @@ cp_parser_new_placement (cp_parser* parser)
 
   /* Parse the expression-list.  */
   expression_list = (cp_parser_parenthesized_expression_list
-		     (parser, false, /*cast_p=*/false, /*allow_expansion_p=*/true,
+		     (parser, non_attr, /*cast_p=*/false,
+		      /*allow_expansion_p=*/true,
 		      /*non_constant_p=*/NULL));
 
   return expression_list;
@@ -6026,7 +6031,8 @@ cp_parser_new_initializer (cp_parser* parser)
     }
   else
     expression_list = (cp_parser_parenthesized_expression_list
-		       (parser, false, /*cast_p=*/false, /*allow_expansion_p=*/true,
+		       (parser, non_attr, /*cast_p=*/false,
+			/*allow_expansion_p=*/true,
 			/*non_constant_p=*/NULL));
 
   return expression_list;
@@ -9963,7 +9969,7 @@ cp_parser_mem_initializer (cp_parser* parser)
   else
     {
       VEC(tree,gc)* vec;
-      vec = cp_parser_parenthesized_expression_list (parser, false,
+      vec = cp_parser_parenthesized_expression_list (parser, non_attr,
 						     /*cast_p=*/false,
 						     /*allow_expansion_p=*/true,
 						     /*non_constant_p=*/NULL);
@@ -15449,7 +15455,7 @@ cp_parser_initializer (cp_parser* parser, bool* is_direct_init,
   else if (token->type == CPP_OPEN_PAREN)
     {
       VEC(tree,gc) *vec;
-      vec = cp_parser_parenthesized_expression_list (parser, false,
+      vec = cp_parser_parenthesized_expression_list (parser, non_attr,
 						     /*cast_p=*/false,
 						     /*allow_expansion_p=*/true,
 						     non_constant_p);
@@ -17764,8 +17770,10 @@ cp_parser_attribute_list (cp_parser* parser)
 	  if (token->type == CPP_OPEN_PAREN)
 	    {
 	      VEC(tree,gc) *vec;
+	      int attr_flag = (attribute_takes_identifier_p (identifier)
+			       ? id_attr : normal_attr);
 	      vec = cp_parser_parenthesized_expression_list
-		    (parser, true, /*cast_p=*/false,
+		    (parser, attr_flag, /*cast_p=*/false,
 		     /*allow_expansion_p=*/false,
 		     /*non_constant_p=*/NULL);
 	      if (vec == NULL)
@@ -18967,7 +18975,7 @@ cp_parser_functional_cast (cp_parser* parser, tree type)
     }
 
 
-  vec = cp_parser_parenthesized_expression_list (parser, false,
+  vec = cp_parser_parenthesized_expression_list (parser, non_attr,
 						 /*cast_p=*/true,
 						 /*allow_expansion_p=*/true,
 						 /*non_constant_p=*/NULL);
diff --git a/gcc/testsuite/g++.dg/ext/attrib38.C b/gcc/testsuite/g++.dg/ext/attrib38.C
new file mode 100644
index 0000000..be3c7f2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attrib38.C
@@ -0,0 +1,11 @@
+// PR c++/36625
+
+template <int N>
+struct A {
+  struct S { short f[3]; } __attribute__ ((aligned (N)));
+};
+
+int main ()
+{
+  A<4>::S s;
+}