[llvm-commits] [llvm] r148463 - /llvm/trunk/include/llvm/ADT/ArrayRef.h
dblaikie at gmail.com
Thu Jan 19 17:41:48 PST 2012
On Thu, Jan 19, 2012 at 5:33 PM, Chris Lattner <clattner at apple.com> wrote:
> On Jan 18, 2012, at 11:01 PM, David Blaikie wrote:
>> On Wed, Jan 18, 2012 at 10:34 PM, Chris Lattner <sabre at nondot.org> wrote:
>>> URL: http://llvm.org/viewvc/llvm-project?rev=148463&view=rev
>>> Introduce a new MutableArrayRef class, which refers to a series of mutable
>>> T's that are consequtively in memory.
>> Just forwarding a brief discussion I had with Chris on IRC for the
>> email record - I'm pretty sure this could be implemented by switching
>> things around & migrating existing ArrayRef<T> code to ArrayRef<const
>> T> (& making his MutableArrayRef be the basic/generic ArrayRef
>> template and partially specializing ArrayRef<const T> using the old
>> ArrayRef implementation). Just thought I'd mention this in case anyone
>> else had a similar thought.
> Hi David,
> After having thought about this a bit, I'm not sure this is a good thing to do.
> By *far* the common case is than non-mutable array-ref version: punishing all clients to have to plop in a const seems really unfortunate, and your diff shows some massive code thrash that this causes. I think that forcing the unusual case to spell out MutableArrayRef is actually a good thing, because it makes things very explicit.
I'm not sure how much that's worth the inconsistency with the rest of
the language (where const is not the default, such as "T*" for
example). Though I realize it'll require a little habit forming as
peolpe get used to the explicit const - but mostly the use case will
guide them by error (if they have const data they'll fail if they try
to make an ArrayRef<T> over it).
Though I realize I'm perhaps a bit of a purist on a great many
matters. (& one of my earliest patches was implementing the const
removal for Type which is rather inconsistent with my own practices
but I understand where you're coming from & implemented it anyway - so
perhaps I don't have much to support this position now)
More information about the llvm-commits