zahen added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:73
+ MSVCToolChain::ToolsetLayout &VSLayout) {
+ // Don't validate the input; trust the value supplied by the user.
+ // The primary motivation is to prevent unnecessary file and registry access.
----------------
hans wrote:
> zahen wrote:
> > hans wrote:
> > > zahen wrote:
> > > > aganea wrote:
> > > > > hans wrote:
> > > > > > What will the error look like if the user sets this but gets it
> > > > > > wrong? I'm sympathetic to not wanting to validate it, but if it's
> > > > > > wrong it would be nice if the error isn't too cryptic.
> > > > > @hans The flag proposed here behaves like `env VCToolsInstallDir=""
> > > > > clang-cl ...`, and in that case there's no validation. Do you think
> > > > > the path to `-vctoolsdir` should be validated and throw an error
> > > > > otherwise? I find it nice to completly disable the toolchain
> > > > > auto-detection if passing in a empty dir, without needing an
> > > > > additional flag, ie `clang-cl -vctoolsdir ""`.
> > > > ```
> > > > D:\>clang-cl -vctoolsdir fake main.cpp
> > > > clang-cl: error: unable to execute command: program not executable
> > > > clang-cl: error: linker command failed with exit code 1 (use -v to see
> > > > invocation)
> > > > ```
> > > I realize this is not because of your patch, but it would be nice if this
> > > error could be more clear. Ideally it would at least print the path to
> > > what's failing to execute. Would you be willing to add that?
> > It's printed when using -v as directed. I don't feel strongly against
> > adding the command line in all failure cases, but did want to point that
> > out. Still think it's worth it?
> No, I'm not asking for it to be validated, but if e.g. execution of link.exe
> fails, it would be nice if the error message at least printed the path it was
> trying to execute so the user has a chance to see what's wrong.
and the invocation with a bogus directory is as follows. No explicit path
provided
```
"link.exe" ...
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85998/new/
https://reviews.llvm.org/D85998
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
