[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D37436#868333, @aaron.ballman wrote:

> In https://reviews.llvm.org/D37436#868295, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D37436#867965, @aaron.ballman wrote:
> >
> > > In https://reviews.llvm.org/D37436#867287, @rsmith wrote:
> > >
> > > > If this is just supposed to be an experiment to get feedback on the feature,  then I don't think we should be treating it as a different attribute syntax at all. Rather, I think we
> > > >  just want to permit C++11 attributes to be parsed in other language modes. If/when this becomes part of some future C working draft, I think that's the time to have a
> > > >  separate attribute syntax with a distinct set of valid unqualified attribute names.
> > >
> > >
> > > I do not think that's the correct approach. These are not C++ attributes (for instance, no `using` insanity is allowed, `::` is a new lexing token in C, etc). Also, I don't think it's a good idea to enable all C++11-style attributes in C mode without giving each attribute some appropriate thought (what does `abi_tag` *do* in C mode? What happens with _Noreturn vs [[noreturn]] etc). Also, I'm not comfortable adding a bunch of vendor-specific `gnu::` attributes that GCC does not implement in C yet.
> >
> >
> > On this last point, I disagree. Implementation experience is about all of the messy things that occur in production. In production, if we have this syntax, then we'll end up enabling it for a bunch of vendor-specific attributes. Do you think that we wouldn't?
>
>
> I'm sure we would. Also, FWIW, I was planning to traverse the attributes we implement to find which clang-specific C++ attributes would make sense to also enable as a follow-up patch once the syntax is in.
>
> > N2137 specifically talks about this as a use case. If so, this will include `gnu::` attributes that we have in Clang (even if GCC does not implement them).
>
> Eventually, yes, but it seems like a problem to implement something under that vendor namespace when the vendor themselves do not. I think it would be really unfortunate were GCC to add a C++ attribute named [[clang::frobble]] that Clang does not implement, and I don't see this case as being all that different. My belief is that GCC will eventually elect to make most of these attributes available in C mode and that's an appropriate time for us to do the same for their vendor namespace.
>
> > From my perspective, lack of consistency here between Clang's C and C++ modes is much more problematic than a lack of consistency between what Clang and GCC implement.
>
> From my perspective, they're both problems in their own right. To me (and maybe I'm weird with this line of reasoning), the only reasonable time to implement an attribute under another vendor's attribute namespace is when you are promising your users that you will attempt to match the owning vendor's semantics for that attribute. A case could be made here that the owning vendor *has* implemented that attribute (since they have in C++), but I'm not too comfortable *assuming* that the GCC folks are okay with this since they don't implement the feature syntax in C yet.
>
> That said, I'm happy to ask Jason at the meetings in Albuquerque to explore the idea -- but I don't think it should hold up this patch, especially since we have our own vendor attributes we can use for gaining experience.


I certainly understand your perspective, but this is an orthogonal concern. If this is something that Clang does, then it should do it consistently. If you'd like us not to support `gnu::` attributes that GCC itself does not support, and that's something that we currently do in C++, then please submit a patch to fix that for all language modes. It should not differ between language modes.

Is the problem here that we're treating `gnu::`, not as a vendor prefix, but as generic escape hatch to get to anything generally provided via GCC-attribute syntax (which many compilers, including ours, have extended with attributes that GCC does not itself support)?


https://reviews.llvm.org/D37436