<div dir="ltr">Since there is no such thing as MutableStringRef, I think a better comparison is ArrayRef / MutableArrayRef.  But close enough :)<div><br></div><div>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.</div><div><br></div><div>It seems easier to just start over with a fresh class and re-implement the methods we need, customizing the interface appropriately.</div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Nov 22, 2017 at 11:12 AM Rafael Avila de Espindola <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Pavel Labath <<a href="mailto:labath@google.com" target="_blank">labath@google.com</a>> writes:<br>
<br>
>>> +/// This interface is similar to MemoryBuffer, but allows writing to the<br>
>>> +/// underlying contents.  It only supports creation methods that are guaranteed<br>
>>> +/// to produce a writable buffer.  For example, mapping a file read-only is not<br>
>>> +/// supported.<br>
>>> +class WritableMemoryBuffer {<br>
>>> +  std::unique_ptr<MemoryBuffer> Buffer;<br>
>>> +<br>
>><br>
>> This part of the patch is a bit odd, but I can't immediately think of<br>
>> anything better that keeps MemoryBuffer and WritableMemoryBuffer as<br>
>> distinct types.<br>
>><br>
>> We could make MemoryBuffer non virtual with a PIMPL. That would make<br>
>> MemoryBuffer and WritableMemoryBuffer symmetrical simple wrappers, but it<br>
>> is not clear if it is worth it.<br>
><br>
> Yeah, it's a bit strange.. The pimpl would introduce an extra layer of<br>
> indirection to MemoryBuffer, which does not sound like a good idea, as<br>
> we probably care more about it's performance than of this new class.<br>
><br>
> IMHO, the cleanest solution here would be to actually not make these<br>
> distinct types, and use *const* MemoryBuffer to denote an immutable<br>
> piece of memory. I did not go with that because this is a "central<br>
> piece of infrastructure" :) and it would require fixing up all current<br>
> uses of MemoryBuffer (well.. almost all. I did find a couple of places<br>
> where we actually are const_casting the buffer contents and writing to<br>
> it).<br>
<br>
True, that is the same issue as StringRef X MutableStringRef.<br>
<br>
David Blaikie maybe?<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div>