[PATCH] D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGee0a3b5c776c: [MinGW] Implicitly add .exe suffix if not 
provided (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71400/new/

https://reviews.llvm.org/D71400

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-implicit-extension-cross.c
  clang/test/Driver/mingw-implicit-extension-windows.c


Index: clang/test/Driver/mingw-implicit-extension-windows.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-windows.c
@@ -0,0 +1,14 @@
+// Test how an implicit .exe extension is added. If running the compiler
+// on windows, an implicit extension is added if none is provided in the
+// given name. (Therefore, this test is skipped when not running on windows.)
+
+// REQUIRES: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck 
%s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.exe 2>&1 | 
FileCheck %s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.q 2>&1 | 
FileCheck %s --check-prefix=CHECK-OUTPUTNAME-Q
+
+// CHECK-OUTPUTNAME-EXE: "-o" "outputname.exe"
+// CHECK-OUTPUTNAME-Q: "-o" "outputname.q"
Index: clang/test/Driver/mingw-implicit-extension-cross.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-cross.c
@@ -0,0 +1,9 @@
+// Test how an implicit .exe extension is added. If not running the compiler
+// on windows, no implicit extension is added. (Therefore, this test is skipped
+// when running on windows.)
+
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck 
%s
+
+// CHECK: "-o" "outputname"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -160,7 +160,19 @@
   }
 
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
+  const char *OutputFile = Output.getFilename();
+  // GCC implicitly adds an .exe extension if it is given an output file name
+  // that lacks an extension. However, GCC only does this when actually
+  // running on windows, not when operating as a cross compiler. As some users
+  // have come to rely on this behaviour, try to replicate it.
+#ifdef _WIN32
+  if (!llvm::sys::path::has_extension(OutputFile))
+CmdArgs.push_back(Args.MakeArgString(Twine(OutputFile) + ".exe"));
+  else
+CmdArgs.push_back(OutputFile);
+#else
+  CmdArgs.push_back(OutputFile);
+#endif
 
   Args.AddAllArgs(CmdArgs, options::OPT_e);
   // FIXME: add -N, -n flags


Index: clang/test/Driver/mingw-implicit-extension-windows.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-windows.c
@@ -0,0 +1,14 @@
+// Test how an implicit .exe extension is added. If running the compiler
+// on windows, an implicit extension is added if none is provided in the
+// given name. (Therefore, this test is skipped when not running on windows.)
+
+// REQUIRES: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck %s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.exe 2>&1 | FileCheck %s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.q 2>&1 | FileCheck %s --check-prefix=CHECK-OUTPUTNAME-Q
+
+// CHECK-OUTPUTNAME-EXE: "-o" "outputname.exe"
+// CHECK-OUTPUTNAME-Q: "-o" "outputname.q"
Index: clang/test/Driver/mingw-implicit-extension-cross.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-cross.c
@@ -0,0 +1,9 @@
+// Test how an implicit .exe extension is added. If not running the compiler
+// on windows, no implicit extension is added. (Therefore, this test is skipped
+// when running on windows.)
+
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck %s
+
+// CHECK: "-o" "outputname"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -160,7 +160,19 @@
   }
 
   

[PATCH] D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71400/new/

https://reviews.llvm.org/D71400



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71400/new/

https://reviews.llvm.org/D71400



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 233863.
mstorsjo marked 2 inline comments as done.
mstorsjo added a comment.

Added a code comment, using `llvm::path::has_extension`, added testcases.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71400/new/

https://reviews.llvm.org/D71400

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-implicit-extension-cross.c
  clang/test/Driver/mingw-implicit-extension-windows.c


Index: clang/test/Driver/mingw-implicit-extension-windows.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-windows.c
@@ -0,0 +1,14 @@
+// Test how an implicit .exe extension is added. If running the compiler
+// on windows, an implicit extension is added if none is provided in the
+// given name. (Therefore, this test is skipped when not running on windows.)
+
+// REQUIRES: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck 
%s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.exe 2>&1 | 
FileCheck %s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.q 2>&1 | 
FileCheck %s --check-prefix=CHECK-OUTPUTNAME-Q
+
+// CHECK-OUTPUTNAME-EXE: "-o" "outputname.exe"
+// CHECK-OUTPUTNAME-Q: "-o" "outputname.q"
Index: clang/test/Driver/mingw-implicit-extension-cross.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-cross.c
@@ -0,0 +1,9 @@
+// Test how an implicit .exe extension is added. If not running the compiler
+// on windows, no implicit extension is added. (Therefore, this test is skipped
+// when running on windows.)
+
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck 
%s
+
+// CHECK: "-o" "outputname"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -160,7 +160,19 @@
   }
 
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
+  const char *OutputFile = Output.getFilename();
+  // GCC implicitly adds an .exe extension if it is given an output file name
+  // that lacks an extension. However, GCC only does this when actually
+  // running on windows, not when operating as a cross compiler. As some users
+  // have come to rely on this behaviour, try to replicate it.
+#ifdef _WIN32
+  if (!llvm::sys::path::has_extension(OutputFile))
+CmdArgs.push_back(Args.MakeArgString(Twine(OutputFile) + ".exe"));
+  else
+CmdArgs.push_back(OutputFile);
+#else
+  CmdArgs.push_back(OutputFile);
+#endif
 
   Args.AddAllArgs(CmdArgs, options::OPT_e);
   // FIXME: add -N, -n flags


