This is an archive of the discontinued LLVM Phabricator instance.

[LLD] [COFF] Add -exclude-all-symbols for MinGW
ClosedPublic

Authored by mstorsjo on Feb 19 2019, 1:10 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.Feb 19 2019, 1:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2019, 1:10 AM
ruiu added inline comments.Feb 19 2019, 10:49 AM
COFF/Driver.cpp
1613 ↗(On Diff #187323)

Shouldn't this be Config->MinGW && Config->DLL && ...? IIUC this is a mingw extension, and we don't want to run the code below if not mingw.

mstorsjo marked an inline comment as done.Feb 19 2019, 11:11 AM
mstorsjo added inline comments.
COFF/Driver.cpp
1613 ↗(On Diff #187323)

We could do that as well. As far as I can see, it would only make a difference for the case when passing -export-all-symbols but not -lldmingw. The former is also an LLD private option only used by the MinGW frontend, so it doesn't really matter either way, but I can make it that way (I agree that the compound condition might be less confusing that way).

ruiu added inline comments.Feb 19 2019, 11:19 AM
COFF/Driver.cpp
1613 ↗(On Diff #187323)

I think my preference is to make code that is expected to run only in MinGW mode to start with if (Config->MinGW) so that a reader can easily identify MinGW-specific features as such. Currently -export-all-symbols can be used without -lldmingw, but that's something no one cares, and I'd think for clarity -export-all-symbols` should be significant only when -lldmingw is also given.

mstorsjo updated this revision to Diff 187420.Feb 19 2019, 11:33 AM

Changed the condition as requested.

ruiu accepted this revision.Feb 19 2019, 12:50 PM

LGTM

This revision is now accepted and ready to land.Feb 19 2019, 12:50 PM
This revision was automatically updated to reflect the committed changes.