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

[Rafael Avila de Espindola via Phabricator via llvm-commits]

espindola added a comment.

>> The callback would only be used for recoverable errors, though, yes? Allowing the user to differentiate the two cases ("recoverable things come through the callback, non-recoverable things are errors seen as a result that didn't pass through/come from the callback"). Seems fair to me.
> 
> I think you may have missed the difference between errors which allow parsing later tables and errors which prevent parsing any later tables. If the unrecoverable errors do not go through a callback, we need the extra information of whether the length field is valid or not available to the caller somehow. I suppose we could add an extra method to the LineTable class instead to query whether the length is valid or not instead of the custom error type, but there's a risk here that users will miss this function and still try to use the length field.

I don't think we missed it. The callback is used when there is any way that the parsing can continue. The callback should be passed information about the issue. What should not happen is for the code is the library to have the notion of an Error severity.

We have discussed two use cases. In dwarfdump we want to parse as much as we can and print whatever issues are found to stderr.

In lld we would be happy to stop as soon as any issue is found, but as you point out that is not required and we can also just print every issue to stderr.

Neither of these suggests the need for a severity. In fact, given the above it seems the callback can return void, take a StringRef and the parsing code produces an Error only when it cannot recover. What use case do you have in mind that needs more than this?

Cheers,
Rafael


Repository:
  rL LLVM

https://reviews.llvm.org/D44560