[llvm-commits] PATCH: refactoring MCInstFragment and MCDataFragment

[Eli Bendersky]

On Fri, Dec 7, 2012 at 9:45 AM, Jim Grosbach <grosbach at apple.com> wrote:
> This looks pretty reasonable to me.
>
> A small question, though. The default size of the SmallString in MCInstFragment increased from 8 to 32. Why is that? That's where your object size change came from.
>
> -Jim
>

Thanks for taking the time to review this, Jim. The answer to your
question, I believe, is at least partially in my message:

"While the increase is insignificant, it can be solved by having the
fragments hold their data in a SmallVector rather than SmallString.
This way the interface can use SmallVectorImpl to escape being tied to
size. Personally I think a SmallVector is more appropriate for such
uses anyway. This can be implemented in a follow-up patch if
requested."

Though I think I should probably elaborate as I may not have been
clear enough. To have a common interface returning contents,
getContents() returns SmallString<32>, so the instruction fragment
should also adhere to that. I'm aware of no way of returning a
size-independent SmallString, please correct me if I'm wrong. However,
I think the better way to approach would be to change the SmallString
to SmallVector, for which we can then use SmallVectorImpl to appease
the compiler with size-independent interfaces. The SmallString is used
just for convenience for the code, and I don't think switching to SV
would be a big loss there. If this sounds reasonable I can sent a
followup patch fairly soon that fixes this, and brings the size of the
instruction fragment down to original.

Eli