[PATCH] D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified
This revision was automatically updated to reflect the committed changes.
Closed by commit rC336655: [MinGW] Skip adding default win32 api libraries if
-lwindowsapp is specified (authored by mstorsjo, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D49059?vs=154524&id=154773#toc
Repository:
rC Clang
https://reviews.llvm.org/D49059
Files:
lib/Driver/ToolChains/MinGW.cpp
test/Driver/mingw-windowsapp.c
Index: test/Driver/mingw-windowsapp.c
===
--- test/Driver/mingw-windowsapp.c
+++ test/Driver/mingw-windowsapp.c
@@ -0,0 +1,6 @@
+// RUN: %clang -v -target i686-pc-windows-gnu -### %s 2>&1 | FileCheck
-check-prefix=CHECK_DEFAULT %s
+// RUN: %clang -v -target i686-pc-windows-gnu -### %s -lwindowsapp 2>&1 |
FileCheck -check-prefix=CHECK_WINDOWSAPP %s
+
+// CHECK_DEFAULT: "-lmsvcrt" "-ladvapi32" "-lshell32" "-luser32" "-lkernel32"
"-lmingw32"
+// CHECK_WINDOWSAPP: "-lwindowsapp" "-lmingw32"
+// CHECK_WINDOWSAPP-SAME: "-lmsvcrt" "-lmingw32"
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -201,6 +201,14 @@
CmdArgs.push_back("-Bdynamic");
}
+ bool HasWindowsApp = false;
+ for (auto Lib : Args.getAllArgValues(options::OPT_l)) {
+if (Lib == "windowsapp") {
+ HasWindowsApp = true;
+ break;
+}
+ }
+
if (!Args.hasArg(options::OPT_nostdlib)) {
if (!Args.hasArg(options::OPT_nodefaultlibs)) {
if (Args.hasArg(options::OPT_static))
@@ -223,15 +231,19 @@
if (Args.hasArg(options::OPT_pthread))
CmdArgs.push_back("-lpthread");
- // add system libraries
- if (Args.hasArg(options::OPT_mwindows)) {
-CmdArgs.push_back("-lgdi32");
-CmdArgs.push_back("-lcomdlg32");
+ if (!HasWindowsApp) {
+// Add system libraries. If linking to libwindowsapp.a, that import
+// library replaces all these and we shouldn't accidentally try to
+// link to the normal desktop mode dlls.
+if (Args.hasArg(options::OPT_mwindows)) {
+ CmdArgs.push_back("-lgdi32");
+ CmdArgs.push_back("-lcomdlg32");
+}
+CmdArgs.push_back("-ladvapi32");
+CmdArgs.push_back("-lshell32");
+CmdArgs.push_back("-luser32");
+CmdArgs.push_back("-lkernel32");
}
- CmdArgs.push_back("-ladvapi32");
- CmdArgs.push_back("-lshell32");
- CmdArgs.push_back("-luser32");
- CmdArgs.push_back("-lkernel32");
if (Args.hasArg(options::OPT_static))
CmdArgs.push_back("--end-group");
Index: test/Driver/mingw-windowsapp.c
===
--- test/Driver/mingw-windowsapp.c
+++ test/Driver/mingw-windowsapp.c
@@ -0,0 +1,6 @@
+// RUN: %clang -v -target i686-pc-windows-gnu -### %s 2>&1 | FileCheck -check-prefix=CHECK_DEFAULT %s
+// RUN: %clang -v -target i686-pc-windows-gnu -### %s -lwindowsapp 2>&1 | FileCheck -check-prefix=CHECK_WINDOWSAPP %s
+
+// CHECK_DEFAULT: "-lmsvcrt" "-ladvapi32" "-lshell32" "-luser32" "-lkernel32" "-lmingw32"
+// CHECK_WINDOWSAPP: "-lwindowsapp" "-lmingw32"
+// CHECK_WINDOWSAPP-SAME: "-lmsvcrt" "-lmingw32"
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -201,6 +201,14 @@
CmdArgs.push_back("-Bdynamic");
}
+ bool HasWindowsApp = false;
+ for (auto Lib : Args.getAllArgValues(options::OPT_l)) {
+if (Lib == "windowsapp") {
+ HasWindowsApp = true;
+ break;
+}
+ }
+
if (!Args.hasArg(options::OPT_nostdlib)) {
if (!Args.hasArg(options::OPT_nodefaultlibs)) {
if (Args.hasArg(options::OPT_static))
@@ -223,15 +231,19 @@
if (Args.hasArg(options::OPT_pthread))
CmdArgs.push_back("-lpthread");
- // add system libraries
- if (Args.hasArg(options::OPT_mwindows)) {
-CmdArgs.push_back("-lgdi32");
-CmdArgs.push_back("-lcomdlg32");
+ if (!HasWindowsApp) {
+// Add system libraries. If linking to libwindowsapp.a, that import
+// library replaces all these and we shouldn't accidentally try to
+// link to the normal desktop mode dlls.
+if (Args.hasArg(options::OPT_mwindows)) {
+ CmdArgs.push_back("-lgdi32");
+ CmdArgs.push_back("-lcomdlg32");
+}
+CmdArgs.push_back("-ladvapi32");
+CmdArgs.push_back("-lshell32");
+CmdArgs.push_back("-luser32");
+CmdArgs.push_back("-lkernel32");
}
- CmdArgs.push_back("-ladvapi32");
- CmdArgs.push_back("-lshell32");
- CmdArgs.push_back("-luser32");
- CmdArgs.push_back("-lkernel32");
if (Args.hasArg(options::OPT_static))
CmdArgs.push_back("--end-group");
___
cfe-commits mail
[PATCH] D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified
smeenai added inline comments. 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" mstorsjo wrote: > 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). 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. Repository: rC Clang https://reviews.llvm.org/D49059 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified
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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified
smeenai added inline comments. 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" Why do we end up with -lmingw32 twice, and why not just check the full line, like you're doing for the default case? Repository: rC Clang https://reviews.llvm.org/D49059 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified
smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.
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?
Comment at: lib/Driver/ToolChains/MinGW.cpp:207
+if (Lib == "windowsapp")
+ HasWindowsApp = true;
+
I don't think it matters very much, but you could break out here.
Comment at: lib/Driver/ToolChains/MinGW.cpp:232
+ if (!HasWindowsApp) {
+// add system libraries
+if (Args.hasArg(options::OPT_mwindows)) {
Might be worth adding a short note to this comment about why we skip adding
these libraries in the WindowsApp case?
Repository:
rC Clang
https://reviews.llvm.org/D49059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified
mstorsjo created this revision.
mstorsjo added reviewers: rnk, martell, compnerd, smeenai.
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.
Repository:
rC Clang
https://reviews.llvm.org/D49059
Files:
lib/Driver/ToolChains/MinGW.cpp
test/Driver/mingw-windowsapp.c
Index: test/Driver/mingw-windowsapp.c
===
--- /dev/null
+++ test/Driver/mingw-windowsapp.c
@@ -0,0 +1,6 @@
+// RUN: %clang -v -target i686-pc-windows-gnu -### %s 2>&1 | FileCheck
-check-prefix=CHECK_DEFAULT %s
+// RUN: %clang -v -target i686-pc-windows-gnu -### %s -lwindowsapp 2>&1 |
FileCheck -check-prefix=CHECK_WINDOWSAPP %s
+
+// CHECK_DEFAULT: "-lmsvcrt" "-ladvapi32" "-lshell32" "-luser32" "-lkernel32"
"-lmingw32"
+// CHECK_WINDOWSAPP: "-lwindowsapp" "-lmingw32"
+// CHECK_WINDOWSAPP-SAME: "-lmsvcrt" "-lmingw32"
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -201,6 +201,11 @@
CmdArgs.push_back("-Bdynamic");
}
+ bool HasWindowsApp = false;
+ for (auto Lib : Args.getAllArgValues(options::OPT_l))
+if (Lib == "windowsapp")
+ HasWindowsApp = true;
+
if (!Args.hasArg(options::OPT_nostdlib)) {
if (!Args.hasArg(options::OPT_nodefaultlibs)) {
if (Args.hasArg(options::OPT_static))
@@ -223,15 +228,17 @@
if (Args.hasArg(options::OPT_pthread))
CmdArgs.push_back("-lpthread");
- // add system libraries
- if (Args.hasArg(options::OPT_mwindows)) {
-CmdArgs.push_back("-lgdi32");
-CmdArgs.push_back("-lcomdlg32");
+ if (!HasWindowsApp) {
+// add system libraries
+if (Args.hasArg(options::OPT_mwindows)) {
+ CmdArgs.push_back("-lgdi32");
+ CmdArgs.push_back("-lcomdlg32");
+}
+CmdArgs.push_back("-ladvapi32");
+CmdArgs.push_back("-lshell32");
+CmdArgs.push_back("-luser32");
+CmdArgs.push_back("-lkernel32");
}
- CmdArgs.push_back("-ladvapi32");
- CmdArgs.push_back("-lshell32");
- CmdArgs.push_back("-luser32");
- CmdArgs.push_back("-lkernel32");
if (Args.hasArg(options::OPT_static))
CmdArgs.push_back("--end-group");
Index: test/Driver/mingw-windowsapp.c
===
--- /dev/null
+++ test/Driver/mingw-windowsapp.c
@@ -0,0 +1,6 @@
+// RUN: %clang -v -target i686-pc-windows-gnu -### %s 2>&1 | FileCheck -check-prefix=CHECK_DEFAULT %s
+// RUN: %clang -v -target i686-pc-windows-gnu -### %s -lwindowsapp 2>&1 | FileCheck -check-prefix=CHECK_WINDOWSAPP %s
+
+// CHECK_DEFAULT: "-lmsvcrt" "-ladvapi32" "-lshell32" "-luser32" "-lkernel32" "-lmingw32"
+// CHECK_WINDOWSAPP: "-lwindowsapp" "-lmingw32"
+// CHECK_WINDOWSAPP-SAME: "-lmsvcrt" "-lmingw32"
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -201,6 +201,11 @@
CmdArgs.push_back("-Bdynamic");
}
+ bool HasWindowsApp = false;
+ for (auto Lib : Args.getAllArgValues(options::OPT_l))
+if (Lib == "windowsapp")
+ HasWindowsApp = true;
+
if (!Args.hasArg(options::OPT_nostdlib)) {
if (!Args.hasArg(options::OPT_nodefaultlibs)) {
if (Args.hasArg(options::OPT_static))
@@ -223,15 +228,17 @@
if (Args.hasArg(options::OPT_pthread))
CmdArgs.push_back("-lpthread");
- // add system libraries
- if (Args.hasArg(options::OPT_mwindows)) {
-CmdArgs.push_back("-lgdi32");
-CmdArgs.push_back("-lcomdlg32");
+ if (!HasWindowsApp) {
+// add system libraries
+if (Args.hasArg(options::OPT_mwindows)) {
+ CmdArgs.push_back("-lgdi32");
+ CmdArgs.push_back("-lcomdlg32");
+}
+CmdArgs.push_back("-ladvapi32");
+CmdArgs.push_back("-lshell32");
+CmdArgs.push_back("-luser32");
+CmdArgs.push_back("-lkernel32");
}
- CmdArgs.push_back("-ladvapi32");
- CmdArgs.push_back("-lshell32");
- CmdArgs.push_back("-luser32");
- CmdArgs.push_back("-lkernel32");
if (Args.hasArg(options::OPT_static))
CmdArgs.push_back("--end-group");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-
