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


Repository:
  rC Clang

https://reviews.llvm.org/D53066



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

Reply via email to