This is an archive of the discontinued LLVM Phabricator instance.

[MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified
ClosedPublic

Authored by mstorsjo on Jul 8 2018, 12:36 PM.

Details

Summary

In this setup, skip adding all the default windows import libraries, if linking to windowsapp (which replaces them, when targeting the windows store/UWP api subset).

With GCC, the same is achieved by using a custom spec file, but since clang doesn't use spec files, we have to allow other means of overriding what default libraries to use (without going all the way to using -nostdlib, which would exclude everything). The same approach, in detecting certain user specified libraries and omitting others from the defaults, was already used in SVN r314138.

Diff Detail

Event Timeline

mstorsjo created this revision.Jul 8 2018, 12:36 PM
smeenai accepted this revision.Jul 9 2018, 2:23 PM

LGTM, particularly given r314138.

There are other umbrella libraries as well, e.g. OneCore and OneCoreUAP. Do you care about those as well or just WindowsApp?

lib/Driver/ToolChains/MinGW.cpp
207

I don't think it matters very much, but you could break out here.

232

Might be worth adding a short note to this comment about why we skip adding these libraries in the WindowsApp case?

This revision is now accepted and ready to land.Jul 9 2018, 2:23 PM
smeenai added inline comments.Jul 9 2018, 2:24 PM
test/Driver/mingw-windowsapp.c
5–6

Why do we end up with -lmingw32 twice, and why not just check the full line, like you're doing for the default case?

LGTM, particularly given r314138.

There are other umbrella libraries as well, e.g. OneCore and OneCoreUAP. Do you care about those as well or just WindowsApp?

I guess we would care, but there's nothing set up within mingw-w64 for those yet.

lib/Driver/ToolChains/MinGW.cpp
207

Ok, can do.

232

Sure, I can add a comment.

test/Driver/mingw-windowsapp.c
5–6

I don't remember exactly why -lmingw32 ends up multiple times; I think it comes from legacy compat with binutils ld, where the library ordering matters more (a later static library doesn't trigger search in an earlier one).

I'm checking twice, since there are other unrelated entries between these that I didn't want to spell out, to avoid making the test overly specific (to avoid having to update the test in case some of those are changed).

smeenai added inline comments.Jul 9 2018, 2:46 PM
test/Driver/mingw-windowsapp.c
5–6

Makes sense; I didn't realize before that all the other libraries in the default case were sandwiched between the -lmsvcrt and the -lmingw32 in the CHECK_WINDOWSAPP-SAME case.

A -NOT check would perhaps have been more obvious, but I think that might end up interacting poorly with the other checks, so I'm fine with this as is.

This revision was automatically updated to reflect the committed changes.