[PATCH] D78050: Introduce `__sanitizer::ArrayRef<T>` type.

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 14 10:11:08 PDT 2020


dblaikie added a comment.

In D78050#1979667 <https://reviews.llvm.org/D78050#1979667>, @delcypher wrote:

> In D78050#1979455 <https://reviews.llvm.org/D78050#1979455>, @dblaikie wrote:
>
> > While I'm in favor of this type looking more like llvm's MutableArrayRef (ie: allowing mutability or immutability to be encoded in the type parameter (const T versus non-const T)) rather than needing the two types LLVM has (ArrayRef + MutableArrayRef). I wonder if having this type called "ArrayRef" when it is subtly different from llvm's ArrayRef might be confusing?
>
>
> Thank you for raising this. I probably should have said "loosely inspired" by ArrayRef because I wasn't aware of MutableArrayRef and I hadn't noticed that LLVM ADT's ArrayRef was immutable. Given that LLVM's ADT ArrayRef is immutable my choice of naming would certainly be confusing. For my use case I actually only need the underlying data to be mutable so I don't need an immutable version.
>
> > Wonder if it needs another/better name - though the standard doesn't offer us a great alternative, it seems the standardized equivalent is std::span, which might be sufficiently different not to be a good name to use either? Not sure.
>
> My early draft name was actually `BufferRef<T>`. Does that seem any better?


Might be best just to resort to calling it MutableArrayRef to match LLVM's since they're so similar? Maybe one day we can revisit the design decision in LLVM and fix both, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78050





More information about the llvm-commits mailing list