[libcxx-commits] [PATCH] D122574: [libcxx] [test] Guard libc++-specific checks with _LIBCPP_VERSION

[Jonathan Wakely via Phabricator via libcxx-commits]

jwakely added inline comments.


================
Comment at: libcxx/test/std/re/re.badexp/regex_error.pass.cpp:116-119
+#ifdef _LIBCPP_VERSION
         assert(e.what() == std::string("There was insufficient memory to determine whether the regular "
                "expression could match the specified character sequence."));
+#endif
----------------
philnik wrote:
> You can use `LIBCPP_ASSERT` for all of these instead.
Ah yes, that would be better.


================
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
----------------
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++**.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122574