[libcxx-commits] [PATCH] D132090: [1a/3][ASan][compiler-rt] API for double ended containers

[Tacet via Phabricator via libcxx-commits]

AdvenamTacet added inline comments.


================
Comment at: compiler-rt/include/sanitizer/common_interface_defs.h:186-190
+void __sanitizer_annotate_de_contiguous_container_front(const void *beg,
+                                                        const void *end,
+                                                        const void *new_con_beg,
+                                                        const void *old_con_beg,
+                                                        const void *con_end);
----------------
AdvenamTacet wrote:
> vitalybuka wrote:
> > maybe just a single function?
> > 
> > void __sanitizer_annotate_de_contiguous_container(
> >                                                         const void *beg,
> >                                                         const void *end,
> >                                                         const void *old_con_beg,
> >                                                         const void *old_con_end,
> >                                                         const void *new_con_beg,
> >                                                         const void *new_con_end);
> That was my first idea, but after some thinking I decided against it.
> - I see almost zero situations when one may want to change the beginning and the end at the same time, so there is no point.
> - It may be very small, but it may have a bad impact on performance.
> - It's less important, but it would be harder to remember the correct order and some mistakes may not be noticed instantly.
> ```
> // Intended:
> __sanitizer_annotate_de_contiguous_container(0x8, 0x1000, 0x10, 0x11, 0x99, 0x100);
> // Mistake:
> __sanitizer_annotate_de_contiguous_container(0x8, 0x1000, 0x10, 0x99, 0x11, 0x100);
> ```
> I feel that it just complicates usage of those functions and brings no benefits, it's easier to just see `_front` and `_back` instead of checking what two arguments are the same.
> One less function in API seems nice, but I suggest two.
> 
> Do you know any examples when changing the beginning and the end at the same time may be helpful?
> Do you see any benefits, which I don't see?
I implemented a single function, order of arguments is same as suggested by you.
At the moment, it does use functions `_front` and `_back` as auxiliary functions, but API is extended only by one function. Also, Error classes / printing functions are updated to support new design.

I hope it's ok and potential improvements may be done in future patches. The API is extended only by one function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132090