[PATCH] D44560: [DWARF] Rework debug line parsing to use llvm::Error and callbacks

[David Blaikie via llvm-commits]

*nod* seems pretty reasonable to me - I haven't been looking closely at the
code so far but just took a bit of a look to try to understand the
difference between what's there and what you're suggesting, Rafael.

If I've got this right - the main thing that'd be simplified by your
suggested change is to remove the need for the additional complexity of a
custom Error type while enabling callers that don't care about
warn-and-continue to stop quickly? (by returning the Error from the handler
- which isn't provided by the currently proposed patch?)

On Mon, Mar 19, 2018 at 3:10 PM Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> David Blaikie via llvm-commits <llvm-commits at lists.llvm.org> writes:
>
> > On Mon, Mar 19, 2018 at 2:52 PM Rafael Avila de Espindola via
> Phabricator <
> > reviews at reviews.llvm.org> wrote:
> >
> >> espindola added a comment.
> >>
> >> One thing that I am probably missing is why we need this much.
> >>
> >> On the lld side we only read this section when there is an error, and if
> >> reading it fails we just want to print the extra info to the user.
> >>
> >> My guess is that something like lldb would be similar, read the line
> table
> >> and print a message if it failed.
> >>
> >> For both cases a simple Expected<> should be sufficient, no?
> >>
> >
> > Mostly probably for things like llvm-dwarfdump that are likely to want to
> > provide more info to the user than to halt at the first error - to aid in
> > debugging/investigation.
>
> OK, dwarfdump is probably the best example as it wants to print as much
> as possible.
>
> But even for dwarfdump it seems that this could be "just":
>
> Expected<const DWARFDebugLine::LineTable *> getOrParseLineTable(
>     DWARFDataExtractor &DebugLineData, uint32_t Offset, const DWARFContext
> &Ctx,
>     const DWARFUnit *U,
>     std::function<Error(StringRef)> RecoverableIssueCallback);
>
> The idea is that every time the parser gets in a situation where it finds
> something it doesn't understand but can recover from (new version of a
> field, but we can skip over it for example) it can do something like
>
> if (Error E = RecoverableIssueCallback("unknown version of foobar"))
>    return E;
>
> The default callback, which would, for exmaple, be used by lld, could
> just always return an error.
>
> The callback used by dwarfdump would just log the issure and return
> success.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180319/f5fef279/attachment.html>