[llvm-commits] [llvm] r149918 - in /llvm/trunk: include/llvm/Bitcode/ include/llvm/MC/ include/llvm/Support/ lib/Bitcode/Reader/ lib/Bitcode/Writer/ lib/MC/MCDisassembler/ lib/Support/ lib/Target/ARM/Disassembler/ lib/Target/MBlaze/Disassembler/

[Derek Schuff]

On Tue, Feb 7, 2012 at 2:33 PM, Jim Grosbach <grosbach at apple.com> wrote:

>
> On Feb 7, 2012, at 2:26 PM, Derek Schuff <dschuff at google.com> wrote:
>
> The problem is that fetching more data can cause the underlying structure
> (e.g. the storage pointer in the example) to be reallocated/resized.
>
>
> That's an implementation detail. It's the exposed interface of the class
> and the semantics associated that I'm concerned with. As a user of the
> class, I should neither know nor care what's inside or how it works. In
> this case, the semantics we want to be able to express are whether the
> consumer of a MemoryObject can change the underlying data (write to the
> memory/file/shared object) or not.
>
> MutableArrayRef type classes are basically just glorified pointers and
> designed not to own the data; but the data has to be owned by something
> accessible to MemoryObject, and MemoryObject::readBytes would have to call
> a non-const method on whatever does own it. The behavior you're describing
> is like a const pointer (const char* foo) but the 'const' qualifier on a
> method basically means 'const char* const foo'
>
>
> It means that the 'this' pointer is const qualified. Any additional
> semantics of the interface are convention up to the designer.
>

OK. I guess we just had different expectations of what those semantics were.


>
> We could of course keep const on the methods and do our game-playing by
> making all the data members of the StreamableMemoryObject mutable, but
> that's worse (IMO) than just having another class that's like MemoryObject
> but doesn't derive directly from it.
>
>
> Why?
>
>
if all of the data members are mutable, then the 'const' on the methods
effectively has no meaning at all. or it's a lie.

More specifically to this case, I think the usage of StreamableMemoryObject
is different enough from MemoryObject that it shouldn't really be derived,
and these other issues are even more reason.
So, I've attached a patch that fixes all the MC stuff and leaves
MemoryObject as it was, and leaves StreamableMemoryObject as the base
class. Please review,

thanks,
-Derek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120207/a9cec8dd/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: MCFix.patch
Type: text/x-patch
Size: 13106 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120207/a9cec8dd/attachment.bin>