[PATCH] D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D49059#1156453, @smeenai wrote:

> 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.



================
Comment at: lib/Driver/ToolChains/MinGW.cpp:207
+    if (Lib == "windowsapp")
+      HasWindowsApp = true;
+
----------------
smeenai wrote:
> I don't think it matters very much, but you could break out here.
Ok, can do.


================
Comment at: lib/Driver/ToolChains/MinGW.cpp:232
+      if (!HasWindowsApp) {
+        // add system libraries
+        if (Args.hasArg(options::OPT_mwindows)) {
----------------
smeenai wrote:
> Might be worth adding a short note to this comment about why we skip adding these libraries in the WindowsApp case?
Sure, I can add a comment.


================
Comment at: test/Driver/mingw-windowsapp.c:5-6
+// CHECK_DEFAULT: "-lmsvcrt" "-ladvapi32" "-lshell32" "-luser32" "-lkernel32" "-lmingw32"
+// CHECK_WINDOWSAPP: "-lwindowsapp" "-lmingw32"
+// CHECK_WINDOWSAPP-SAME: "-lmsvcrt" "-lmingw32"
----------------
smeenai wrote:
> Why do we end up with -lmingw32 twice, and why not just check the full line, like you're doing for the default case?
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).


Repository:
  rC Clang

https://reviews.llvm.org/D49059