<div dir="ltr">*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.<br><br>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?)</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Mar 19, 2018 at 3:10 PM Rafael Avila de Espindola <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">David Blaikie via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> writes:<br>
<br>
> On Mon, Mar 19, 2018 at 2:52 PM Rafael Avila de Espindola via Phabricator <<br>
> <a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br>
><br>
>> espindola added a comment.<br>
>><br>
>> One thing that I am probably missing is why we need this much.<br>
>><br>
>> On the lld side we only read this section when there is an error, and if<br>
>> reading it fails we just want to print the extra info to the user.<br>
>><br>
>> My guess is that something like lldb would be similar, read the line table<br>
>> and print a message if it failed.<br>
>><br>
>> For both cases a simple Expected<> should be sufficient, no?<br>
>><br>
><br>
> Mostly probably for things like llvm-dwarfdump that are likely to want to<br>
> provide more info to the user than to halt at the first error - to aid in<br>
> debugging/investigation.<br>
<br>
OK, dwarfdump is probably the best example as it wants to print as much<br>
as possible.<br>
<br>
But even for dwarfdump it seems that this could be "just":<br>
<br>
Expected<const DWARFDebugLine::LineTable *> getOrParseLineTable(<br>
    DWARFDataExtractor &DebugLineData, uint32_t Offset, const DWARFContext &Ctx,<br>
    const DWARFUnit *U,<br>
    std::function<Error(StringRef)> RecoverableIssueCallback);<br>
<br>
The idea is that every time the parser gets in a situation where it finds<br>
something it doesn't understand but can recover from (new version of a<br>
field, but we can skip over it for example) it can do something like<br>
<br>
if (Error E = RecoverableIssueCallback("unknown version of foobar"))<br>
   return E;<br>
<br>
The default callback, which would, for exmaple, be used by lld, could<br>
just always return an error.<br>
<br>
The callback used by dwarfdump would just log the issure and return<br>
success.<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div>