<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, 6 Dec 2018 at 13:44, Louis Dionne <<a href="mailto:ldionne@apple.com">ldionne@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><blockquote type="cite"><div>On Dec 6, 2018, at 14:41, Richard Smith via libcxx-dev <<a href="mailto:libcxx-dev@lists.llvm.org" target="_blank">libcxx-dev@lists.llvm.org</a>> wrote:</div><br class="m_-8018434448965140409Apple-interchange-newline"><div><div dir="ltr" style="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;text-decoration:none"><div class="gmail_quote"><div dir="ltr">On Thu, 6 Dec 2018 at 09:58, Marshall Clow <<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>> wrote:<br></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"><div class="gmail_quote"><div dir="ltr">On Wed, Dec 5, 2018 at 5:21 PM Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br></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"><div class="gmail_quote"><div dir="ltr">On Tue, 4 Dec 2018 at 19:34, Marshall Clow via libcxx-dev <<a href="mailto:libcxx-dev@lists.llvm.org" target="_blank">libcxx-dev@lists.llvm.org</a>> wrote:<br></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"><div dir="ltr"><div dir="ltr"><div dir="ltr">In the tests, we have the following usage:<div><br></div><div><div>#if TEST_STD_VER > 14</div><div># if !defined(__cpp_lib_filesystem)</div><div>#  error "__cpp_lib_filesystem is not defined"</div><div># elif __cpp_lib_filesystem < 201703L</div><div>#  error "__cpp_lib_filesystem has an invalid value"</div><div># endif</div><div>#endif</div></div><div><br></div><div>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><br></div><div>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><br></div><div>No. The test here is for the correct definition of the macro in question.</div><div>With the adoption of feature test macros, the committee has explicitly sanctioned the idea of incomplete implementations.</div></div></div></blockquote><div><br></div><div>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. 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><br></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"><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"><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"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>In D55308, I am using the following form:</div><div><br></div><div><div>#if TEST_STD_VER > 17</div><div> <span class="m_-8018434448965140409Apple-converted-space"> </span>LIBCPP_ASSERT(IS_DEFINED(__cpp_lib_char8_t)); // libc++ implements this</div><div># if defined(__cpp_lib_char8_t)</div><div>#  if __cpp_lib_char8_t < 201811L</div><div>#   error "__cpp_lib_char8_t has an invalid value"</div><div>#  endif</div><div># endif</div><div>#endif</div></div><div><br></div><div>Basically it (unconditionally) checks that if the macro is defined, then it has a sane value.</div><div>Additionally, since we know that libc++ defines this, we check that.</div><div><br></div><div>An alternate formulation - w/o the `IS_DEFINED` trickery.</div><div><br></div><div><div><div>#if TEST_STD_VER > 17</div><div># if !defined(__cpp_lib_char8_t)  </div><div> <span class="m_-8018434448965140409Apple-converted-space"> </span>LIBCPP_STATIC_ASSERT(false, "__cpp_lib_char8_t is not defined");</div><div># else</div><div>#  if __cpp_lib_char8_t < 201811L</div><div>#   error "__cpp_lib_char8_t has an invalid value"</div><div>#  endif</div><div># endif</div><div>#endif</div></div><div><br></div><div>Comments welcome.</div></div></div></div></div></div></blockquote><div><br></div><div>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><br></div><div>It would; I haven't considered that.</div><div> </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"><div class="gmail_quote"><div>There are two things that I think are reasonable:</div><div><br></div><div>1) Condition all your tests on the relevant feature test macros, and add</div><div><br></div><div>LIBCPP_STATIC_ASSERT(__cpp_lib_char8_t >= 201811L, "libc++ should implement this");</div><div><br></div><div>to make sure that libc++ in particular behaves as expected, or</div><div><br></div><div>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><br></div><div>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"></a><br></div></div></div></blockquote><div><br></div><div>I'd rather avoid that; long-term test failures are not in anyones best-interest.</div><div>It leads to people ignoring the test results "oh, yeah we always have test failures"<br></div></div></div></blockquote><div><br></div><div>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></div></blockquote><div><br></div><div>Yes, although we have other examples where we're more lenient. For example, we have plenty of machinery to make sure that the tests pass when you use an old libc++ dylib shipped with e.g. macosx10.7, even though it does not implement the full standard.</div><div><br></div><div>Feature test macros bring back the question of what to do for incomplete implementations to an extreme level of granularity. I'd much rather keep the libc++ test suite as close as possible to a conformance test suite, meaning it should fail if a standard library does not satisfy everything. We can then add some implementation-specific XFAILs to say "oh, we're aware of this on that implementation". My opinion is that all of this "being lenient to incomplete implementations" should be implemented in LIT, not in the test suite itself.</div></div></div></blockquote><div><br></div><div>I think maybe something between my option 1 and option 2 above might work to split the difference. Divide the tests into two categories:</div><div><br></div><div> a) Functionality tests, conditioned on feature-test macros that indicate support for the tested feature</div><div> b) Feature-test macro tests that ensure the macros are at the current highest version</div><div><br></div><div>That way, a failure in the tests in category (a) indicate a serious bug: you're advertising support for a feature that you don't have. But a failure in the tests in category (b) indicate that your implementation is incomplete (and if no category (a) test fails, it's correctly reporting its incompleteness to the user). Then you can XFAIL the category (b) tests for the implementations that don't yet support whichever features.</div></div></div>