__has_feature coverage

Clang Frontend
1

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

features.cpp (4.35 KB)

2

The fix is easy, if the traits are fully implemented?

traits.patch (1.19 KB)

3

IIRC there were concerns that __has_feature might not be the right place to be surfacing these traits.

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?)

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.

4

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)

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.

Alp.

5

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).

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).

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 :slight_smile: ).

6

        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

        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.

7

        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

8

Probably historical reasons; but I was under the impression that __has_feature was the shiny, cross-compiler way to tell if a compiler had implemented some feature. [ People use libc++ on other compilers than clang ]

A quick search turned up no uses of __has_extension in libc++

[ Apologies for belaboring the obvious, but …]
For libc++ to use a compiler intrinsic, I have to know if it’s available - so I need a test, and
I’d like a method that works for different compilers.

— Marshall

9

        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.)

Probably historical reasons; but I was under the impression that
__has_feature was the shiny, cross-compiler way to tell if a compiler had
implemented some feature. [ People use libc++ on other compilers than
clang ]

__has_feature is not cross-compiler, it's a Clang extension just like
__has_extension. The shiny cross-compiler way is the SG10 __cpp_* macros
(but it's murky whether they should map to __has_feature or
__has_extension, and they're not widely supported yet, and they don't yet
cover all the things you want to test, so they're probably not exactly what
you want here.)

A quick search turned up no uses of __has_extension in libc++

[ Apologies for belaboring the obvious, but ..]
For libc++ to use a compiler intrinsic, I have to know if it’s available -
so I need a test, and
I’d like a method that works for different compilers.

If that's the case, then your current __has_feature checks are not doing
what you want.