[PATCH] D34328: [ELF] - Add ability for DWARFContextInMemory to exit early when any error happen.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

I'd probably favor the API change of returning Expected<unique_ptr<DWARFContext>> - potentially with a backwards-compatible API that maintains the old API (& calls the new API, reports any Error and silently returns the DWARFContext). Though as you say, some callers want to continue on error - so this new API wouldn't express that.

Probably what might work better is if a callback for error handling was provided (a default one would be to maintain the existing behavior: print and continue. But other callers could say "print and stop". (eg: instead of "AllowsError", have llvm::function_ref<bool(Error E)> HandleError then do 'if (HandleError(std::move(E))) { return; }" or the like)

Why is the speed of lld important when the input is broken anyway? Printing all the errors may make it easier for a user to address them all without going through several slow link steps to get more errors (the same reason the compiler doesn't stop after the first error)? (eg: one corrupted compressed section, then one non-corrupt compressed section)

Also important to include test coverage - potentially in a unit test (though testing the IO would be tricky - but you could test if, given an input that had one broken, followed by a non-broken thing, then with the "stop on error" case returns no things and the "continue on error" returns the one non-broken thing.


https://reviews.llvm.org/D34328