[PATCH] MIR Serialization: Connect the machine function analysis pass to MIR parser.

[Justin Bogner]

Alex L <arphaman at gmail.com> writes:
> 2015-06-11 15:57 GMT-07:00 Justin Bogner <mail at justinbogner.com>:
>> Alex Lorenz <arphaman at gmail.com> writes:
>>> I've updated the patch based on Justin's comments:
>>>
>>> - renamed MIRMachineFunctionInitializer to MachineFunctionInitializer
>>> - simplified the MIRParser library interface - now the functions only
>>>   create the parser, they don't parse the LLVM module. The LLVM module
>>>   can be parsed by calling the appropriate method from the MIRParser
>>>   class.
>>
>> Thanks.
>>
>>> - reworked the error handling to use LLVM's diagnostic info.
>>
>> The particular mix of SMDiagnostic and DiagnosticInfo we've ended up
>> with here is kind of confusing and awkward. We should probably take the
>> SMDiagnostic out of the MachineFunctionInitializer::parse API entirely
>> and use LLVMContext::diagnose consistently:
>
> That's a good idea, but I still would like keep the SMDiagnostic inside of
> DiagnosticInfo (I explain why below).
 ...
>    
>     We can probably do with just storing "std::string Message" here
>     for now, instead of a reference to an SMDiagnostic. Maybe also the
>     .mir filename if that's appropriate. We can add more structure
>     later if we need it.
>
> SMDiagnostics print colors, and they a nice and consistent look when
> printed, which shows the line and column number before the 'error: '
> indicator. They also print the pointer which shows you where the error
> is and can print the appropriate source ranges as well. This is really
> important for me, as a lot of the errors that will come later rely on
> all of those features.
>
> I can't just print out SMDiagnostics to a string and store the string
> in the DiagnosticInfo - the color information isn't preserved, and the
> default printer for the diagnostic infos prints the 'error: '
> indicator before the message which would break the consistency if my
> message would contain the line and column numbers.
>
> This is the reason why it's important for me to store the SMDiagnostic
> and to print it out from the custom diagnostic info handler in llc
> directly to errs().

Sure, this makes sense.