Errors that occur when reading an MRI script would be clearer if they included a corresponding line number.
rupprecht MaskRay grimar jhenderson
- rG04398c729b20: [llvm-ar] Include a line number when failing to parse an MRI script
rGaa03c14827fa: Revert [llvm-ar] Include a line number when failing to parse an MRI script
rL372374: Reapply [llvm-ar] Include a line number when failing to parse an MRI script
rG25040f8dec2e: Reapply [llvm-ar] Include a line number when failing to parse an MRI script
rL372311: Revert [llvm-ar] Include a line number when failing to parse an MRI script
rL372309: [llvm-ar] Include a line number when failing to parse an MRI script
A bit inconvenience for you (sorry!): after D67558, the error messages are no longer capitalized, so this needs a rebase.
The only suggestion I have is to pass in an error handler from the calling code that can be used to generate an Error instance with a message customised to the caller. Default would be to do nothing special of course, but it would allow the MRI parser to pass one that refers back to a handler that appends the line number. That's potentially quite a significant refactor, so might be worth delaying to a subsequent change, but it might also be worth being done before this, so that we don't have to introduce the if statement in fail. What do people think?
Nit: white space -> whitespace
I have no strong preference for # versus ##, as long as we use one in tests which don't necessarily need them, as it makes the comments stand-out from the rest of the test (same reason for using ## in tests that need # for RUN lines etc). If someone else has a preference to use ## everywhere, that's fine with me.
I am not good familar with this tool's code yet, but at first look introducing a custom handler looks reasonable (that would help to remove most of these new variables it seems).
Is there any reason to check Saved? Can it be just like the following?
ParsingMRIScript = false; // Nothing to do if not saved. if (Saved) performOperation(ReplaceOrInsert, &NewMembers);