r350572 - Add a __has_feature check for namespaces on #pragma clang attribute.

[Richard Smith via cfe-commits]

On Mon, 7 Jan 2019 at 16:12, Erik Pilkington via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> On 1/7/19 3:51 PM, Richard Smith wrote:
>
> On Mon, 7 Jan 2019 at 13:57, Erik Pilkington via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: epilk
>> Date: Mon Jan  7 13:54:00 2019
>> New Revision: 350572
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=350572&view=rev
>> Log:
>> Add a __has_feature check for namespaces on #pragma clang attribute.
>>
>
> Should this be __has_extension rather than __has_feature, since it's not a
> standard feature?
>
>
> I suppose, but it doesn't really seem like that's the rule that we're
> following here. If you look at every other FEATURE(...) above this, they
> all have to do with clang extensions like attributes and sanitizers, which
> obviously aren't standard features. Every EXTENSION(...) has to do with
> language features that are shared between languages (cxx_fixed_enum in C,
> for instance). So it seems like the most internally consistent place to put
> this is in FEATURE(...). WDYT?
>
What you're seeing is a historical legacy of the time before the current
approach. The design and documented intent of __has_feature is that it
checks for standardized features. See the documentation here, and
particularly the note about backwards compatibility:

https://clang.llvm.org/docs/LanguageExtensions.html#has-feature-and-has-extension

... and the design discussion when __has_extension was introduced:

http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20110425/041452.html

... and the comment on HasFeature in Lex/PPMacroExpansion.cpp:

/// HasFeature - Return true if we recognize and implement the feature
/// specified by the identifier as a standard language feature.

We seem to have detached that comment from the actual list of features when
the features were moved to a .def file. Oops :(

If we want to revisit the design based on experience using __has_feature /
__has_extension, I'm all for that (the distinction between the two seems
confusing and not especially useful to me; the behavior of the current
language mode can be tested using eg __STDC_VERSION__ and __cplusplus, so
the only thing that's really interesting for us to expose is what features
the current compliation supports), but we should follow the current design
until we do.

> Support for this was added in r349845.
>>
>> Modified:
>>     cfe/trunk/docs/LanguageExtensions.rst
>>     cfe/trunk/include/clang/Basic/Features.def
>>     cfe/trunk/test/Sema/pragma-attribute-namespace.c
>>
>> Modified: cfe/trunk/docs/LanguageExtensions.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=350572&r1=350571&r2=350572&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/docs/LanguageExtensions.rst (original)
>> +++ cfe/trunk/docs/LanguageExtensions.rst Mon Jan  7 13:54:00 2019
>> @@ -2725,7 +2725,9 @@ same namespace. For instance:
>>  Without the namespaces on the macros, ``other_function`` will be
>> annotated with
>>  ``[[noreturn]]`` instead of ``__attribute__((unavailable))``. This may
>> seem like
>>  a contrived example, but its very possible for this kind of situation to
>> appear
>> -in real code if the pragmas are spread out across a large file.
>> +in real code if the pragmas are spread out across a large file. You can
>> test if
>> +your version of clang supports namespaces on ``#pragma clang attribute``
>> with
>> +``__has_feature(pragma_clang_attribute_namespaces)``.
>>
>>  Subject Match Rules
>>  -------------------
>>
>> Modified: cfe/trunk/include/clang/Basic/Features.def
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=350572&r1=350571&r2=350572&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/Features.def (original)
>> +++ cfe/trunk/include/clang/Basic/Features.def Mon Jan  7 13:54:00 2019
>> @@ -69,6 +69,7 @@ FEATURE(attribute_overloadable, true)
>>  FEATURE(attribute_unavailable_with_message, true)
>>  FEATURE(attribute_unused_on_fields, true)
>>  FEATURE(attribute_diagnose_if_objc, true)
>> +FEATURE(pragma_clang_attribute_namespaces, true)
>>  FEATURE(blocks, LangOpts.Blocks)
>>  FEATURE(c_thread_safety_attributes, true)
>>  FEATURE(cxx_exceptions, LangOpts.CXXExceptions)
>>
>> Modified: cfe/trunk/test/Sema/pragma-attribute-namespace.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/pragma-attribute-namespace.c?rev=350572&r1=350571&r2=350572&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Sema/pragma-attribute-namespace.c (original)
>> +++ cfe/trunk/test/Sema/pragma-attribute-namespace.c Mon Jan  7 13:54:00
>> 2019
>> @@ -1,5 +1,9 @@
>>  // RUN: %clang_cc1 -fsyntax-only -verify %s
>>
>> +#if !__has_feature(pragma_clang_attribute_namespaces)
>> +#error
>> +#endif
>> +
>>  #pragma clang attribute MyNamespace.push (__attribute__((annotate)),
>> apply_to=function) // expected-error 2 {{'annotate' attribute}}
>>
>>  int some_func(); // expected-note{{when applied to this declaration}}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190107/d802ca00/attachment-0001.html>