[PATCH] D67449: [llvm-ar] Include a line number when failing to parse an MRI script

Tue Sep 17 02:07:49 PDT 2019

grimar added a comment.

In D67449#1672179 <https://reviews.llvm.org/D67449#1672179>, @grimar wrote:

> In D67449#1671048 <https://reviews.llvm.org/D67449#1671048>, @jhenderson wrote:
>
> > In D67449#1670266 <https://reviews.llvm.org/D67449#1670266>, @MaskRay wrote:
> >
> > > I wish we can create a new function for MRI error reporting but as it stands, `runMRIScript` can call `readLibrary, addChildMember, and addMember` which can in turn call `fail` or `failIfError`, so `fail` has to be aware of the MRI line number. + at grimar @jhenderson in case they have suggestions.
> >
> >
> > 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?
>
>
> 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).

I am not sure though. The current approach is simple and I do not know/see for what else such handler can be usefull here. So I'd just go with this patch for now probably.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67449/new/

https://reviews.llvm.org/D67449