[PATCH] D40291: [Support] Add WritableMemoryBuffer class

[Zachary Turner via llvm-commits]

All of the methods for constructing a MemoryBuffer are static methods of
the MemoryBuffer base class.  And they all return MemoryBuffers.  It
doesn't seem like a good interface to have:

struct MemoryBuffer {
   static MemoryBuffer createNonWritableBuffer1();
   static MemoryBuffer createNonWritableBuffer2();
};

struct WritableMemoryBuffer : public MemoryBuffer {
  static WritableMemoryBuffer createWritableBuffer1();
  static WritableMemoryBuffer createWritableBuffer2();
};

You can use private inheritance so someone can't say
WritableMemoryBuffer::createNonWritableBuffer1() (since that operation
doesn't really make sense), but now you lose the primary advantages of
using inheritance in the first place, which is that you can cast to a base
class pointer.

There might be a case to be made for extracting the static factory
interface out of the MemoryBuffer class and into some global functions, but
that's a rather intrusive change.

On Thu, Dec 14, 2017 at 1:28 PM David Blaikie <dblaikie at gmail.com> wrote:

> Not sure I follow - a derived class wouldn't inherit/necessarily have any
> relationship to its base class in the ways it's constructed, only in the
> ways it's used, right?
>
> & if WritableMemoryBuffer is in any sense a MemoryBuffer (either by
> conversion or otherwise) it seems WritableMemoryBuffer does provide all the
> same operations (once constructed) as MemoryBuffer plus whatever additional
> mutation operations it provides.
>
>
> On Thu, Dec 14, 2017 at 1:22 PM Zachary Turner <zturner at google.com> wrote:
>
>> Creating a MutableMemoryBuffer requires totally different code than
>> creating a non-mutable memory buffer.  They can fail in different ways, for
>> example, and some ways of initializing a memory buffer only apply in one
>> case or the other.
>>
>> This works for ArrayRef and MutableArrayRef because the only difference
>> between the underlying type is its constness, but that doesn't work when
>> the underlying type is a class that has a different interface when it's
>> const vs non-const.
>>
>> On Thu, Dec 14, 2017 at 1:16 PM David Blaikie <dblaikie at gmail.com> wrote:
>>
>>> Looking at it - MutableArrayRef just derives from ArrayRef.
>>>
>>> I'm not sure why a similar design wouldn't be used here - with
>>> WritableMemoryBuffer deriving from MemoryBuffer. Is there a good summary of
>>> why the difference in design exists here?
>>>
>>>
>>> On Mon, Dec 4, 2017 at 2:32 PM David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>> I advocated for const-ifying ArrayRef when MutableArrayRef was added
>>>> (so const all the existing ArrayRef, and then have non-const ArrayRef for
>>>> when mutation is desired - avoiding the need to introduce an explicit
>>>> MutableArrayRef type) but Chris wasn't a fan of that, so we have ArrayRef +
>>>> MutableArrayRef (the argument being that most ArrayRefs are const, so
>>>> making the more common case more verbose was unfavorable)
>>>>
>>>> I believe in that case MutableArrayRef has an implicit conversion
>>>> operator to ArrayRef (or ArrayRef has a conversion from MutableArrayRef, I
>>>> forget which) & expect we could do the same thing here, to be consistent?
>>>>
>>>> - Dave
>>>>
>>>> On Mon, Dec 4, 2017 at 9:00 AM Pavel Labath via Phabricator <
>>>> reviews at reviews.llvm.org> wrote:
>>>>
>>>>> labath added a comment.
>>>>>
>>>>> Hi David,
>>>>>
>>>>> could you look at this by any chance?
>>>>>
>>>>> thanks.
>>>>>
>>>>>
>>>>> https://reviews.llvm.org/D40291
>>>>>
>>>>>
>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171214/85505955/attachment.html>