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