[PATCH] D73457: [Clang] Warn about 'z' printf modifier in old MSVC.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham marked an inline comment as done.
simon_tatham added inline comments.


================
Comment at: clang/lib/AST/FormatString.cpp:754
+          LO.isMSCompatibilityVersionSpecified() &&
+          !LO.isCompatibleWithMSVC(LangOptions::MSVC2015)) {
+        // The standard libraries before MSVC2015 didn't support the 'z' length
----------------
aaron.ballman wrote:
> I'd rather not see `isMSCompatibilityVersionSpecified()` be introduced, but instead make `isCompatibleWithMSVC()` do the right thing when it's not specified.
I tried making `isCompatibleWithMSVC` return true if `MSCompatibilityVersion == 0` on the basis that it should be considered 'newest' rather than 'oldest'. That caused a lot of knock-on test failures which I don't really understand all of:
```    Clang :: CodeGenCXX/dllexport-no-dllexport-inlines.cpp
    Clang :: CodeGenCXX/exceptions-cxx-new.cpp
    Clang :: CodeGenCXX/mangle-ms-exception-spec.cpp
    Clang :: CodeGenCXX/msabi-blocks.cpp
    Clang :: Rewriter/properties.m
    Clang :: SemaCXX/microsoft-cxx0x.cpp
    Clang :: SemaCXX/pragma-init_seg.cpp
    Clang :: SemaCXX/warn-static-outside-class-definition.cpp
```
`mangle-ms-exception-spec.cpp` in particular looks as if it's expecting some kind of completely different mangling without any compatibility version.

Perhaps I should go with the existing behavior and make 'unspecified' keep defaulting to oldest rather than newest? I think in general the driver will not leave it unspecified, so perhaps it won't make much difference outside the test suite anyway. Hmmm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73457