I added support for __has_declspec_attribute in r223467, and modified support for __has_attribute in r223468. I will keep an eye on the lists and bots to ensure the latter does not have unintended side effects.
Thanks! ~Aaron On Thu, Dec 4, 2014 at 9:47 PM, Aaron Ballman <[email protected]> wrote: > On Thu, Dec 4, 2014 at 7:16 PM, Richard Smith <[email protected]> wrote: >> On Thu, Dec 4, 2014 at 7:22 AM, Aaron Ballman <[email protected]> >> wrote: >>> >>> Ping >>> >>> On Mon, Nov 24, 2014 at 10:02 AM, Aaron Ballman <[email protected]> >>> wrote: >>> > On Thu, Nov 20, 2014 at 7:22 PM, Richard Smith <[email protected]> >>> > wrote: >>> >> On Thu, Nov 20, 2014 at 3:48 PM, Aaron Ballman <[email protected]> >>> >> wrote: >>> >>> >>> >>> On Thu, Nov 20, 2014 at 6:27 PM, Richard Smith <[email protected]> >>> >>> wrote: >>> >>> > On Thu, Nov 20, 2014 at 1:38 PM, Aaron Ballman >>> >>> > <[email protected]> >>> >>> > wrote: >>> >>> >> >>> >>> >> The __has_attribute implementation does not pay attention to the >>> >>> >> syntax supported by attributes -- instead, it looks to see whether >>> >>> >> an >>> >>> >> attribute is generally known with that spelling. Since pragmas can >>> >>> >> now >>> >>> >> be spelled as attributes, this means __has_attribute(loop) returns >>> >>> >> true because of the #pragma loop functionality. Same for unroll. >>> >>> >> >>> >>> >> Should __has_attribute ignore attributes spelled with a #pragma >>> >>> >> spelling? >>> >>> > >>> >>> > >>> >>> > I would go further: __has_attribute should probably only look for >>> >>> > GNU-syntax >>> >>> > attributes. We have __has_cpp_attribute for C++-syntax attributes >>> >>> > now, >>> >>> > and I >>> >>> > don't think anyone is (yet) using this for __declspec, so now seems >>> >>> > like >>> >>> > a >>> >>> > good time to make this change. >>> >>> >>> >>> A long, long while back, we discussed having a way to determine >>> >>> attributes by syntax, because that's sometimes important, as well as a >>> >>> general query mechanism. >>> >>> >>> >>> How about adding: >>> >>> >>> >>> __has_declspec_attribute >>> >>> __has_keyword_attribute >>> >>> __has_gnu_attribute >>> >>> >>> >>> and leaving __has_attribute generic across syntaxes? >>> >>> >>> >>> This also reduces the chances of breaking code by allowing the >>> >>> __has_attribute syntax to continue to work as it always has. >>> >> >>> >> >>> >> Back when we only had GNU attributes, that's all it detected. I'm not >>> >> convinced that people are actually using it for anything else, >>> > >>> > I believe I've seen some code using it for __declspec attributes. >>> > IIRC, we may have run into this with MinGW, which implements >>> > __declspec as a macro, replaced by __attribute__. >>> > >>> >> and I think >>> >> it would be surprising if it said an attribute was supported but that >>> >> attribute didn't work with GNU syntax. A generic-across-syntaxes >>> >> __has_attribute is basically useless. >>> > >>> > The more I think about cross-syntax attribute checking, the more I >>> > agree it's useless. >>> > >>> > I still think it would make sense to add the various forms of this, >>> > and I definitely think that pragmas should be excluded from >>> > __has_attribute. >>> > >>> > How does this sound as a path forward: >>> > >>> > * Change __has_attribute to only support GNU-style attributes. This >>> > has the potential to break code, so this will require careful watching >>> > of the lists. >>> > * Add __has_declspec_attribute & __has_keyword_attribute, that only >>> > apply to __declspecs and keywords. >> >> >> Sounds good to me, except perhaps for the __has_keyword_attribute part. We >> already provide that functionality through __is_identifier, and it's not >> completely clear to me that we want both mechanisms > > Agreed (also agree with your implementation detail comment). I'll drop > that from what I produce. > > Thanks! > > ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
