[libcxx-commits] [PATCH] D153895: [libc++][hardening] Add an ABI macro `_LIBCPP_ABI_BOUNDED_ITERATORS`.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__config:197
+// Changes the iterator type of select containers (see below) to a bounded iterator that:
+// - keeps track of which container it refers to;
+// - keeps track of whether it's valid or not and asserts validity on every dereference;
----------------
This is not true right now, so it should be removed.


================
Comment at: libcxx/include/__config:198
+// - keeps track of which container it refers to;
+// - keeps track of whether it's valid or not and asserts validity on every dereference;
+// - allows enabling the check that a given range is valid (sentinel is reachable from the begin iterator, etc.) in
----------------



================
Comment at: libcxx/include/__config:199-200
+// - keeps track of whether it's valid or not and asserts validity on every dereference;
+// - allows enabling the check that a given range is valid (sentinel is reachable from the begin iterator, etc.) in
+//   constant time.
+//
----------------
This is also aspirational so it could be removed (for now).


================
Comment at: libcxx/include/__config:202-203
+//
+// ABI impact: increases the size and changes the layout of containers (to store additional information that is used by
+// bounded iterators), changes the return type of container functions that return iterators.
+//
----------------
This seems simpler but also more accurate.


================
Comment at: libcxx/include/__config_site.in:33
 #cmakedefine _LIBCPP_ENABLE_DEBUG_MODE
+#cmakedefine _LIBCPP_ABI_BOUNDED_ITERATORS
 
----------------
We generally don't do that for ABI macros (except e.g. `_LIBCPP_ABI_FORCE_ITANIUM` and `_LIBCPP_ABI_FORCE_MICROSOFT` which are kind of hacks and don't have the same purpose as other ABI macros). I don't think this is necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153895