rsmith added a comment.
In https://reviews.llvm.org/D37436#869851, @hfinkel wrote:
> A large fraction of the number of attributes we'll want to use are going to
> fall into this category (because Clang doesn't have its own attributes, but
> copied GCC's, for many things). I don't think we'll get good implementation
> feedback until we have this figured out. If we can't sync with GCC soon, I
> suggest just making a reasonable guess. My first choice would be to just
> allowing everything, and then we can see what people found to be useful and
> why. Our experience here can also provide feedback to GCC (and we can always
> update late if needed - it is still an experimental feature).
I think I would be more comfortable taking those attributes that we want to
support in C and making them available in the `clang::` attribute namespace
(across all languages). Because GCC has distinct C and C++ parsers (sharing
some, but not all, code) there are often differences between how they handle
the same construct in C and C++ mode, and accepting all `[[gnu::something]]`
attributes in C mode seems likely to create incompatibilities, even if GCC
decides that all `__attribute__`s should be available as `[[gnu::x]]` in C.
Also, your wording paper appears to allow things like
struct [[foo]] S *p; // ok in c n2137, ill-formed in c++
struct T {};
int n = sizeof(struct [[foo]] T); // ok in c n2137, ill-formed in c++
You don't seem to implement those changes; are they bugs in the wording paper?
(The relevant C++ rule is [dcl.type.elab]p1: "An //attribute-specifier-seq//
shall not appear in an //elaborated-type-specifier// unless the latter is the
sole constituent of a //declaration//." Neither of the above cases is
prohibited by n2137's 6.7.2.1/6: "An //attribute-specifier-seq// shall not
appear in a //struct-or-union-specifier// of the form //struct-or-union//
//attribute-specifier-seq//opt //identifier// if the
//struct-or-union-specifier// is an incomplete type used in a
//parameter-declaration// or //type-name//." -- the first case is not in a
//parameter-declaration// or //type-name// and the second case is not an
incomplete type -- and no other rule seems to disallow this.)
================
Comment at: include/clang/Basic/LangOptions.def:140
+LANGOPT(CAttributes , 1, 0, "'[[]]' attributes extension to C")
+
----------------
aaron.ballman wrote:
> rsmith wrote:
> > Can we give this a more general name, then enable it under `CPlusPlus` too?
> > That's what we do for other similar features.
> I'm not keen on naming it 'Attributes', but the other names are worse
> (`GeneralAttributes`, `StandardAttributes`). Do you have a suggestion?
It's kind of long, but how about `DoubleSquareBracketAttributes`
================
Comment at: include/clang/Driver/Options.td:607
+def fc_attributes : Flag<["-"], "fc-attributes">, Group<f_Group>,
+ Flags<[DriverOption, CC1Option]>, HelpText<"Enable '[[]]' attributes in C">;
----------------
aaron.ballman wrote:
> rsmith wrote:
> > Hmm. On reflection, if we're going to have a flag to enable C++11
> > attributes in other language modes, it should probably work in C++98 mode
> > too, so calling this `-fc-attributes` is probably not the best idea.
> > `-fc++11-attributes` might make sense, though.
> I can't think of a reason why you'd want to control C and C++ attributes
> separately, so I think it makes sense to add a more general name for this.
>
> I'm definitely not keen on that flag name. I wouldn't be opposed to
> `-fattributes`, but that may lead to confusion about other vendor-specific
> attributes (which we could always decide to use flags like
> `-fdeclspec-attributes` etc).
>
> What should happen if a user compiles with `-std=c++11
> -fno-<whatever>-attributes`? Do we want to support such a construct?
I wouldn't recommend anyone actually does that, but I'd expect clang to respect
their wishes if they ask for it, just like we do for `-fms-extensions
-fno-declspec`.
================
Comment at: lib/Parse/ParseDecl.cpp:4219
+
+ // Attributes are prohibited in this location in C2x (and forward
+ // declarations are prohibited in C++).
----------------
aaron.ballman wrote:
> rsmith wrote:
> > I don't think we can reasonably say what C2x will do. Also, doesn't this
> > reject valid code such as:
> > ```
> > struct __attribute__((deprecated)) Foo;
> > ```
> > ?
> Sorry for the lack of context in the patch (TortoiseSVN doesn't make this
> easy). This has to do with enum specifiers, not struct or unions -- it will
> reject `enum __attribute__((deprecated)) Foo;` as not allowing an attribute
> list *and* as being a forward reference in C++ mode, but accepts in C mode.
OK, but GCC accepts that with a warning in the `friend` case, and this would
also seem to reject valid constructs such as
```
enum __attribute__((deprecated)) Foo : int;
```
But... C permits neither the `TUK_Declaration` nor `TUK_Friend` cases for
`enum`s. The change in your wording proposal appears to be for the
`TUK_Reference` case instead, which you didn't change here.
https://reviews.llvm.org/D37436
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
