On 1/7/19 4:43 PM, Richard Smith wrote:
On Mon, 7 Jan 2019 at 16:12, Erik Pilkington via cfe-commits
<cfe-commits@lists.llvm.org <mailto:cfe-commits@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@lists.llvm.org <mailto:cfe-commits@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.
Yeah, it doesn't really seem like a useful distinction. Anyways, I added
a comment to this file and fixed this in r350642. Thanks!
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@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
