Re: [PATCH] PR 53528 c++/ C++11 Generalized Attribute support

[Jason Merrill <jason at redhat dot com>]

On 07/26/2012 11:19 AM, Dodji Seketeli wrote:
> +// Example taken from dcl.attr.grammar:
> +
> +int p[10];
> +void f()
> +{
> +    int x = 42, y[5];
> +    /* Here, the '[[gnu::' should have introduced an attribute, ont a
> +       lambda invocation an array subscripting expression.  */
> +    int(p[[gnu::x] { return x; }()]); // { dg-error "expected|consecutive" }
> +    /* Likewise, the '[[gnu::' is invalid here.  */
> +    y[[gnu::] { return 2; }()] = 2; // { dg-error "expected|consecutive" }

The example in the standard doesn't have gnu:: in it.  Search and 
replace error?

> +      if (cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE) == NULL
> +         || cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE) == NULL)

Let's use ! rather than == NULL so that these lines fit in 80 chars.

> +      /* If E is a constant, fold it and return it right away.
> +        Otherwise, build an ALIGNOF_EXPR that will be substituted
> +        into, later at template instantiation time.  */
> +      tree cst = TYPE_P (e) ? NULL_TREE : cxx_constant_value (e);

I think you want fold_non_dependent_expr_sfinae rather than 
cxx_constant_value.

> +      /*
> +        [dcl.align]/3

Extra newline.

> +int **** [[gnu::format(printf, 1, 2)]] foo(const char *, ...);

This seems wrong; you can't apply the format attribute to a pointer type.

About your earlier question on IRC, the solution to the problem you're 
having with

> struct A {int i;} [[gnu::aligned(16)]] a;

is that C++11 attributes after the closing brace should not be passed to 
finish_struct.  Instead, we should apply them to the type (without 
TYPE_IN_PLACE) after the type is complete.

> +typedef union { int i; } U [[gnu::transparent_union]];

For the same reason, this should also be rejected; the testcase should 
put the attribute before the opening brace.  We accept this with 
GNU-style attributes for backward compatibility, but there's no reason 
to propagate that lossage into the new syntax.

> +template<typename T> struct A
> +{
> +  void foo() const;
> +} [[gnu::aligned(4)]];

Likewise.

> +  typedef void ([[gnu::__stdcall__]] T::*F) (L*);

I don't think a C++11 attribute can appear in this position.  I think it 
should be

 typedef void (T::*F)(L*) [[gnu::__stdcall__]]

> +    virtual void [[gnu::__stdcall__]] A(L *listener) = 0;

Similarly, here the stdcall attribute appertains to "void", which makes 
no sense.

> +S [[gnu::__stdcall__]] getS();
> +extern int * ([[gnu::stdcall]] *fooPtr)( void);
> +int * [[gnu::stdcall]] myFn01( void) { return 0; }

Likewise.

> +  typedef [[gnu::aligned (16)]] struct {

This also seems ill-formed, as there are no type-specifiers before the 
attribute, so there's nothing for it to appertain to.

> +int
> +[[gnu::noreturn]]
> +[[gnu::unused]]
> +one(void); // OK

noreturn doesn't make sense for "int".  Nor does unused, really.  C++11 
attributes that appertain to the function must either come before any 
specifiers or after the declarator.

> +template
> +[[gnu::packed]]
> +struct A<int>;                 // { dg-warning "attribute" }

Here the patch is giving the wrong warning; it complains about applying 
the attribute to A<int>, but actually we should be warning about an 
attribute that would appertain to all the declarators in a declaration 
with no declarators for it to appertain to.

> +[[gnu::deprecated]] enum E { E0 };     // { dg-warning "attribute ignored in declaration of" }
> +// { dg-message "must follow the" "" { target *-*-* } 3 }

Same here.

> +         if (strcmp (attr_name, IDENTIFIER_POINTER (get_attribute_name (list))) == 0)
> +      gcc_checking_assert (TREE_CODE (get_attribute_name (list)) == IDENTIFIER_NODE);
> +             for (a = lookup_ident_attribute (get_attribute_name (a2), attributes);
> +                  a = lookup_ident_attribute (get_attribute_name (a2), TREE_CHAIN (a)))

Line too long.

> +  /* A given attribute has been parsed as a C++-11 generalized
> +     attribute.  */

Let's drop the word "generalized" throughout the patch.  I don't see how 
these attributes are any more generalized than GNU attributes; we should 
just describe them as C++11 attributes.

> +cxx_11_attribute_p (const_tree attr)

Let's change cxx_11 to cxx11.

Jason