[PATCH] D40291: [Support] Add WritableMemoryBuffer class

[Zachary Turner via llvm-commits]

Since there is no such thing as MutableStringRef, I think a better
comparison is ArrayRef / MutableArrayRef.  But close enough :)

I actually tried this route in my original implementation, but abandoned it
because WritableMemoryBuffer needs an entirely different interface for
creating one.  A lot of the existing static factory functions in
MemoryBuffer either don't make sense, or need a different set of arguments
to make sense in the writable case.

It seems easier to just start over with a fresh class and re-implement the
methods we need, customizing the interface appropriately.

On Wed, Nov 22, 2017 at 11:12 AM Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Pavel Labath <labath at google.com> writes:
>
> >>> +/// This interface is similar to MemoryBuffer, but allows writing to
> the
> >>> +/// underlying contents.  It only supports creation methods that are
> guaranteed
> >>> +/// to produce a writable buffer.  For example, mapping a file
> read-only is not
> >>> +/// supported.
> >>> +class WritableMemoryBuffer {
> >>> +  std::unique_ptr<MemoryBuffer> Buffer;
> >>> +
> >>
> >> This part of the patch is a bit odd, but I can't immediately think of
> >> anything better that keeps MemoryBuffer and WritableMemoryBuffer as
> >> distinct types.
> >>
> >> We could make MemoryBuffer non virtual with a PIMPL. That would make
> >> MemoryBuffer and WritableMemoryBuffer symmetrical simple wrappers, but
> it
> >> is not clear if it is worth it.
> >
> > Yeah, it's a bit strange.. The pimpl would introduce an extra layer of
> > indirection to MemoryBuffer, which does not sound like a good idea, as
> > we probably care more about it's performance than of this new class.
> >
> > IMHO, the cleanest solution here would be to actually not make these
> > distinct types, and use *const* MemoryBuffer to denote an immutable
> > piece of memory. I did not go with that because this is a "central
> > piece of infrastructure" :) and it would require fixing up all current
> > uses of MemoryBuffer (well.. almost all. I did find a couple of places
> > where we actually are const_casting the buffer contents and writing to
> > it).
>
> True, that is the same issue as StringRef X MutableStringRef.
>
> David Blaikie maybe?
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171122/d011c504/attachment.html>