[libcxx-commits] [PATCH] D122574: [libcxx] [test] Use LIBCPP_ASSERT for libc++-specific checks

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik accepted this revision.
philnik added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: libcxx/test/std/re/re.const/re.synopt/syntax_option_type.pass.cpp:41-46
+#if defined _LIBCPP_ABI_REGEX_CONSTANTS_NONZERO || !defined _LIBCPP_VERSION
+    // https://llvm.org/PR35967
     assert(std::regex_constants::ECMAScript != 0);
 #else
     assert(std::regex_constants::ECMAScript == 0);
 #endif
----------------
jwakely wrote:
> philnik wrote:
> > jwakely wrote:
> > > An alternative to this would be for me to define `_LIBCPP_ABI_REGEX_CONSTANTS_NONZERO` in `test/support/test_macros.h` when libstdc++ is detected.
> > > 
> > > It seems slightly more correct to check this, rather than have non-libc++ implementations define a macro about the libc++ ABI variant, but I'll do it whichever way you like.
> > If this isn't portable this should be a `LIBCPP_ASSERT` IMO.
> How would that work? The standard guarantees that it's non-zero, and libstdc++ implements that. Libc++ makes it zero by default, or non-zero with a non-default ABI setting. So it's portable that the constant exists, but whether it's non-zero is not portable **even among different builds of libc++**.
Oh, sorry. I didn't realize that there is a standard behaviour and libc++ just gets it wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122574