[llvm] r211595 - Pass a unique_ptr<MemoryBuffer> to the constructors in the Binary hierarchy.

[Rafael Espíndola]

> Hi Rafael,
>
> It doesn't make much sense to me that these objects own the buffer. After
> all they only read from the buffer, and there's no reason why it shouldn't
> be shared.

In a way that is true, but then they should *never* delete the buffer.
The old situation where they would sometimes own it was very error
prone.

> As an implementation detail, it's OK if they delete the buffer for now
> (r211542 is fantastic) but I'm not at all keen on expressing that in the
> interface because it ends up formalizing what's really a flaw in the
> implementation.

Well, we should have an interface that makes the ownership clear. If
it possible to make them never own the buffer, that would be awesome.
Once that is the case the constructors should probably be changed to
even take a StringRef and a name instead of MemoryBuffer, but if they
own what they point to then a unique_ptr is the correct interface
IMHO.

Would passing a pair of StringRefs (Data and Name) work for you?

> I have a changeset that makes these objects use the MemoryBuffer without
> deleting it, but r211595 and r211546 get in the way of the approach by
> making the interface require unique ownership.
>
> (I was expecting the underlying changes to land well before now which is why
> I didn't speak up earlier, could you take a look at
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140630/224492.html
> ?)

I consider reference counting to be the lest desirable option of all.
It makes it incredibly hard to reason about the ownership, even more
so than the old "owned" bool.

Cheers,
Rafael