[cfe-dev] [PATCH] New syntax and functionality for __has_attribute

[Aaron Ballman]

On Sun, Jan 12, 2014 at 1:22 PM, dblaikie at gmail.com <dblaikie at gmail.com> wrote:
>
>> It's likely I didn't understand the question, so keep poking me until
>> I get it right. :-)
>
>
> Let's see how we go.
>
>>
>>
>> > What I'm asking is, is there ever a case where a compile-time check is
>> > required to decide which syntax to use? (where two compilers (including
>> > two
>> > different versions of the same compiler) have support for the same
>> > attribute
>> > but in disparate syntaxes?)
>> >
>> > The existence of an attribute with more than one, or less than all
>> > syntaxes
>> > is not an issue - if in every compiler that implements the attribute it
>> > is
>> > implemented in the same set of syntaxes, then there's no need to query
>> > which
>> > syntax is valid, is there? It would be a universal constant.
>>
>> I may still be totally misunderstanding your concern, so I apologize
>> if I'm being dense. But yes, there are cases where this comes up.
>> Going back to my dllexport example since I think it's the most
>> easily-recognizable. Here's a probably example of using this code in a
>> cross-compiler code base:
>>
>> #if __has_attribute(dllexport)
>>   #define DLLEXPORT __attribute__((dllexport))
>> #else
>>   #define DLLEXPORT
>> #endif
>>
>> void func(void) DLLEXPORT;
>>
>> If you were to use this code today in clang, it would fail to compile.
>>  __has_attribute(dllexport) will be replaced with 1, and so the
>> resulting code would become: void func(void)
>> __attribute__((dllexport));  However, we do not support dllexport with
>> that syntax (we only support __declspec(dllexport)). So the code would
>> look reasonable, but fail to compile.
>
>
> Sure - my argument here is this is /potentially/ just a lack of
> documentation. It's perhaps no more relevant than having a macro that tells
> you whether the return type is written "<here> func();" or "func() void" -
> it's an invariant of the language (or an invariant of the language
> extension, in the attribute case), not something that needs compile-time
> detection.
>
>>
>> We could solve this by making __has_attribute *only* correspond to
>> GNU-style attributes, but that's not how it works currently, and such
>> a change could break existing code that currently works.
>
>
> I don't think that's necessary. If each attribute has a documented list of
> syntaxes/valid places, that should be sufficient.

Then why have __has_attribute in the first place? After all, each
vendor has a documented list of attributes they support, and there are
ways to determine which vendor-specific version is compiling the code.

This is a feature-test macro. It makes sense to test which feature is
supported. __has_attribute(ident) does not provide sufficient
resolution to determine whether the feature is supported. The proposed
feature has the granularity required, and can be written in a
future-proof manner (unlike the existing implementation).

>
>>
>> Not every compiler implements the same set of attributes, which is why
>> __has_attribute exists in the first place.
>
>
> Sure - I get that, I'm not questioning the motivation for __has_attribute as
> it stands today. I'm questioning the value of the ability to query for
> particular attribute syntax/spelling support that's being added here.

The rationale is the same either way. Outside of the
standards-required attributes, each vendor can choose to implement any
syntax and spelling (or combination thereof) they desire, so having a
test for syntax can be necessary.

>>
>> Furthermore, not every
>> compiler implements a given attribute with the same set of syntaxes
>> (such as dllexport, which is implemented in gcc as both a GNU-style
>> and an MS-style attribute, but it only an MS-style attribute in
>> clang).
>
>
> OK - this is what I'm interested in, but on further consideration I think
> there's an even stronger requirement:
>
> For the has_feature per-syntax checking to be useful, we would have to be in
> a world in which two compilers (including different versions of the same
> compiler) support the same attribute in non-overlapping syntaxes.
>
> Anyone trying to write portable code isn't going to write:
>
> #if __has_attribute(__attribute__((dllexport)))
>   #define DLLEXPORT __attribute__((dllexport))
> #elif __has_attribute(declspec(dllexport))
>   #define DLLEXPORT declspec(dllexport)
> #else
>   #define DLLEXPORT
> #endif
>
> They're just going to use the common subset that all compiler support
> (because generally these attributes have been implemented for compatibility
> with other compilers, yes?), thus just preferring the MS syntax in all
> cases.
>
> One difference would be that having specific syntax feature detection would
> allow safe fallback - if someone had written the dllexport with GCC syntax
> using this new feature then on Clang it would've silently not exported (do
> people actually ever want that? For some attributes there's a logical
> fallback to just not use the attribute, dllexport/import doesn't seem to be
> one of those - but that's an aside) - whereas without the syntax-specific
> check you'd get an error and need to update the attribute syntax used in the
> DLLEXPORT macro to the one that's common between GCC and Clang (& whatever
> else).
>
>>
>> Even in situations where the compiler *does* implement all of
>> the syntaxes consistently, it can be beneficial to know which syntax
>> you are referring to within the context of __has_attribute because
>> attribute placement is different between varying attribute syntaxes.
>
>
> Again, this knowledge would be gained through explicit compiler
> documentation (as any other language feature or language extension) - not
> necessarily meriting a compile-time query system.

Consider, for instance, a vendor with pluggable attributes (which is
something clang could definitely make use of). The vendor may not have
the documentation for the plugin attributes, and discoverability would
be important because the attribute can be written different ways with
varying syntaxes based on each pluggable attribute.

Also, as I mentioned above, the same argument could be made for any of
the feature test macros. Saying "but there's documentation" is not a
compelling argument to me.

>> Does that clear things up, or am I still not understanding your concerns?
>
>
> We're closer on the concerns, but I'm still not quite understanding the
> justification.

A feature request was filed asking for improved behavior, which was
discussed and expanded upon by others, then designed and implemented
by me. So it can be stated that several people felt there's a
justification for this incremental improvement, and I've tried my best
to explain reasons why this is justified.

Do you have technical concerns about the way it is designed or implemented?

~Aaron