[cfe-dev] __has_feature coverage
Richard Smith
richard at metafoo.co.uk
Mon May 5 13:09:07 PDT 2014
On Fri, May 2, 2014 at 1:00 AM, Alp Toker <alp at nuanti.com> wrote:
>
> On 02/05/2014 07:35, Richard Smith wrote:
>
> On Thu, May 1, 2014 at 10:36 PM, Alp Toker <alp at nuanti.com <mailto:
>> alp at nuanti.com>> wrote:
>>
>>
>> On 02/05/2014 05:34, Richard Smith wrote:
>>
>> IIRC there were concerns that __has_feature might not be the
>> right place to be surfacing these traits.
>>
>>
>> Right, I took some notes back when I was looking into this and my
>> findings were:
>>
>> __has_feature() is intended for standardised language features and
>> it's prominently user-facing so not really appropriate for trait
>> primitives whether or not they're feature-complete. Once an
>> alternative is found we should phase those out.
>>
>> __has_extension() also suffers from being user-facing, but perhaps
>> less so.
>>
>> __has_builtin() would let us automatically list all built-in trait
>> primitives and I did some work to support that but it wasn't clear
>> what value this adds so I've put that aside.
>>
>> Observation: There's no need to list "compatibility-only" or
>> poorly defined trait primitives in any detection macro because
>> they only exist for compatibility with MSVC and GCC which use
>> without checking, with the possible exception of Embarcadero(?).
>>
>> As you see this is a bit of a tangle. As a way forward the viable
>> options to provide a feature test for _quality_ type trait
>> primitives could be one of:
>>
>> a) A new __has_primitive()
>> b) Shoehorning into __has_extension() but naming the features more
>> clearly as primitives: e.g.
>> __has_extension(primitive_is_constructible)
>>
>>
>> Another option is recommending people use __is_identifier, which already
>> works for this purpose (because we treat these tokens as keywords), but I'd
>> be against that one since we can't mark a trait as unavailable when it's in
>> a not-fully-working state.
>>
>> One advantage of sticking with __has_feature or __has_extension is that
>> we don't immediately gain a legacy deprecated interface to support and be
>> unhappy about =) Here's what our documentation says:
>>
>> "For type trait __X, __has_extension(X) indicates the presence of the
>> type trait primitive in the compiler."
>>
>> We could stick with that, using the is_ prefix as our indicator that the
>> extension is a type trait (and adding these and any further type traits
>> only to __has_extension and not to __has_feature).
>>
>
> Yes, I think __has_extension() is the way to go.
>
> I wonder however if the feature names need a clearer prefix given that the
> C++ standard library is considered part of the implementation.
>
> The existing names makes it sound to users as though this makes sense:
>
> #if __has_feature(is_constructible) || __has_extension(is_constructible)
> std::is_constructible<T>::value
> #else
> ...
> #endif
I don't feel particularly uncomfortable about this. If users are using
__has_feature without reading the documentation (maybe because they're
cargo-culting something?), I think we're justified in saying they're doing
it wrong. I'd prefer to keep the names the same (but to only document the
__has_extension checks going forward).
Marshall: libc++ has a lot of __has_feature checks. Is there a good reason
you're not using __has_extension that you know of? (__has_extension always
returns true for a superset of the cases where __has_feature does.)
Some of the traits don't work in general (and support just
>> enough to get through the MS headers); I don't recall exactly
>> which ones, though, and I think it was more than just
>> __is_*_destructible. Alp, do you recall? (Maybe it was the
>> __is_nothrow_* ones?)
>>
>>
>> Many of the _is_ ones are sketchy. I cooked up some tests to
>> compare them against GCC and while compatibility was good, overall
>> usefulness wasn't clear and some of the results were sketchy. I'll
>> see if I can dig this up but the results may be lost (wish we had
>> a wiki to throw stuff like this).
>>
>> Meanwhile our tests for the trait primitives have inconsistent
>> coverage and fixing them would be a lot of work. Fortunately the
>> libc++ test suite has great coverage and it might be possible to
>> reuse that work..
>>
>> Marshall, do you think a strategically placed static_assert()
>> could be added somewhere in your type trait tests to compare
>> answers from the clang builtin type trait primitives against your
>> quality library implementations?
>>
>>
>>
>> Do we advertise support for the __has_* ones? We should stop
>> doing that if we do -- they're fundamentally broken and pretty
>> much useless (they exist to support libstdc++ and give wrong
>> answers in some cases for GCC compatibility). Eventually I
>> would ilke to make them synonyms for the correct traits, but
>> I'm concerned that people might be relying on the GCC bugs
>> that we emulate for them.
>>
>>
>> My understanding of the Embarcadero usage is that they _DO_ use
>> the feature detection macros. If this is indeed the case (they
>> should comment, do we have a contact?) then removing them from
>> __has_feature() is tantamount to removing them completely. I'm not
>> at all opposed to that but it should be done after we get the
>> facts from the original authors.
>>
>>
>> I'm not sure whether we're talking about the same traits. Just to make
>> sure, I mean __has_trivial_* and __has_nothrow_* (which GCC added a few
>> years ago based on the then-C++0x type traits proposal, which got reworked
>> substantially before C++11).
>>
>
> Oh right. The __has ones have issues like PR16627 and ignoring members
> with default arguments. For the __is ones, there are various quirks and
> FIXMEs in SemaExprCXX.cpp which need going through. The newly added
> primitives not marked incomplete are good for general usage though. We
> should probably only expose feature checks for the non-quirky ones going on
> a case by case basis.
>
> Alp.
>
>
>> I don't have any particular views on the __is_lvalue_expr,
>> __is_rvalue_expr, __is_same, __is_convertible, __array_* Embarcadero traits
>> (except that the names array_rank and array_extent are icky :-) ).
>>
>> Alp.
>>
>>
>>
>>
>>
>> On Thu, May 1, 2014 at 7:48 AM, Marshall Clow
>> <mclow.lists at gmail.com <mailto:mclow.lists at gmail.com>
>> <mailto:mclow.lists at gmail.com <mailto:mclow.lists at gmail.com>>>
>>
>> wrote:
>>
>> Recently, reading
>> llvm/tools/clang/docs/LanguageExtensions.rst, I
>> saw a list of type_trait primitives
>> supported by clang.
>>
>> One of them caught my eye: __is_nothrow_constructible — that
>> could be handy for use in libc++.
>>
>> So I ran a quick test:
>>
>> __is_nothrow_constructible(int)
>> and it returned true.
>>
>> Then I tried:
>> __has_feature(is_nothrow_constructible)
>> and it returned false.
>>
>> Well, that’s not useful to me; I need to be able to tell
>> when I
>> can use that feature.
>> So I whipped up a program to test all of the type_traits
>> listed in
>> that file (attached),
>> and to see if I can check (using __has_feature) whether or not
>> they are implemented.
>>
>> Turns out that __has_feature returns false for the
>> following type
>> traits:
>>
>> __has_feature(is_interface_class) = 0
>> __has_feature(is_destructible) = 0
>> __has_feature(is_nothrow_destructible) = 0
>> __has_feature(is_nothrow_assignable) = 0
>> __has_feature(is_nothrow_constructible) = 0
>>
>> __is_interface_class is a MS extension; so that’s fine.
>> __is_destructible and __is_nothrow_destructible are
>> described as
>> “partially implemented”, and attempts to use them fail.
>>
>> On the other hand:
>> __is_nothrow_constructible(int) returns 1
>> __is_nothrow_assignable(int&, int) returns 1
>>
>> So these two seem to work (for at least one case)
>> It seems to me that we should have feature tests for these
>> type
>> traits if we expect people to use them
>>
>> So, I’d like to see
>> __has_feature(is_nothrow_constructible)
>> and __has_feature(is_nothrow_assignable)
>>
>> return 1
>>
>>
>> — Marshall
>>
>>
>>
>>
>> -- http://www.nuanti.com
>> the browser experts
>>
>>
>>
> --
> http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140505/67ec8342/attachment.html>
More information about the cfe-dev
mailing list