[libcxx-dev] How to check for a feature-test macro

[Richard Smith via libcxx-dev]

On Fri, 7 Dec 2018 at 13:51, Marshall Clow <mclow.lists at gmail.com> wrote:

> On Thu, Dec 6, 2018 at 11:41 AM Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Thu, 6 Dec 2018 at 09:58, Marshall Clow <mclow.lists at gmail.com> wrote:
>>
>>> On Wed, Dec 5, 2018 at 5:21 PM Richard Smith <richard at metafoo.co.uk>
>>> wrote:
>>>
>>>> On Tue, 4 Dec 2018 at 19:34, Marshall Clow via libcxx-dev <
>>>> libcxx-dev at lists.llvm.org> wrote:
>>>>
>>>>> In the tests, we have the following usage:
>>>>>
>>>>> #if TEST_STD_VER > 14
>>>>> # if !defined(__cpp_lib_filesystem)
>>>>> #  error "__cpp_lib_filesystem is not defined"
>>>>> # elif __cpp_lib_filesystem < 201703L
>>>>> #  error "__cpp_lib_filesystem has an invalid value"
>>>>> # endif
>>>>> #endif
>>>>>
>>>>> I submit that that's non-portable, because some standard libraries may
>>>>> not implement all the features (that's the point of the feature-test
>>>>> macro), and this test should not fail.
>>>>>
>>>>
>>>> Maybe this is lack of imagination on my part, but isn't a test failure
>>>> for an unimplemented standard library feature exactly the desired behavior?
>>>>
>>>
>>> No. The test here is for the correct definition of the macro in question.
>>> With the adoption of feature test macros, the committee has explicitly
>>> sanctioned the idea of incomplete implementations.
>>>
>>
>> Maybe I'm misunderstanding what you mean, but I don't agree. An
>> incomplete implementation is non-conforming, just as it always was. No part
>> of the language or library is optional. All parts are required to be
>> implemented, and all feature test macros are required to be defined, by a
>> conforming implementation.
>>
>
> I believe that this is a red herring. If all we were concerned about were
> "conforming implementations", then there would be no need for feature test
> macros. All conforming implementations have implemented all features, so
> there's no need to check.
>
> But that's not what the committee said in adopting SD-6. They said "We
> recognize that there are incomplete implementations, and we're providing
> mechanisms for working with them".
>
>
> The existence of feature test macros do not sanction the absence of
>> features in a conforming implementation, they merely provide a mechanism by
>> which a user of an implementation can query whether they're using an
>> incomplete (and therefore non-conforming) implementation, and if so, which
>> parts are available and which parts are still work in progress.
>>
>> In D55308, I am using the following form:
>>>>>
>>>>> #if TEST_STD_VER > 17
>>>>>   LIBCPP_ASSERT(IS_DEFINED(__cpp_lib_char8_t)); // libc++ implements
>>>>> this
>>>>> # if defined(__cpp_lib_char8_t)
>>>>> #  if __cpp_lib_char8_t < 201811L
>>>>> #   error "__cpp_lib_char8_t has an invalid value"
>>>>> #  endif
>>>>> # endif
>>>>> #endif
>>>>>
>>>>> Basically it (unconditionally) checks that if the macro is defined,
>>>>> then it has a sane value.
>>>>> Additionally, since we know that libc++ defines this, we check that.
>>>>>
>>>>> An alternate formulation - w/o the `IS_DEFINED` trickery.
>>>>>
>>>>> #if TEST_STD_VER > 17
>>>>> # if !defined(__cpp_lib_char8_t)
>>>>>   LIBCPP_STATIC_ASSERT(false, "__cpp_lib_char8_t is not defined");
>>>>> # else
>>>>> #  if __cpp_lib_char8_t < 201811L
>>>>> #   error "__cpp_lib_char8_t has an invalid value"
>>>>> #  endif
>>>>> # endif
>>>>> #endif
>>>>>
>>>>> Comments welcome.
>>>>>
>>>>
>>>> A standard library implementation that partially supports a feature may
>>>> define the feature test macro to a value lower than the standard one. Your
>>>> revised approach would reject that.
>>>>
>>>
>>> It would; I haven't considered that.
>>>
>>>
>>>> There are two things that I think are reasonable:
>>>>
>>>> 1) Condition all your tests on the relevant feature test macros, and add
>>>>
>>>> LIBCPP_STATIC_ASSERT(__cpp_lib_char8_t >= 201811L, "libc++ should
>>>> implement this");
>>>>
>>>> to make sure that libc++ in particular behaves as expected, or
>>>>
>>>> 2) Write tests that test for conformance: the feature test macros
>>>> should be correct and the functionality should be available. That way a
>>>> standard library implementation that fails to implement a feature (and
>>>> doesn't advertise the feature via feature test macros) doesn't get an "all
>>>> tests pass" despite not implementing all of the standard library.
>>>>
>>>> I'd lean towards option 2 myself, but maybe vendors of other stdlibs
>>>> using the libc++ testsuite might have a different opinion.
>>>> <http://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-dev>
>>>>
>>>
>>> I'd rather avoid that; long-term test failures are not in anyones
>>> best-interest.
>>> It leads to people ignoring the test results "oh, yeah we always have
>>> test failures"
>>>
>>
>> Test failures on tests for features that you don't implement seem like
>> exactly the right behavior to me. Isn't that already the status quo for the
>> rest of libc++'s test suite? If someone doesn't implement, say,
>> std::unique_ptr, shouldn't all the unique_ptr tests fail for their
>> implementation?
>>
>
> Really? do you expect that the clang test suite will fail ON EVERY TEST
> RUN because clang lacks support for concepts (as an example)?
> Or explicit(bool) ?
>
> I'm not seeing that reflected in the test matrix. I see lots of green for
> clang.
>

Green is not the same as "all tests pass", it's the same as "all tests that
were expected to pass passed, and all tests that were expected to fail
failed". I would expect Clang's test suite to be green. I would not expect
all tests to pass. Specifically, if we had implemented tests for those new
features prior to implementing the corresponding Clang support, it would
make sense to me to XFAIL those tests.

If we wanted to share a testsuite between Clang and other compilers, the
strategy I described in my previous message (condition tests under the
relevant feature test macro, and have separate tests that all the feature
test macros are defined appropriately, with XFAILs for compilers that don't
support the feature yet) seems entirely reasonable to me for that.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-dev/attachments/20181207/edba1da2/attachment-0001.html>