<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Dec 7, 2018, at 19:25, Richard Smith via libcxx-dev <<a href="mailto:libcxx-dev@lists.llvm.org" class="">libcxx-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><div class="gmail_quote"><div dir="ltr" class="">On Fri, 7 Dec 2018 at 13:51, Marshall Clow <<a href="mailto:mclow.lists@gmail.com" class="">mclow.lists@gmail.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div dir="ltr" class=""><div class="gmail_quote"><div dir="ltr" class="">On Thu, Dec 6, 2018 at 11:41 AM Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank" class="">richard@metafoo.co.uk</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div class="gmail_quote"><div dir="ltr" class="">On Thu, 6 Dec 2018 at 09:58, Marshall Clow <<a href="mailto:mclow.lists@gmail.com" target="_blank" class="">mclow.lists@gmail.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, Dec 5, 2018 at 5:21 PM Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank" class="">richard@metafoo.co.uk</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div class="gmail_quote"><div dir="ltr" class="">On Tue, 4 Dec 2018 at 19:34, Marshall Clow via libcxx-dev <<a href="mailto:libcxx-dev@lists.llvm.org" target="_blank" class="">libcxx-dev@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class="">In the tests, we have the following usage:<div class=""><br class=""></div><div class=""><div class="">#if TEST_STD_VER > 14</div><div class=""># if !defined(__cpp_lib_filesystem)</div><div class=""># error "__cpp_lib_filesystem is not defined"</div><div class=""># elif __cpp_lib_filesystem < 201703L</div><div class=""># error "__cpp_lib_filesystem has an invalid value"</div><div class=""># endif</div><div class="">#endif</div></div><div class=""><br class=""></div><div class="">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.</div></div></div></div></div></blockquote><div class=""><br class=""></div><div class="">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?</div></div></div></blockquote><div class=""><br class=""></div><div class="">No. The test here is for the correct definition of the macro in question.</div><div class="">With the adoption of feature test macros, the committee has explicitly sanctioned the idea of incomplete implementations.</div></div></div></blockquote><div class=""><br class=""></div><div class="">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.</div></div></div></blockquote><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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".</div><div class=""><br class=""></div><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div class="gmail_quote"><div class="">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.</div><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">In D55308, I am using the following form:</div><div class=""><br class=""></div><div class=""><div class="">#if TEST_STD_VER > 17</div><div class=""> <span class="Apple-converted-space"> </span>LIBCPP_ASSERT(IS_DEFINED(__cpp_lib_char8_t)); // libc++ implements this</div><div class=""># if defined(__cpp_lib_char8_t)</div><div class=""># if __cpp_lib_char8_t < 201811L</div><div class=""># error "__cpp_lib_char8_t has an invalid value"</div><div class=""># endif</div><div class=""># endif</div><div class="">#endif</div></div><div class=""><br class=""></div><div class="">Basically it (unconditionally) checks that if the macro is defined, then it has a sane value.</div><div class="">Additionally, since we know that libc++ defines this, we check that.</div><div class=""><br class=""></div><div class="">An alternate formulation - w/o the `IS_DEFINED` trickery.</div><div class=""><br class=""></div><div class=""><div class=""><div class="">#if TEST_STD_VER > 17</div><div class=""># if !defined(__cpp_lib_char8_t) </div><div class=""> <span class="Apple-converted-space"> </span>LIBCPP_STATIC_ASSERT(false, "__cpp_lib_char8_t is not defined");</div><div class=""># else</div><div class=""># if __cpp_lib_char8_t < 201811L</div><div class=""># error "__cpp_lib_char8_t has an invalid value"</div><div class=""># endif</div><div class=""># endif</div><div class="">#endif</div></div><div class=""><br class=""></div><div class="">Comments welcome.</div></div></div></div></div></div></blockquote><div class=""><br class=""></div><div class="">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.</div></div></div></blockquote><div class=""><br class=""></div><div class="">It would; I haven't considered that.</div><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div class="gmail_quote"><div class="">There are two things that I think are reasonable:</div><div class=""><br class=""></div><div class="">1) Condition all your tests on the relevant feature test macros, and add</div><div class=""><br class=""></div><div class="">LIBCPP_STATIC_ASSERT(__cpp_lib_char8_t >= 201811L, "libc++ should implement this");</div><div class=""><br class=""></div><div class="">to make sure that libc++ in particular behaves as expected, or</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">I'd lean towards option 2 myself, but maybe vendors of other stdlibs using the libc++ testsuite might have a different opinion.<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-dev" rel="noreferrer" target="_blank" class=""></a><br class=""></div></div></div></blockquote><div class=""><br class=""></div><div class="">I'd rather avoid that; long-term test failures are not in anyones best-interest.</div><div class="">It leads to people ignoring the test results "oh, yeah we always have test failures"<br class=""></div></div></div></blockquote><div class=""><br class=""></div><div class="">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?</div></div></div></blockquote><div class=""><br class=""></div><div class="">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) ?</div><div class=""><br class=""></div><div class="">I'm not seeing that reflected in the test matrix. I see lots of green for clang.</div></div></div></div></blockquote><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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.</div></div></div></div></blockquote><div><br class=""></div><div>Yes, I think the strategy you outline makes the most sense too. It allows us to keep the test suite a conformance suite, which is what we want (I think?), while still allowing incomplete implementations.</div><div><br class=""></div><div>This will either require enclosing all tests for features that might not be implemented inside `#ifdef <feature-test-macro>`, or doing it at the LIT level using one feature for each feature-test-macro and then using `XFAIL: !<feature-test-macro>`. And then in `test/libcxx`, we can add tests for all the feature test macros that we should be defining. Other implementations can do something similar when they run the test suite for their implementation.</div><div><br class=""></div><div>I dislike this as it makes everything more complicated and more brittle, but it seems like this is the best way to go given the need to implement feature-test macros. <cries a little></div><div><br class=""></div><div>We should also see how other implementations that use our test suite feel about this (CCing them).</div></div><br class=""><div class="">Louis</div><div class=""><br class=""></div></body></html>