[libcxx-commits] [PATCH] D151637: DRAFT: hardening "interface"

Tue Jun 27 11:08:50 PDT 2023

var-const added a comment.

In D151637#4441919 <https://reviews.llvm.org/D151637#4441919>, @ldionne wrote:

> 1. Extract the ABI macro for bounded iterators out of this patch. This will make this patch smaller and also get that out of the way when you try to remove the legacy debug mode.

https://reviews.llvm.org/D153895

> 2. Introduce the assertions machinery in a separate patch: `_LIBCPP_ENABLE_HARDENED_MODE` and `_LIBCPP_ENABLE_NEW_DEBUG_MODE`, but without any of the check macros like `_LIBCPP_ASSERTIONS_ENABLE_CHECK_BAD_CONTAINER_ACCESS`. Yes, that patch is going to look kinda stupid, but that will allow us to properly consider the interaction between those settings and the existing settings we have.

https://reviews.llvm.org/D153902

> 3. Then, in a separate patch, let's introduce some (or all) of the `_LIBCPP_ASSERTIONS_ENABLE_CHECK_foo` checks and plumb them into `_LIBCPP_ENABLE_HARDENED_MODE`/`_LIBCPP_ENABLE_NEW_DEBUG_MODE` which will already have been introduced.

I'll reuse this patch once the other two patches have landed.

================
Comment at: libcxx/include/__config:223
+//
+//#define _LIBCPP_ENABLE_NEW_DEBUG_MODE 1
+
----------------
Mordante wrote:
> I really dislike names with _NEW_ in it, it tends to get stale at some point. Just like terms as "modern C++" which some people feel is C++11...
> 
> I would suggest `_LIBCPP_ENABLE_HARDENED_DEBUG_MODE` instead. Or would it make sense to do
> `//#define _LIBCPP_ENABLE_HARDENED_MODE 1` as is
> `//#define _LIBCPP_ENABLE_HARDENED_MODE 2` the debug mode. This automatically makes them mutually exclusive.
My hope is that I can land https://reviews.llvm.org/D153672 first which would allow me to reuse the "obvious" name `_LIBCPP_ENABLE_DEBUG_MODE`. Otherwise, I like `_LIBCPP_ENABLE_HARDENED_DEBUG_MODE` -- I wasn't very happy with the "new" name either, so thanks a lot for the suggestion!

Re. using `1` and `2` as levels -- I agree it automatically makes them mutually exclusive which is a good property to have. On the other hand, having named macros is perhaps a little more readable than using numbers to represent each mode -- it's not obvious without reading the documentation what exactly `_LIBCPP_ENABLE_HARDENED_MODE 2` means (and whether `2` is the max value or not).

Following @ldionne's feedback, I created a new patch that only adds the configuration macros, without any assertions, and used the name `_LIBCPP_ENABLE_HARDENED_DEBUG_MODE` there: https://reviews.llvm.org/D153902

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151637