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

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/__config:219-221
+#    if _LIBCPP_ENABLE_ASSERTIONS
+#      define _LIBCPP_ENABLE_HARDENED_MODE 1
+#    endif
----------------
ldionne wrote:
> 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.
Added `libcxx/test/libcxx/assertions/modes/enabling_assertions_enables_hardened_mode.pass.cp`.


================
Comment at: libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator.pass.cpp:9
 
 // REQUIRES: stdlib=libc++
 
----------------
Question unrelated to this patch: is this necessary? (Since the test is under `test/libcxx`)


================
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
----------------
ldionne wrote:
> 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.
Thanks!


================
Comment at: libcxx/test/libcxx/assertions/assertions_disabled.pass.cpp:1
-//===----------------------------------------------------------------------===//
-//
----------------
ldionne wrote:
> 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?
This should be covered by `test/libcxx/assertions/modes/debug_mode_disabled_in_tu.pass.cpp` and `test/libcxx/assertions/modes/hardened_mode_disabled_in_tu.pass.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154997