Page MenuHomePhabricator

ELF: Do not use fatal in LinkerScript.cpp.
ClosedPublic

Authored by ruiu on Jan 27 2016, 6:44 PM.

Details

Reviewers
rafael
Summary

Propagating errors back to callers is annoying in recursive descendent
parser because they are naturally constructed as mutually recursive
functions. In this patch, I took a simple approach. The accessors of
the token stream behaves as if they are at EOF once we saw any errors.
It naturally makes all functions to return.

This is the final change to not use fatal in the linker.

Diff Detail

Event Timeline

ruiu updated this revision to Diff 46209.Jan 27 2016, 6:44 PM
ruiu retitled this revision from to ELF: Do not use fatal in LinkerScript.cpp..
ruiu updated this object.
ruiu added a reviewer: rafael.
ruiu added a subscriber: llvm-commits.

Totally a casual comment & I could be totally off: Parser error recovery
can be really helpful to users (eg: Clang's error recovery), though (much
like using TLS for the general error handling improvements, and my comments
there) I expect this is a good first step and better error recovery can be
added over time to improve the user experience here.

As you've said, it's certainly not a terribly easy thing to do great error
recovery, and I wouldn't suggest trying to solve everything up-front, for
sure.

rafael edited edge metadata.Feb 1 2016, 5:39 AM
rafael added a subscriber: rafael.

testcase?

ruiu updated this revision to Diff 46585.Feb 1 2016, 4:08 PM
ruiu edited edge metadata.
  • Added test cases and fixed a few bugs found by the tests.
rafael accepted this revision.Feb 2 2016, 11:59 AM
rafael edited edge metadata.

LGTM

test/ELF/invalid-linkerscript.test
6

Add a comment saying all the "no input files" are here just to show that the parser gives up, but the linker keeps going when an error is found.

This revision is now accepted and ready to land.Feb 2 2016, 11:59 AM
ruiu closed this revision.Feb 2 2016, 12:33 PM