[libcxx-commits] [PATCH] D132092: [2a/3][ASan][libcxx] std::deque annotations

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik accepted this revision.
philnik added a comment.
This revision is now accepted and ready to land.

LGTM % nits.



================
Comment at: libcxx/include/deque:1044-1046
+// This macro should be defined only during testing ASAN.
+// Include the "asan_testing.h" file which defines it
+// to be able to use it in ASan tests.
----------------
I think this comment is wrong now?


================
Comment at: libcxx/include/deque:902
+        // NOTE: if __end % __block_size == 0, __annotations_end_mp points at the next block, which may not exist
+        typename __map::const_iterator __annotations_beg_mp = __map_.begin() + __beg / __block_size;
+        typename __map::const_iterator __annotations_end_mp = __map_.begin() + __end / __block_size;
----------------
AdvenamTacet wrote:
> philnik wrote:
> > Maybe use `__map_const_pointer`?
> I did try it at the beginning and there are type related issues. It's much cleaner that way than working around it.
Let's adda new alias `__map_const_iterator` then. The `typename __map::const_iterator` is really irritating.


================
Comment at: libcxx/include/deque:903
+        typename __map::const_iterator __annotations_beg_mp = __map_.begin() + __beg / __block_size;
+        typename __map::const_iterator __annotations_end_mp = __map_.begin() + __end / __block_size;
+
----------------
AdvenamTacet wrote:
> philnik wrote:
> > What does the `mp` stand for?
> I believe `map_pointer`, but I used this name based on the rest of the code where it's used.
Let's drop it if nobody knows what it actually stands for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132092