On Mon, 7 Jan 2019 at 16:12, Erik Pilkington via cfe-commits < 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> 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@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 >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
