[PATCH] D40291: [Support] Add WritableMemoryBuffer class

[Rafael Avila de Espindola via llvm-commits]

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