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

Louis Dionne via Phabricator reviews at reviews.llvm.org
Thu Dec 6 13:48:05 PST 2018


ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

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. But it would be nice to extract this check into a separate utility header to avoid duplication.



================
Comment at: test/std/language.support/support.limits/support.limits.general/limits.version.pass.cpp:18
+
+#include <string_view>
+#include <cassert>
----------------
ldionne wrote:
> `<limits>`
Still wrong?


================
Comment at: test/std/language.support/support.limits/support.limits.general/locale.version.pass.cpp:18
+
+#include <string_view>
+#include <cassert>
----------------
ldionne wrote:
> `<locale>`
Still wrong?


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

https://reviews.llvm.org/D55308





More information about the libcxx-commits mailing list