<div dir="ltr">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:<div><br></div><div>struct MemoryBuffer {</div><div>   static MemoryBuffer createNonWritableBuffer1();</div><div>   static MemoryBuffer createNonWritableBuffer2();</div><div>};</div><div><br></div><div>struct WritableMemoryBuffer : public MemoryBuffer {</div><div>  static WritableMemoryBuffer createWritableBuffer1();</div><div>  static WritableMemoryBuffer createWritableBuffer2();<br></div><div>};</div><div><br></div><div>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.</div><div><br></div><div>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.</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Dec 14, 2017 at 1:28 PM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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?<br><br>& 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.</div><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Dec 14, 2017 at 1:22 PM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.<div><br></div><div>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.</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Dec 14, 2017 at 1:16 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Looking at it - MutableArrayRef just derives from ArrayRef.<br><br>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?</div><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Dec 4, 2017 at 2:32 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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)<div><br>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?<br><br>- Dave</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Dec 4, 2017 at 9:00 AM Pavel Labath via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">labath added a comment.<br>
<br>
Hi David,<br>
<br>
could you look at this by any chance?<br>
<br>
thanks.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D40291" rel="noreferrer" target="_blank">https://reviews.llvm.org/D40291</a><br>
<br>
<br>
<br>
</blockquote></div></blockquote></div></div></blockquote></div>
</blockquote></div></div></blockquote></div>