[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

Tue Aug 25 05:13:32 PDT 2020

hans added inline comments.

================
Comment at: clang/test/Driver/cl-options.c:686
+// vctoolsdir is handled by the driver; just check that we don't error. Pass -c because fakedir isn't a real toolchain path
+// RUN: %clang_cl -c -vctoolsdir fakedir -- %s 2>&1
+
----------------
zahen wrote:
> zahen wrote:
> > hans wrote:
> > > zahen wrote:
> > > > hans wrote:
> > > > > aganea wrote:
> > > > > > Check that we're not detecting a local installation, and that we fallback to the default triple 19.11, ie.
> > > > > > ```
> > > > > > // RUN: %clang_cl ... -vctoolsdir "" -### | FileCheck %s --check-prefix VCTOOLSDIR
> > > > > > // VCTOOLSDIR: "-cc1" "-triple" "x86_64-pc-windows-msvc19.11.0"
> > > > > > ```
> > > > > Maybe add a comment explaining the purpose of the test.
> > > > > 
> > > > > And would it be possible to have a test where -vctoolsdir is set to something, to see that it's picked up correctly?
> > > > What's the expectation on the test boxes?  I can author such a test but it would be very brittle unless we have a spec for how VS should be installed.
> > > I'd suggest passing -vctoolsdir "/foo" and check that some /foo dir is getting passed as a system include dir to cc1, and that the link.exe invocation is under /foo.
> > > 
> > > I don't think it would be brittle.
> > Clever idea! I'll do it that way and key off the "ignored directory" warning that's emitted.
> Did further digging and the path test would depend on other environment variables.  If the %INCLUDE% variable is set, as it is by vcvars.bat, those paths are adopted and VCToolChainPath is not used.  I'm hesitant to change that logic without stronger motivation.
> 
> Other ideas for a test that would work run both from a vc command prompt and a vanilla command prompt?  I don't see anything else in the full verbose invocation to key off of:
> 
> 
> ```
> D:\repos\llvm\llvm-project\build\Debug\bin>clang-cl -vctoolsdir "/fake" -v main.cpp
> clang version 12.0.0 (https://github.com/llvm/llvm-project 537f5483fe4e09c39ffd7ac837c00b50dd13d676)
> Target: x86_64-pc-windows-msvc
> Thread model: posix
> InstalledDir: D:\repos\llvm\llvm-project\build\Debug\bin
>  "D:\\repos\\llvm\\llvm-project\\build\\Debug\\bin\\clang-cl.exe" -cc1 -triple x86_64-pc-windows-msvc19.11.0 -emit-obj -mrelax-all -mincremental-linker-compatible --mrelax-relocations -disable-free -main-file-name main.cpp -mrelocation-model pic -pic-level 2 -mframe-pointer=none -relaxed-aliasing -fmath-errno -fno-rounding-math -mconstructor-aliases -munwind-tables -target-cpu x86-64 -mllvm -x86-asm-syntax=intel -D_MT -flto-visibility-public-std --dependent-lib=libcmt --dependent-lib=oldnames -stack-protector 2 -fms-volatile -fdiagnostics-format msvc -v -resource-dir "D:\\repos\\llvm\\llvm-project\\build\\Debug\\lib\\clang\\12.0.0" -internal-isystem "D:\\repos\\llvm\\llvm-project\\build\\Debug\\lib\\clang\\12.0.0\\include" -internal-isystem "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.27.29110\\ATLMFC\\include" -internal-isystem "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.27.29110\\include" -internal-isystem "C:\\Program Files (x86)\\Windows Kits\\NETFXSDK\\4.8\\include\\um" -internal-isystem "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\ucrt" -internal-isystem "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\shared" -internal-isystem "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\um" -internal-isystem "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\winrt" -internal-isystem "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\cppwinrt" -fdeprecated-macro -fdebug-compilation-dir "D:\\repos\\llvm\\llvm-project\\build\\Debug\\bin" -ferror-limit 19 -fmessage-length=121 -fno-use-cxa-atexit -fms-extensions -fms-compatibility -fms-compatibility-version=19.11 -std=c++14 -fdelayed-template-parsing -fcolor-diagnostics -faddrsig -o "C:\\Users\\zahen\\AppData\\Local\\Temp\\main-8f329a.obj" -x c++ main.cpp
> clang -cc1 version 12.0.0 based upon LLVM 12.0.0git default target x86_64-pc-windows-msvc
> #include "..." search starts here:
> #include <...> search starts here:
>  D:\repos\llvm\llvm-project\build\Debug\lib\clang\12.0.0\include
>  C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\ATLMFC\include
>  C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\include
>  C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um
>  C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\ucrt
>  C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\shared
>  C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\um
>  C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\winrt
>  C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\cppwinrt
> End of search list.
>  "link.exe" -out:main.exe -nologo "C:\\Users\\zahen\\AppData\\Local\\Temp\\main-8f329a.obj"
> 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)
> ```
Maybe -vctoolsdir should take precedence over %INCLUDE%? It would at least be good to understand the semantics. Like if run inside a VC command prompt, should it be possible to use -vctoolsdir to point to a different VC installation?

Also, in the clang invocation above (using -### is probably better than -v, so it doesn't try to actually run the commands), shouldn't it be executing link.exe under /fake somewhere?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85998/new/

https://reviews.llvm.org/D85998