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

[Dan Liew via Phabricator via llvm-commits]

delcypher marked an inline comment as done.
delcypher 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
----------------
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?


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D54879