[PATCH] D27477: Refactor how the MSVC toolchain searches for a compatibility version.

[David L. Jones via Phabricator via llvm-commits]

dlj marked 2 inline comments as done.
dlj added a comment.

In https://reviews.llvm.org/D27477#615983, @rnk wrote:

> Can you update the commit message to outline the new algorithm? I think it's the same, except on all platforms, -fmsc-version and -fms-compatibility version are considered. Only with the MSVC toolchain do we go to the triple step, then the exe, then default to 18 with -fms-extensions. I guess this lets people compile code for Linux while pretending to be MSVC.


Right, so I think that there is actually *no* change from the previous behavior. The tests in Misc/diag-format.c in particular failed without this (MSVC2010 needs to be detected for the off-by-one in the diagnostics). The only code path that didn't appear to be tested was what I'm adding in https://reviews.llvm.org/D27498.

It might make sense for the MSVC logic to only live in the MSVC toolchain, and I'd be happy to move it there if you agree, but I'd prefer to do that as a follow-up, since this is already sort of non-trivial.



================
Comment at: lib/Driver/MSVCToolChain.cpp:476
 
+VersionTuple MSVCToolChain::separateMSVCFullVersion() const {
+  unsigned Major, Minor, Micro;
----------------
rnk wrote:
> This function isn't doing the separation. Whatever code does the repeated mod 100 division is the separation code. This is more `getMSVCVersionFromTriple`.
Ah, sorry about that... I misunderstood your previous comment.


https://reviews.llvm.org/D27477