[PATCH] D55308: Implement the second part of P0482 (char8_t)
Louis Dionne via Phabricator
reviews at reviews.llvm.org
Mon Dec 10 11:04:26 PST 2018
ldionne added inline comments.
================
Comment at: include/string:4173
+#if _LIBCPP_STD_VER > 17 && defined(__cpp_char8_t)
+typedef basic_string<char8_t> u8string;
----------------
mclow.lists wrote:
> ldionne wrote:
> > I hate feature test macros already. This should just be `#if _LIBCPP_STD_VER > 17`... Can't we take the slightly more aggressive stance of "if you're trying to use libc++ as a c++20 library but your compiler does not support c++20, you're out of luck"?
> > I hate feature test macros already.
> Agreed.
>
> > Can't we take the slightly more aggressive stance of "if you're trying to use libc++ as a c++20 library but your compiler does not support c++20, you're out of luck"
>
> We can't do that, because (for example) LLVM 7.0 claims to support C++20, but doesn't support `char8_t`.
>
> In general, I agree with you, but this feature (`char8_t`) requires a compiler feature, or it doesn't work.
> We can't do that, because (for example) LLVM 7.0 claims to support C++20, but doesn't support char8_t.
Isn't that claim wrong, then? Also, surely you can't claim you implement C++20 at this point since it hasn't been voted?
I'm not trying to be rhetorical -- I'd _really_ like to avoid having to guard everything with a feature test macro because that will make the library much more complex and I'm sure it will introduce some tricky bugs. Just supporting many compilers and many C++ Standard versions is already difficult -- supporting partial implementations of a standard scares me a lot.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55308/new/
https://reviews.llvm.org/D55308
More information about the libcxx-commits
mailing list