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

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

Reply via email to