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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik requested changes to this revision.
philnik added a comment.
This revision now requires changes to proceed.

Thanks for working on this! I haven't done a thorough review, but here are a few style issues. FYI this patch will merge-conflict with D132081 <https://reviews.llvm.org/D132081>.



================
Comment at: libcxx/include/deque:1790
+// to be able to use it in ASan tests.
+#ifdef __LIBCPP_VERIFY_ASAN_DEQUE_ANNOTATIONS
+ public:
----------------
Every implementation-detail symbol should either be `_Uglified` or `__uglified`, but never `__Uglified`.


================
Comment at: libcxx/include/deque:1849
+ private:
+#endif
+
----------------
Please add an end comment to longer preprocessor blocks.


================
Comment at: libcxx/include/deque:1284
+private:
+    typedef allocator<_Tp>                      __default_allocator_type;
 public:
----------------
Please use `using` instead of `typedef` through out.


================
Comment at: libcxx/include/deque:1536
  private:
+    // Implementation based on the one from std::vector.
+    //
----------------
Please clang-format the new code.


================
Comment at: libcxx/include/deque:1543
+#ifndef _LIBCPP_HAS_NO_ASAN
+    _LIBCPP_INLINE_VISIBILITY
+
----------------
Please use `_LIBCPP_HIDE_FROM_ABI` instead.


================
Comment at: libcxx/include/deque:1558
+    }
+        void __annotate_contiguous_container_back(const void *__beg, const void *__end,
+                                               const void *__old_con_end,
----------------
You are missing a `_LIBCPP_HIDE_FROM_ABI` here.


================
Comment at: libcxx/include/deque:1597
+
+        const void *__p = _VSTD::__to_address(*__mp + (__b % __base::__block_size));
+
----------------
The same through out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132092