sbc100 marked an inline comment as done.
sbc100 added inline comments.
================
Comment at: clang/tools/driver/driver.cpp:391
+ if (ClangCLMode ||
+ llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows())
+ Tokenizer = &llvm::cl::TokenizeWindowsCommandLine;
----------------
probinson wrote:
> Testing the triple is a functional change. I should think the test for CL
> mode is sufficient. Note that for PS4 we run on Windows but use a
> GNU-compatible command line and response-file style. If you want
> Windows-style on Windows, invoke the compiler as "clang-cl" instead of
> "clang".
Yes this is a functional change. It changes the default when running on
windows. I will see if I can write a (windows-only) test for it.
Some more background for why I think this change is right way to go:
In the emscripten toolchain we run clang as a cross compiler, we don't want
cl.exe emulation.
We drive the compiler and other tools from a python script and we create
response files whenever a command line is tool long.
It doesn't make sense to me that we should use windows response files for all
other tools except for `clang`. In particular, `llvm-ar`, `llc`, `opt`,
`wasm-ld` all default to windows response files on windows.
The function in question in our toolchain looks like this:
```
def generate_response_file(command):
if is_windows:
use_windows_style
else:
use_gnu_style
```
Without this change I have to write something like this:
```
def generate_response_file(command):
# On windows all the tools we run use windows style response files.
# Except for clang (for some reason?).
if is_windows and 'clang.exe' not in command[0] :
use_windows_style
else:
use_gnu_style
```
I don't see why clang would want to be different here. Can you think of any
reason?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80876/new/
https://reviews.llvm.org/D80876
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
