[cfe-commits] [cfe-dev] Determine whether the current clang version has a specific bug

[Daniel Jasper]

Can someone please review this patch?

Thank you!
Daniel

On Mon, Jun 25, 2012 at 5:41 PM, Daniel Jasper <djasper at google.com> wrote:

>
>
> On Thu, Jun 21, 2012 at 7:44 PM, John McCall <rjmccall at apple.com> wrote:
>
>> On Jun 21, 2012, at 8:56 AM, Jordan Rose wrote:
>> > On Jun 21, 2012, at 1:06 AM, Sebastian Redl wrote:
>> >> I had the same idea (for some app's plugin API, but the principle is
>> the same). In this case, however, I think we should give the builtin a
>> clang-specific name. __has_feature and __has_extension could be done the
>> same way by other compilers with matching feature names, and code would
>> profit. However, another compiler is unlikely to have exactly the same bug,
>> or realize it and come up with the same name scheme (I would just use
>> Bugzilla numbers). What's more, since all unknown bug names are considered
>> not fixed, that would mean that each such test would need a compiler
>> predicate first.
>> >
>> > I'm actually very disturbed by the idea of __has_bug / __has_clang_bug
>> because of this. There is no way to sync bug numbers up across compilers,
>> especially if there's a bug in Clang that we fix in version X that always
>> behaved correctly in GCC. The advantage of __has_feature and friends is
>> that they're pessimistic -- if you get a 1, you know you can use the
>> feature. Getting a 1 from __has_bug might just mean it's not a bug to begin
>> with.
>> >
>> > (What counts as a fixed bug? When we add a feature in SVN rXXX and then
>> close a PR a month later when we notice it's been fixed, what's the right
>> thing to do? What about regressions? Who is going to update the list when
>> they fix a bug? Do our internal bugs count as bugs? Do our incremental
>> fixes on longer projects count as bugs?)
>> >
>> > The motivating use case is indeed motivating, since you get a warning
>> if you do include [[unused]] on your private fields in old compilers (and
>> in GCC), and a warning if you don't in new-Clang. And here __has_bug is
>> being used pessimistically as well. But I don't think this is the way to
>> solve the problem in general. Because __has_bug is vendor-specific, it's no
>> better than comparing version numbers (trunk is not supposed to be stable)
>> and probably not really more semantic. (If we came up with unique
>> identifiers for the bugs it would be a little better, but that's more
>> effort that I honestly don't think is necessary.)
>> >
>> > For this one specific case, I'd rather extend __has_attribute to allow
>> an optional context for the attribute. Another possibility would be to add
>> __has_warning, but I'd be concerned that people would start using this to
>> conditionally comment out code when compiling with certain warnings. (I
>> haven't really thought that one through.)
>>
>> __has_bug is a maintenance / code-bloat nightmare by design, and people
>> should feel bad for proposing it. :)
>>
>
> I won't tell, who first proposed it ;-).
>
>
>> It would be totally reasonable to have a
>> __has_feature(unused_attribute_on_fields), though.
>>
>
> Implemented in the attached patch, kindly asking for a review. I have
> chosen __has_feature(attribute_unused_on_fields) to better fit with the
> others.
>
> Cheers,
> Daniel
>
>
>> John.
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120627/ea5ffdb0/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: has_feature.patch
Type: application/octet-stream
Size: 1144 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120627/ea5ffdb0/attachment.obj>