[PATCH] D53066: [RFC] [MinGW] Print paths with forward slashes in commands printed with -v

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.


================
Comment at: lib/Driver/Job.cpp:104-105
+  std::string Buf;
+  if (PathStyle == llvm::sys::path::Style::posix) {
+    Buf = llvm::sys::path::convert_to_slash(Arg);
+    Arg = Buf;
----------------
rnk wrote:
> mstorsjo wrote:
> > rnk wrote:
> > > This is blindly treating every argument as if it were a path. I can imagine someone wanting to define a macro to a string value containing backslashes, such as: `-DAWESOME_MACRO="\"asdf\""`
> > > Given that this is used for -### and crash diagnostic printing, I'm hesitant to do this here. I think it might be better to, in a cygming environment, try to convert all our paths to forward slashes before we add them to the command line.
> > A quick test of the concept is at http://martin.st/temp/clang-forward-slash.patch; this seems to be roughly enough to get forward slashes for all paths in the linker command at least. Does this look like what you have in mind?
> > 
> > For a plain "clang -v test.c", this takes care of everything except the temporary object file. It doesn't do anything for the compile command though. This should be roughly the minimal amount needed for this particular case of libtool, but then again makes the -v output wildly inconsistent.
> > 
> > Changing the path separator earlier would kinda require changing the preferred path separator throughout all callsites to any path api, or change the path utils to allow overriding the global default somehow.
> I think our output is already pretty inconsistent. Most build systems use forward slash paths internally, so that's what the compiler sees on the command line. Then, we dutifully take those paths and join them with the "preferred" separator (`\`), and make paths with mixed slashes, which nobody wants.
> 
> I would go with the patch you put forward, if that solves the issue. Everything there is very gnu toolchain oriented, it makes sense to try to match their path style.
> 
> The one change I might object to is `ToolChain::GetProgramPath`, since cmd really wants the name of the program to have back slashes. As long as that comes out with backslashes in -### and crash report .sh files, I'd be OK with it.
Ok, thanks - I'll test run things with that patch (minus the executable change) and update this review properly later then.


Repository:
  rC Clang

https://reviews.llvm.org/D53066