[PATCH] D55308: Implement the second part of P0482 (char8_t)

[Marshall Clow via Phabricator]

mclow.lists added a comment.

In D55308#1321975 <https://reviews.llvm.org/D55308#1321975>, @ldionne wrote:

> In D55308#1320679 <https://reviews.llvm.org/D55308#1320679>, @mclow.lists wrote:
>
> > Agreed.
> >  Though I like the formulation that I posted to livcxx-dev:
> >
> >   #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
> >
> >
> > This fails silently if the macro is not defined - for libraries other than libc++.
> >  If other library maintainers want to add (say) a libstdc++-specific assertion, then there's a place for it.
>
>
> I'd err on the side of what Richard said and always assert, but otherwise I'm fine with something like this.


I would not; consider the test `algorithm.version.pass.cpp`, which should check for six different flags. Do you want that test to fail until libc++ implements all six of them? I do not. I want the tests to check for all the flags that libc++ has defined, and make sure that they are (a) defined, and (b) have a sane value. Things that we haven't implemented yet should not cause test failures.

> But it would be nice to extract this check into a separate utility header to avoid duplication.

I don't think that's a good idea; that header would quickly grow into all the tests for all the feature flags.
But I'm willing to discuss that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55308/new/

https://reviews.llvm.org/D55308