[PATCH] D42762: Rewrite the VS Integration Scripts

Wed Jan 31 13:02:44 PST 2018

zturner created this revision.
zturner added reviewers: hans, rnk, amccarth, aaron.ballman, thakis.
Herald added subscribers: hintonda, kristof.beyls, mgorny, aemerson.

Not sure who knows MSBuild, so I'm adding several random people in hopes that someone can review this.

I expect this to need to go through a couple of iterations, but this represents a complete re-write of the Clang Visual Studio Integration.  I'll try to summarize the high level differences between how this works now and how it used to work, as well as mention some future work I think we should do:

1. Previously we would install into `%ProgramFiles%\MSBuild`.  At least with 2017, this no longer appears to be what you do.  You now have to install to the VS instance.  Rather than try to detect Installed VS Instances from a batch file, for now I just hardcode the path to the default installation path, and in follow-up patches we can port this to using a VSIX installer, which should handle all of this for us.

2. Previously we had separate `.props` and `.targets` files for every version of VS.  This turns out to be unnecessary, so all of those have been removed and now there is just 1.  The good news is that this means we should continue to work in subsequent versions of VS without any additional maintenance.

3. Previously would do a configure-style modification of the props and targets file to fill in with compile time values.  The only one of these that's tricky is the LLVM Version number, the rest are already available via existing MSBuild substitutions.  So all of those are fixed, and to handle the version number the installer is updated to write an additional value to the registry, which the MSBuild files pull.  Decoupling these values from the particular build of LLVM opens the door for us to be able to package this in a VSIX and distribute it on the marketplace and update it independently of the actual toolchain.

4. An xml file is provided to present a custom view of compiler options (e.g. Project > Properties > C/C++).  Options we don't support or ignore are grouped together under special tabs in the UI, while the settings we do support are in the original locations.  This is done because we additionally will now warn (or error) on certain options which could have been enabled in a project prior to changing the toolset, and we like to be able to point the user exactly to which setting to change to silence the warning / error.

5. Previously we would copy `clang-cl.exe` to `$(InstallDir)\bin\cl.exe` because this is how MSBuild would find the compiler.  This is now fixed so that it locates `clang-cl.exe` directly.

6. Options that we ignore are now not even passed to clang-cl.  This might seem unimportant because if we ignore the option, there's no harm in passing it in.  But some of these options trigger additional MSBuild logic that happens before the compiler even has a chance to run.  For example, in llvm.org/pr36140, a user noted that using `/Zi` triggered a full rebuild every time, even if nothing changed.  In any case, having smaller command lines makes for easier diagnostics.  An additional side benefit of this is that whereas before you would get lots of `-Wunused-command-line-argument` warnings, now there will be no such warnings.

Future Work (in rough order of priority):

1. Wrap this all in a VSIX so that it can be properly installed, instead of relying on batch file hackery.
2. Expose all of clang warnings through a custom UI page.
3. Expose all of clang optimizations through a custom UI page.
4. Replace `link.exe` with `lld-link.exe` similar to how we currently replace `cl.exe` with `clang-cl.exe`
5. Install a clang platform (in additional to a platform toolset), so that we have `Clang (x86)` and `Clang (x64)` platforms for people to enable building side-by-side configurations.
6. Add a 'Clang (Cross-Platform)` platform that exposes the `clang++` driver in all of its glory and allows the user to set the target triple.

https://reviews.llvm.org/D42762

Files:
  llvm/CMakeLists.txt
  llvm/tools/msbuild/CMakeLists.txt
  llvm/tools/msbuild/Clang.Cpp.Common.props
  llvm/tools/msbuild/Clang.Toolset.x64.props
  llvm/tools/msbuild/Clang.Toolset.x86.props
  llvm/tools/msbuild/LLVM.props
  llvm/tools/msbuild/Microsoft.Cpp.Win32.LLVM-vs2010.targets
  llvm/tools/msbuild/Microsoft.Cpp.Win32.LLVM-vs2012.targets
  llvm/tools/msbuild/Microsoft.Cpp.Win32.LLVM-vs2012_xp.targets
  llvm/tools/msbuild/Microsoft.Cpp.Win32.llvm.props.in
  llvm/tools/msbuild/Toolset.targets
  llvm/tools/msbuild/clang-cl.xml
  llvm/tools/msbuild/install.bat
  llvm/tools/msbuild/toolset-vs2013.targets
  llvm/tools/msbuild/toolset-vs2013_xp.targets
  llvm/tools/msbuild/toolset-vs2014.targets
  llvm/tools/msbuild/toolset-vs2014_xp.targets

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D42762.132243.patch
Type: text/x-patch
Size: 82706 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180131/a8224487/attachment-0001.bin>