[PATCH] D54879: Introduce `LocalAddressSpaceView::LoadWritable(...)` and make the `Load(...)` method return a const pointer.

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_local_address_space_view.h:43
+  template <typename T>
+  static const T *Load(T *target_address, uptr num_elements = 1) {
+    // The target address space is the local address space so
----------------
delcypher wrote:
> vitalybuka wrote:
> > const T *target_addres?
> > and then you maybe can remove Writable from the another name
> @vitalybuka I considered this but this ends up putting the burden of giving `Load(...)` a const pointer on all callers.
> 
> This isn't good because it involves having to write a lot of `reinterpret_cast<const Something*>(...)` at the call sites. Given that getting a `const` pointer for enumeration of the allocator is the common case it makes more sense to force the caller to write the "longer" thing when we call site needs a non-const pointer (the uncommon case). So in that case the call-site uses `LoadWritable(...)` rather than `Load(...)`.  Does this make sense?
I am not sure what is going to be implementation of these.
Is there going to be any disadvantages of non-const vs const implementations?


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D54879