[llvm] r181466 - Pull up AssemblyWriter interface into header to allow subclassing

[Malea, Daniel]

Is it making the functions virtual that's worrisome, or are you
uncomfortable with exposing the AssemblyWriter interface at all? This
commit makes all the AssemblyWriter::print* functions virtual to keep the
interface consistent.

Really though, DebugIR's implementation only needs to "hook into"
printInstructionLine() and printInstruction(), in order to:
a) use the line counting logic that is now available in the
raw_formatted_ostream class to create new DebugLoc metadata nodes
b) print only the important parts of the LLVM IR that is sent to the
debugger while hiding stuff users don't care about (for example llvm.dbg.*
intrinsics that get 'skipped' in machine code, and debug metadata that
gets encoded in DWARF anyways). I found it to be a much nicer user
experience when these 'extras' are omitted from the debuggers "source
code" views.

That said, after thinking about the approach a little more, I think we can
accomplish b) by cloning the module and removing the un-interesting parts
of the IR rather than just skipping outputting them in an AssemblyWriter
subclass. After looking briefly at some other classes (like InstVisitor)
that avoid virtual calls with template magic, I think similar tricks can
be used in AssemblyWriter to accomplish a) without using virtual functions
by relying on static_casts and static overloads instead.

If this sounds like a reasonable way to address your concerns, I can put
together a patch that does what I have in mind. Given the similarities
between what DebugIR needs to do and and what AssemblyWriter does, I am
not sure how to avoid exposing the class in a header though, so I'm hoping
that part is acceptable :)



Thanks,
Dan




On 2013-05-20 11:08 AM, "Chris Lattner" <clattner at apple.com> wrote:

>
>On May 8, 2013, at 1:38 PM, Daniel Malea <daniel.malea at intel.com> wrote:
>
>> Author: dmalea
>> Date: Wed May  8 15:38:31 2013
>> New Revision: 181466
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=181466&view=rev
>> Log:
>> Pull up AssemblyWriter interface into header to allow subclassing
>> - made all functions virtual so that subclasses can specialize them
>> - add printInstructionLine so that subclasses can choose whether or not
>>to
>>  print the newline character (without having to implement
>>printBasicBlock()
>> - added a second constructor to AssemblyWriter that does not require a
>>  SlotTracker, as required in order to keep the SlotTracker helper class
>>outside
>>  AsmWriter.h and buried in the implementation.
>
>This is another *really* unfortunate change required by DebugIR.  Why is
>this functionality important, and why is this the right way to implement
>it?
>
>-Chris