amccarth added a comment.

In D73457#1842493 <https://reviews.llvm.org/D73457#1842493>, @simon_tatham 
wrote:

> Removed the special case for `MSCompatibilityVersion == 0`. If the default 
> compatibility setting needs to be changed, that's a separate piece of work 
> and should be done by someone who understands more than I do about all those 
> failing tests :-) The new version of this patch just uses the existing 
> default.


+1.  The issue of how the default version gets set is a separate issue.  I 
think it's best to keep this patch decoupled from that.

> With the typical command line I use with the `clang-cl` driver, a specific 
> version has been set anyway by the time cc1 is running. So probably I 
> shouldn't have been worrying in the first place about what happens if there 
> isn't one set.

The default you found for MSVC15 is the last result default.  (Though I would 
have expected that to be MSVC17 by now.)  I believe the driver will actually 
try to figure out the most recent version of MSVC installed on the machine 
(assuming it's Windows), and use that.  Only if it can't find one, will it use 
that default.

As I recall, this is because clang will still (by default) use the MS runtime 
libraries for Windows builds, in which case it's important for the 
compatibility version to match the one for the libraries that are going to be 
used.  I think we can/should reconsider this when clang-cl starts using 
different run-time libraries for Windows builds.

I'm not officially a reviewer on this patch, but LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73457/new/

https://reviews.llvm.org/D73457



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

Reply via email to