amccarth added inline comments. ================ Comment at: lib/Driver/MSVCToolChain.cpp:478 @@ +477,3 @@ + + const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(), + nullptr); ---------------- thakis wrote: > amccarth wrote: > > Yes, it looks in the executable (which I tried to emphasize with the method > > name). > > > > I don't think this is very expensive given that Explorer often makes > > zillions of such calls, but I'm open to other suggestions. > > > > I know that you can't use a library that's newer than the compiler (because > > it may use new language features), but I don't know if that applies in the > > other direction or how we would safely and reliably map directory names to > > library versions and therefore to compiler versions. > I agree that figuring out the right value for fmsc-version automatically > somehow is definitely something we should do. > > I forgot that `getVisualStudioBinariesFolder` already works by looking for > cl.exe in PATH, so cl.exe's metadata is already warmed up in the disk cache. > However, GetFileVersionInfoW() probably opens cl.exe itself and does some PE > parsing to get at the version, and that probably is in cold cache territory. > (https://msdn.microsoft.com/en-us/library/windows/desktop/ms647003(v=vs.85).aspx > suggests that this function might open several files). > > `getVisualStudioBinariesFolder` checks: > > 1. getenv("VCINSTALLDIR") > 2. cl.exe in getenv("PATH") > 3. registry (via getVisualStudioInstallDir) > > The common cases are 1 and 3. For 1, for default installs, the version number > is part of the directory name (for default installs, what most people have). > For 3, the version number is in the registry key we query. So in most cases > we shouldn't have to look at cl.exe itself. And for the cases where we would > have to look, maybe it's ok to require an explicit fmsc-version flag. The version number in the directory name and the registry is the version number of Visual Studio not of the compiler. Yes, we could do a mapping (VS 14 comes bundled with CL 19), assuming Microsoft continues to keep VS releases and compiler releases in sync, and it means this code will forever need updates to the mapping data.
The mapping would give just the major version number, which might be all that matters now, but if there's ever a CL 19.1 that has different compatibility requirements (and is maybe released out-of-band with Visual Studio), we'd be stuck. Getting the actual version from the compiler seems the most accurate and future-proof way to check. If that's too expensive, then maybe we should abandon the idea of detecting the default for compatibility. http://reviews.llvm.org/D20136 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
