[libcxx-commits] [PATCH] D146214: [ASan][libc++] Annotating std::basic_string with all allocators

[Tacet via Phabricator via libcxx-commits]

AdvenamTacet added inline comments.


================
Comment at: libcxx/include/string:1845
         const void* __end = data() + capacity() + 1;
+        #  if _LIBCPP_CLANG_VER < 1600
         if (!__libcpp_is_constant_evaluated() && __begin != nullptr && is_same<allocator_type, __default_allocator_type>::value)
----------------
EricWF wrote:
> conditional compilation blocks are the source of a lot of bugs.
> 
> Instead of an `#if` and `#else` block here. Why not make it so that `__asan_annonate_container_with_allocator` only returns true when the other conditions `_LIBCPP_CLANG_VER > 1600` etc.
> 
> Then you could simply write the condition
> ```
> if (!__libcpp_is_constant_evaluated() && __begin != nullptr &&  __asan_annotate_container_with_allocator<allocator_type, __default_allocator_type>)
> ```
> 
> Where `__asan_annotate_container_allocator_type` can return true in the case that the allocator is the default.
> 
> 
Moving those checks into `__asan_annonate_container_with_allocator` is a nice idea. I will try to do it Tomorrow. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146214