Can someone please review this patch? Thank you! Daniel
On Mon, Jun 25, 2012 at 5:41 PM, Daniel Jasper <[email protected]> wrote: > > > On Thu, Jun 21, 2012 at 7:44 PM, John McCall <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev >> > >
has_feature.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
