[libcxx-commits] [PATCH] D154997: [libc++][hardening] Deprecate `_LIBCPP_ENABLE_ASSERTIONS`.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

Nice, thanks! This LGTM except for the `_LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK` test, which I have to investigate a bit more.



================
Comment at: libcxx/include/__config:219-221
+#    if _LIBCPP_ENABLE_ASSERTIONS
+#      define _LIBCPP_ENABLE_HARDENED_MODE 1
+#    endif
----------------
I think we should keep (or add) a test that you get the hardened mode when you define `_LIBCPP_ENABLE_ASSERTIONS`. If we mess that up, it could be very embarrassing in the upcoming release.


================
Comment at: libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator.pass.cpp:13
 // UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: !libcpp-has-legacy-debug-mode
 // XFAIL: availability-verbose_abort-missing
----------------
This test isn't related to the legacy debug mode, so we don't want to disable it. I'm not sure what's the right call here cause I missed the review where they added `-D_LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK`, but naively I'd say that we should remove `-D_LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK` and instead add `// UNSUPPORTED: !libcpp-has-hardened-mode && !libcpp-has-debug-mode`. I'll dig a bit more.

For now, to unblock you, I think you can just add

```
// UNSUPPORTED: !libcpp-has-hardened-mode && !libcpp-has-debug-mode
```

like you did everywhere around, and still keep `ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK` below. I'll follow up on D150264 and we can fix this test separately.


================
Comment at: libcxx/test/libcxx/assertions/assertions_disabled.pass.cpp:1
-//===----------------------------------------------------------------------===//
-//
----------------
Can you confirm that we have tests to ensure that `-D_LIBCPP_ENABLE_HARDENED_MODE=0` or `-D_LIBCPP_ENABLE_DEBUG_MODE=0` has a similar effect?


================
Comment at: libcxx/test/support/test.support/test_check_assertion.pass.cpp:10
 // REQUIRES: has-unix-headers
-// UNSUPPORTED: c++03
+// UNSUPPORTED: c++03, !libcpp-has-hardened-mode && !libcpp-has-debug-mode
 // XFAIL: availability-verbose_abort-missing
----------------
Maybe

```
// UNSUPPORTED: c++03
// UNSUPPORTED: !libcpp-has-hardened-mode && !libcpp-has-debug-mode
```

since you do it consistently everywhere else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154997