[llvm-commits] [llvm] r168587 - in /llvm/trunk: include/llvm/MC/MCInstBuilder.h lib/Target/ARM/ARMAsmPrinter.cpp lib/Target/PowerPC/PPCAsmPrinter.cpp lib/Target/X86/X86MCInstLower.cpp

[Eli Bendersky]

On Mon, Nov 26, 2012 at 6:17 AM, Eli Bendersky <eliben at google.com> wrote:
>>> I would really prefer to separate the "builder" from emission. This
>>> method doesn't save a lot of typing and doesn't logically belong in a
>>> builder class. One of the manifestations is inconsistency - it not
>>> always makes sense to emit the MCInst right after building, so in some
>>> cases it will be emitted with this method, and in others with just
>>> calling OutStreamer.EmitInstruction.
>>
>> I thought about that, but I was unsure how to represent in the least ugly way. The options I see are
>>
>> - Give the builders names, and then pass it to EmitInstruction, which brings back the problem of too many variables names TmpInst.
>> - OutStreamer.EmitInstruction(MCInstBuilder(...).addFoo().addBar());
>>
>> Any preferences?
>>
>
> IMHO it's better to be explicit - the builder should be able to expose
> the object it has built and let consumers figure out what they want to
> do with it.

Re-reading, I didn't really answer your question :-) I don't have a
preference between the two, as long as the emission is removed from
the builder's capabilities.
Eli