[PATCH] D78508: [Clang] Allow long as size_t printf argument on 32-bit Windows platforms.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/AST/FormatString.cpp:407
+            if ((isSizeT() || isPtrdiffT()) &&
+                C.getTargetInfo().getTriple().isOSMSVCRT() &&
+                C.getTargetInfo().getTriple().isArch32Bit())
----------------
amccarth wrote:
> I'm not convinced `isOSMSVCRT` is the right check.  The troubling typedefs are [[ https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types#size-t | `SIZE_T` and `SSIZE_T` ]], which come from the Windows API and are unrelated to Microsoft's C run-time library.  I think `isOSWindows` would be more precise.
We already care about typedef-names used to spell argument types in some cases (see `namedTyoeToLengthModifier`. If this is a case of MSVC providing a typedef that people *think* is `size_t`, that is always the same size and representation as `size_t`, and that is intended to be fully interchangeable with `size_t`, but sometimes is a different type, then it might be reasonable to detect whether the type was spelled using that `SIZE_T` typedef when targeting Windows, and exempt only that case.

I mean, assuming that we want to allow such code at all and not push people to cast explicitly from `SIZE_T` to `size_t`. I don't have a strong opinion on that; I think it rather depends on intended idiomatic usage on that platform.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78508