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

Marshall Clow via Phabricator reviews at reviews.llvm.org
Wed Dec 5 14:13:40 PST 2018


mclow.lists added a comment.

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

> Two overarching comments.
>
> 1. Maybe we could/should encapsulate the logic for checking the feature test macro in a utility header for the tests. This way, we wouldn't have to copy/paste ``` #if TEST_STD_VER > 17 LIBCPP_ASSERT(IS_DEFINED(__cpp_lib_char8_t)); // libc++ implements this
> 2. if defined(__cpp_lib_char8_t)
> 3. if __cpp_lib_char8_t < 201811L
> 4. error "__cpp_lib_char8_t has an invalid value"
> 5. endif
> 6. endif #endif ``` so many times. Also, it would then be reasonable to have this instead: ``` #if TEST_STD_VER > 17
> 7. if defined(_LIBCPP_VERSION) && !defined(__cpp_lib_char8_t)
> 8. error "libc++ implements this"
> 9. endif
> 10. if defined(__cpp_lib_char8_t)
> 11. if __cpp_lib_char8_t < 201811L
> 12. error "__cpp_lib_char8_t has an invalid value"
> 13. endif
> 14. endif #endif ```
>
>   This would remove the need for the `IS_DEFINED` macro, which is a bit tacky.


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.


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

https://reviews.llvm.org/D55308





More information about the libcxx-commits mailing list