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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for working on this!



================
Comment at: libcxx/include/__config:206
+// - `string_view`;
+// - `array`.
+// #define _LIBCPP_ABI_BOUNDED_ITERATORS
----------------
Is it intended to add all containers in the future?


================
Comment at: libcxx/include/__config:223
+//
+//#define _LIBCPP_ENABLE_NEW_DEBUG_MODE 1
+
----------------
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.


================
Comment at: libcxx/include/__config:238
+
+// Checks any calls into a container that attempt to access a non-existent element. Types like `optional` and `function`
+// are considered one-element containers for the purposes of this check. This check only applies to member functions of
----------------



================
Comment at: libcxx/include/__config:244
+
+// TODO:
+// See docs/DesignDocs/UnspecifiedBehaviorRandomization.rst
----------------
Can you change them to `TODO HARDENING` or similar to make grepping them easier.


================
Comment at: libcxx/include/__config:253
+//
+// #define _LIBCPP_ASSERTIONS_ENABLE_CHECK_UNSORTED_ETC_INPUT 1
+
----------------
Thanks a lot for the documentation! It really helps to understand what the hardening mode will do.


================
Comment at: libcxx/include/__config:88
+/*
+#define _LIBCPP_ENABLE_ASSERTIONS 1
+#define _LIBCPP_ENABLE_HARDENED_MODE 1
----------------
var-const wrote:
> These are leftovers from testing, but I'm keeping them for now as useful reminders.
Would it make sense to try to land the `__config` split patch first?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151637