Page MenuHomePhabricator

Implement semantic action for SEARCH_DIR linker script command

Authored by davide on Jan 27 2015, 6:52 PM.

Diff Detail


Event Timeline

davide updated this revision to Diff 18802.Jan 27 2015, 6:52 PM
davide retitled this revision from to Implement semantic action for SEARCH_DIR linker script command.
davide updated this object.
davide edited the test plan for this revision. (Show Details)
davide added reviewers: ruiu, rafaelauler.
davide added a project: lld.
davide added subscribers: emaste, Unknown Object (MLST).
emaste added inline comments.Jan 28 2015, 6:47 AM
293–295 ↗(On Diff #18802)

To me this comment implies we recognize a subset of GROUP() and SEARCH_DIR().

Maybe Currently we only recognize this subset of linker script commands:

ruiu edited edge metadata.Jan 28 2015, 10:41 AM

I just committed r227341 to test the existing linker script functionality. Please add a test in the same way as my commit for this patch too.

276 ↗(On Diff #18802)

This function will never fail and it's too simple to be a separate function -- it's just one line. I'd write it in-line.

davide updated this revision to Diff 18937.Jan 28 2015, 7:31 PM
davide edited edge metadata.

Attempt to write an unit test and address Ed's and Rui's comments.

emaste added inline comments.Jan 28 2015, 7:35 PM
290 ↗(On Diff #18937)

phabricator often has trouble rendering whitespace properly, but please double-check tabs vs spaces here

ruiu accepted this revision.Jan 29 2015, 12:20 PM
ruiu edited edge metadata.


272 ↗(On Diff #18937)

Don't make a function virtual if not needed.

184 ↗(On Diff #18937)

I think there's code in the GNU driver to add /usr/lib to the default search path, so this test may pass even if the linker script parser doesn't work. I'd make it something unrealistic, like "/foo/bar", to avoid possible conflict.

This revision is now accepted and ready to land.Jan 29 2015, 12:20 PM
This revision was automatically updated to reflect the committed changes.