Index: clang/test/Driver/mingw-implicit-extension-windows.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-windows.c
@@ -0,0 +1,14 @@
+// Test how an implicit .exe extension is added. If running the compiler
+// on windows, an implicit extension is added if none is provided in the
+// given name. (Therefore, this test is skipped when not running on windows.)
+
+// REQUIRES: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck %s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.exe 2>&1 | FileCheck %s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.q 2>&1 | FileCheck %s --check-prefix=CHECK-OUTPUTNAME-Q
+
+// CHECK-OUTPUTNAME-EXE: "-o" "outputname.exe"
+// CHECK-OUTPUTNAME-Q: "-o" "outputname.q"
Index: clang/test/Driver/mingw-implicit-extension-cross.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-cross.c
@@ -0,0 +1,9 @@
+// Test how an implicit .exe extension is added. If not running the compiler
+// on windows, no implicit extension is added. (Therefore, this test is skipped
+// when running on windows.)
+
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck %s
+
+// CHECK: "-o" "outputname"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -160,7 +160,19 @@
   }
 
   CmdArgs.push_back("-o");
-  

[PATCH] D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/Driver/ToolChains/MinGW.cpp:164
+  const char *OutputFile = Output.getFilename();
+#ifdef _WIN32
+  if (!llvm::sys::path::filename(OutputFile).contains('.'))

Can you add what you wrote in the commit message as a comment here to explain 
the divergence in behavior based on the compiler's host OS? I can imagine that 
a future maintainer will try to make the code behave the same way regardless of 
host.



Comment at: clang/lib/Driver/ToolChains/MinGW.cpp:165
+#ifdef _WIN32
+  if (!llvm::sys::path::filename(OutputFile).contains('.'))
+CmdArgs.push_back(Args.MakeArgString(Twine(OutputFile) + ".exe"));

I think this can be spelled `llvm::sys::path::has_extension`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71400/new/

https://reviews.llvm.org/D71400



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: rnk.
Herald added a project: clang.

GCC implicitly adds an .exe suffix if it is given an output file name, but the 
file name doesn't contain a suffix, and there are certain users of GCC that 
rely on this behaviour (and run into issues when trying to use Clang instead of 
GCC). And MSVC's cl.exe also does the same (but not link.exe).

However, GCC only does this when actually running on windows, not when 
operating as a cross compiler.

This was reported to me at https://github.com/mstorsjo/llvm-mingw/issues/60.

As GCC doesn't have this behaviour when cross compiling, we definitely 
shouldn't introduce the behaviour in such cases (as it would break at least as 
many cases as this fixes).

Is it worth adding such inconsistent behaviour (with two separate tests, one 
for running on windows and one elsewhere)?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71400

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp


Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -160,7 +160,15 @@
   }
 
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
+  const char *OutputFile = Output.getFilename();
+#ifdef _WIN32
+  if (!llvm::sys::path::filename(OutputFile).contains('.'))
+CmdArgs.push_back(Args.MakeArgString(Twine(OutputFile) + ".exe"));
+  else
+CmdArgs.push_back(OutputFile);
+#else
+  CmdArgs.push_back(OutputFile);
+#endif
 
   Args.AddAllArgs(CmdArgs, options::OPT_e);
   // FIXME: add -N, -n flags


Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -160,7 +160,15 @@
   }
 
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
+  const char *OutputFile = Output.getFilename();
+#ifdef _WIN32
+  if (!llvm::sys::path::filename(OutputFile).contains('.'))
+CmdArgs.push_back(Args.MakeArgString(Twine(OutputFile) + ".exe"));
+  else
+CmdArgs.push_back(OutputFile);
+#else
+  CmdArgs.push_back(OutputFile);
+#endif
 
   Args.AddAllArgs(CmdArgs, options::OPT_e);
   // FIXME: add -N, -n flags
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits