rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land.
lgtm ================ Comment at: lib/Driver/MSVCToolChain.cpp:34 + #if 0 + #define USE_VS_SETUP_CONFIG + #endif ---------------- hamzasood wrote: > rnk wrote: > > What are the outstanding issues preventing you from enabling this by > > default? > Building on Win32 doesn't imply that you'll have the required header; it > currently needs to be installed [[ > https://blogs.msdn.microsoft.com/heaths/2016/09/15/changes-to-visual-studio-15-setup/ > | separately ]] via nuget. While it would be possible to have cmake create a > packages.config file when configuring a visual studio project, the API is > only at the RC stage and so the distribution method could potentially change > between now and the Visual Studio 2017 release (even if that's not a concern, > it's probably out of the scope of this patch anyway). > Although the code looks useless sitting there ifdefed out, it could be useful > for someone eager enough to get the package themselves and run a custom Clang > build. > In the meantime, Visual Studio 2017 installations can only be detected when > running in the correct developer command prompt, or by putting one of its > toolchain's bin directories at the top of PATH. This patch looks good, so I don't want to block it, but can you add something like: // FIXME: Use cmake to auto-detect if the necessary headers exist. I'm assuming we should test for <Setup.Configuration.h> eventually. https://reviews.llvm.org/D28365 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
