[clang][Sema] check default argument promotions for printf

[inclyc]

Summary

The main focus of this patch is to make ArgType::matchesType check for
possible default parameter promotions when the argType is not a pointer.
If so, no warning will be given for int, unsigned int types as
corresponding arguments to %hhd and %hd. However, the usage of %hhd
corresponding to short is relatively rare, and it is more likely to be a
misuse. This patch keeps the original behavior of clang like this as
much as possible, while making it more convenient to consider the
default arguments promotion.

Fixes https://github.com/llvm/llvm-project/issues/57102

[inclyc]

https://reviews.llvm.org/D132266

This patch seems to ignore this scenario:

printf("%hhf", [int])

Where we have length modifier h, and the argument type is int. Causes clang to suppress warnings about type mismatches between floats and integers.

clang/test/FixIt/format.mm

10 ↗

(On Diff #455235)

The language built-in wchar_t will be properly diagnosed.

clang/test/SemaObjC/format-strings-objc.m

194 ↗

(On Diff #455235)

This wchar_t is defined as typedef __WCHAR_TYPE__ wchar_t;. Actually it is int on my machine, I'm not sure if we should still report this warning as before, because it is just int, not a real "wchar_t".

[aaron.ballman]

Thank you for the patch, but it'd be really helpful to me as a reviewer if you and @nickdesaulniers could coordinate so there's only one patch trying to address #57102 instead of two competing patches (I'm happy to review whichever patch comes out). From a quick look over the test case changes, this patch looks like it's more in line with how I'd expect to resolve that issue compared to D132266.

[nickdesaulniers]

In D132568#3746551, @aaron.ballman wrote:

Thank you for the patch, but it'd be really helpful to me as a reviewer if you and @nickdesaulniers could coordinate so there's only one patch trying to address #57102 instead of two competing patches (I'm happy to review whichever patch comes out). From a quick look over the test case changes, this patch looks like it's more in line with how I'd expect to resolve that issue compared to D132266.

The addition to clang/test/Sema/format-strings.c looks like it will resolve Linus' complaints in https://github.com/llvm/llvm-project/issues/57102; I'm happy to abandon D132266 in preference of this.

However, the usage of %hhd corresponding to short is relatively rare, and it is more likely to be a misuse.

Do we want to encode that in test_promotion in clang/test/Sema/format-strings.c? Seems like tests on shorts are missing.

clang/lib/AST/FormatString.cpp

359–360

might as well make this auto while you're here.

411–413

{} aren't necessary

417–420

{} aren't necessary

429–431

{} aren't necessary

clang/lib/Sema/SemaChecking.cpp

10084–10087

I think it would be slightly more readable to make these two assignments two dedicated statements.

clang/test/Sema/format-strings-scanf.c

271–272

Hmm...

274

delete empty newline

[inclyc]

Do we want to encode that in test_promotion in clang/test/Sema/format-strings.c? Seems like tests on shorts are missing.

Tests for short and char "incompatibility" could be found elsewhere in this file.

format-strings.c

void should_understand_small_integers(void) {
  printf("%hhu", (short) 10); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'short'}}
  printf("%hu\n", (unsigned char)1); // warning with -Wformat-pedantic only
  printf("%hu\n", (uint8_t)1);       // warning with -Wformat-pedantic only
}
/* ... */
void test13(short x) {
  char bel = 007;
  printf("bel: '0%hhd'\n", bel); // no-warning
  printf("x: '0%hhd'\n", x); // expected-warning {{format specifies type 'char' but the argument has type 'short'}}
}

Do I need to explicitly test again in the test_promotion?

[inclyc]

Address comments & more tests & docs

[inclyc]

rm {}

[inclyc]

Add tests for character literals

I've noticed that Linus mentioned the following code triggered warning by
clang. (With a little modifications shown below)

Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=21f9c8a13bb2a0c24d9c6b86bc0896542a28c197

printf("%hhd", 'a');
printf("%hd", 'a');

character literals are int in C. So clang-14 generates warnings like this:

local/format.c:9:20: warning: format specifies type 'char' but the argument has type 'int' [-Wformat]
    printf("%hhd", 'a');
            ~~~~   ^~~
            %d
local/format.c:10:19: warning: format specifies type 'short' but the argument has type 'char' [-Wformat]
    printf("%hd", 'a');
            ~~~   ^~~
            %hhd
2 warnings generated.

Ironically, we advise our users to change their source code, which leads to
another warning. I've added tests for this case, after this patch, only (%hd,
"char") will cause warnings. And clang will generate warning like this:

local/format.c:10:19: warning: format specifies type 'short' but the argument has type 'char' [-Wformat]
    printf("%hd", 'a');
            ~~~   ^~~
            %hhd
1 warning generated.

Use %hd with char is not reasonable. (Probabaly another misuse) So I kept
this warning as-is.

[inclyc]

Comments

[inclyc]

comments

[inclyc]

Fix CI issue

[aaron.ballman]

In D132568#3747598, @nickdesaulniers wrote:

In D132568#3746551, @aaron.ballman wrote:

Thank you for the patch, but it'd be really helpful to me as a reviewer if you and @nickdesaulniers could coordinate so there's only one patch trying to address #57102 instead of two competing patches (I'm happy to review whichever patch comes out). From a quick look over the test case changes, this patch looks like it's more in line with how I'd expect to resolve that issue compared to D132266.

The addition to clang/test/Sema/format-strings.c looks like it will resolve Linus' complaints in https://github.com/llvm/llvm-project/issues/57102; I'm happy to abandon D132266 in preference of this.

Thanks, Nick!

I asked some questions about wchar_t behavior in the thread, but I sort of wonder if we should handle those concerns separately (if changes are needed) as it looks more like missing functionality than promotion-related behavior. We have other changes we'll need in this area anyway for C23 because of https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2630.pdf, https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2680.pdf, and https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2763.pdf so there's no need to cram it all into one patch.

clang/include/clang/AST/FormatString.h

263–264

clang/lib/AST/FormatString.cpp

368

Isn't this a match promotion as well? e.g. printf("%hhd", (_Bool)0); where the _Bool is promoted to int but then cast back down to a char type (which seems rather unlikely to be a type confusion issue).

380

In C, wchar_t is an integer type and that underlying type might be int or it might be promotable to int. Should we handle it here as a type confused promotion match?

404

It's a bit strange that we have two switches over the same BT->getKind() and the only difference is !Ptr; would it be easier to read if we combined the two switches into one and had logic in the individual cases for Ptr vs not Ptr?

411

wchar_t?

416–417

I agree that printf("%hhd", (short)0); is potentially type confused, but why printf("%d", (short)0);?

FWIW, I think that printf("%lc", (short)0)); is potentially type confused as well.

425

The comment is talking about passing a char (promoted to int) then printed as a short but the check is looking for the type to be printed as an int; that doesn't seem like a type confusion situation so much as a match due to promotion. Should the test below be looking for a short type instead of an int type?

432

Why is this automatically type confusion regardless of the type specified by the format string?

clang/lib/Sema/SemaChecking.cpp

10114

Do we need a similar special case for L'a' in C, which has type wchar_t (which is a typedef to an integer type)?

clang/test/FixIt/format.m

40 ↗

(On Diff #455485)

It looks like some whitespace only changes snuck in.

173–176 ↗

(On Diff #455485)

Should we leave the test but remove the expected warnings and fix-it comments instead?

195–208 ↗

(On Diff #455485)

Hmmm, did we intend to change the behavior here? %C is an extension, so it's not clear to me if we wanted to modify this behavior or not. CC @rjmccall and @dexonsmith for some ObjC opinions.

clang/test/FixIt/format.mm

14–25 ↗

(On Diff #455485)

Same questions here as above.

clang/test/Sema/format-strings-scanf.c

15

Looks like more whitespace-only changes snuck in.

265–266

WG14 hasn't made an official determination, but one test we should add is:

// FIXME: does this match what the C committee allows or should it be pedantically warned on?
char c;
void *vp;
scanf("%hhd", &c); // Pedantic warning?
scanf("%hhd", vp); // Pedantic warning?

I asked on the reflectors whether folks wanted me to file an NB comment about this and the answers have been mixed so far. I'll most likely still file an NB comment to get the committee officially on record though.

271–272

@nickdesaulniers -- are you thinking this might be a pedantic warning because it's always valid to cast to a pointer to char and write into it? e.g.,

short s;
char *cp = (char *)&s;
*cp = 0; // Not UB

289–290

Is what a clang bug?

Also, what is with the "or no effect" in that wording? "This could cause major issues or nothing bad at all might happen" is a very odd way to word a diagnostic. :-D (Maybe something to look into improving in a later patch.)

[inclyc]

clang/test/FixIt/format.m

40 ↗

(On Diff #455485)

It looks like some whitespace only changes snuck in.

I'm sorry for this, do I need to discard these whitespace-only changes? Since this patch changes this file, is it my responsibility to keep other parts that don't belong to this patch unchanged, or am I correcting them by the way?

clang/test/Sema/format-strings-scanf.c

289–290

Is what a clang bug?

Look at sc which is a signed char, but scanf need a pointer float *, I add this comment because I think there should be a warning like

format specifies type 'float *' but the argument has type 'signed short *'

See printf tests below

[aaron.ballman]

clang/test/FixIt/format.m

40 ↗

(On Diff #455485)

I'm sorry for this, do I need to discard these whitespace-only changes? Since this patch changes this file, is it my responsibility to keep other parts that don't belong to this patch unchanged, or am I correcting them by the way?

You should discard the whitespace only changes; you are welcome to land an NFC commit that fixes just the whitespace issues though!

We typically ask that changes unrelated to the patch summary should be separated out so that each patch is self-contained for just one thing. Not only is that easier to review, it also makes it less likely that we end up reverting good changes if we need to revert a patch.

clang/test/Sema/format-strings-scanf.c

289–290

Oh I see what you're saying! There are two issues with the code: one is that the format specifier itself is UB and the other is that the argument passed to this confused format specifier does not agree.

To me, the priority is the invalid format specifier; unless the user corrects that, we're not really certain what they were aiming to scan in. And I think it's *probably* more likely that the user made a typo in the format specifier instead of trying to scan a half float (or whatever they thought they were scanning) into a signed char, so it seems defensible to silence the mismatched specifier diagnostic.

WDYT?

[inclyc]

clang/lib/AST/FormatString.cpp

425

The comment is talking about passing a char (promoted to int) then printed as a short but the check is looking for the type to be printed as an int; that doesn't seem like a type confusion situation so much as a match due to promotion. Should the test below be looking for a short type instead of an int type?

Sorry for this (sometimes shit happens). There is a lot of redundant logic in this place, I will upload the corrected code immediately.

clang/test/Sema/format-strings-scanf.c

289–290

printf("%hf", // expected-warning{{length modifier 'h' results in undefined behavior or no effect with 'f' conversion specifier}}
  sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}

Then clang seems to have two different behaviors to printf and scanf, and for printf it will give the kind of diagnosis I said. I don't think this problem is particularly serious, because the key point is that using %hf UB. So maybe we can just keep clang as-is? Or we should consider implement the same warning in scanf ?

[inclyc]

clang/lib/AST/FormatString.cpp

368

Hmm? _Bool just matches AnyCharTy perfectly. MatchPromotion means the types having different length and they can be considered as "partially matched" because of promotions).

Isn't _Bool and AnyChar here just have the same length?

[aaron.ballman]

clang/lib/AST/FormatString.cpp

425

No worries!

clang/test/Sema/format-strings-scanf.c

289–290

So maybe we can just keep clang as-is?

I think let's keep clang as-is for the moment so we can keep the concerns separate.

[aaron.ballman]

clang/lib/AST/FormatString.cpp

368

Hmm? _Bool just matches AnyCharTy perfectly. MatchPromotion means the types having different length and they can be considered as "partially matched" because of promotions).

Isn't _Bool and AnyChar here just have the same length?

_Bool is not a character type, so it doesn't match AnyCharTy perfectly. The size can be the same but they're still different types.

[inclyc]

Address comments

[nickdesaulniers]

In D132568#3747971, @inclyc wrote:

Do we want to encode that in test_promotion in clang/test/Sema/format-strings.c? Seems like tests on shorts are missing.

Tests for short and char "incompatibility" could be found elsewhere in this file.

format-strings.c

void should_understand_small_integers(void) {
  printf("%hhu", (short) 10); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'short'}}
  printf("%hu\n", (unsigned char)1); // warning with -Wformat-pedantic only
  printf("%hu\n", (uint8_t)1);       // warning with -Wformat-pedantic only
}
/* ... */
void test13(short x) {
  char bel = 007;
  printf("bel: '0%hhd'\n", bel); // no-warning
  printf("x: '0%hhd'\n", x); // expected-warning {{format specifies type 'char' but the argument has type 'short'}}
}

Do I need to explicitly test again in the test_promotion?

Feel free to delete or move existing test cases if they make more sense to remain together in your added block, rather than more isolated in other test functions.

clang/lib/AST/FormatString.cpp

404

I almost made the same recommendation myself. For the below switch pair, and the pair above.

411–413

make sure to mark these done in phabricator UI

clang/test/Sema/format-strings-scanf.c

271–272

No, was just curious about the comment Is this a clang bug ?. Looks like my comment has moved from the source location, and that you have a similar thread below. This one can be marked done.

[inclyc]

clang/lib/AST/FormatString.cpp

404

It's a bit strange that we have two switches over the same BT->getKind() and the only difference is !Ptr; would it be easier to read if we combined the two switches into one and had logic in the individual cases for Ptr vs not Ptr?

These two switch pairs have different functions. The lower one is only responsible for checking whether there is a signed or unsigned integer, and the upper one is checking whether there is a promotion (or type confusing). Will they be more difficult to understand if they are written together?

[inclyc]

Remove wchar tests.

These tests may need to be re-added after we have implemented wchar_t checks in C

[nickdesaulniers]

clang/lib/AST/FormatString.cpp

404

Perhaps. I think the comments you added to all switches are helpful!

[nickdesaulniers]

clang/lib/Sema/SemaChecking.cpp

10131–10133

There's something about this conditional...I can't quite put my finger on.

Isn't ImplicitMatch only updated in the const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E) branch above, where IsCharacterLiteralInt cannot be?

Simultaneously, isn't IsCharacterLiteralInt only updated in the const CharacterLiteral *CL = dyn_cast<CharacterLiteral>(E) branch above, where ImplicitMatch cannot be?

I wonder if we should instead hoist:

if (Match == ArgType::MatchPromotion) {
  if (ImplicitMatch != ArgType::NoMatchPromotionTypeConfusion &&
      ImplicitMatch != ArgType::NoMatchTypeConfusion)
    return true;
  Match = ArgType::NoMatch;
}

into the const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E) branch after ImplicitMatch is updated.

Then hoist

if (Match == ArgType::MatchPromotion)
  return true;

into the const CharacterLiteral *CL = dyn_cast<CharacterLiteral>(E) branch after IsCharacterLiteralInt is updated? Then we could delete the IsCharacterLiteralInt variable perhaps?

[nickdesaulniers]

clang/lib/Sema/SemaChecking.cpp

10137–10139

Similarly, if ImplicitMatch can only be updated to one of these two values within the above const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E) branch, does it make sense to additionally hoist this conditional check into that branch as well?

10137–10139

If we can do both suggestions, then ImplicitMatch can remain scoped to that branch, as it previously was.

[inclyc]

Address comments

[inclyc]

clang/lib/Sema/SemaChecking.cpp

10131–10133

There's something about this conditional...I can't quite put my finger on.

Isn't ImplicitMatch only updated in the const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E) branch above, where IsCharacterLiteralInt cannot be?

Simultaneously, isn't IsCharacterLiteralInt only updated in the const CharacterLiteral *CL = dyn_cast<CharacterLiteral>(E) branch above, where ImplicitMatch cannot be?

I wonder if we should instead hoist:

if (Match == ArgType::MatchPromotion) {
  if (ImplicitMatch != ArgType::NoMatchPromotionTypeConfusion &&
      ImplicitMatch != ArgType::NoMatchTypeConfusion)
    return true;
  Match = ArgType::NoMatch;
}

into the const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E) branch after ImplicitMatch is updated.

The control flow may not entering any of these branches at all, and if we have Match == MatchPromotion we depend on the origin value of ImplicitMatch to determine whether or not to discard the MatchPromotion property. For example,

printf("%hd", 0);

We have MatchPromotion because the argument is an int and the format string specifies signed short, and we will not entering ImplicitCastExpr branch because the argument is an int already (so we don't need an "implicit cast")

Then hoist

if (Match == ArgType::MatchPromotion)
  return true;

into the const CharacterLiteral *CL = dyn_cast<CharacterLiteral>(E) branch after IsCharacterLiteralInt is updated? Then we could delete the IsCharacterLiteralInt variable perhaps?

Thank you for this! I've hoisted this into CharacterLiteralExpr branch. And now IsCharacterLiteralInt is unnecessary.

[dexonsmith]

clang/test/FixIt/format.m

195–208 ↗

(On Diff #455485)

I’m not sure, but I agree it’s suspicious. Besides John, Mike Ash might be another person to check with (@-completion isn’t working for me on my phone right now, but perhaps someone else will have more luck?). Or, @arphaman, any other suggestions?

[inclyc]

Trim whitespace in format.mm

[inclyc]

Add LangOpts so we keep the Obj-C behavior of clang just as-is.

[inclyc]

clang/lib/AST/FormatString.cpp

368

I've add case BuiltinType::Bool: back to preserve https://reviews.llvm.org/D66856

[inclyc]

Update comments

[inclyc]

Delete extra newline.

Currently this patch has not fully implemented wchar_t related support, this
type seems to be even platform dependent in C language, if possible, maybe we
can consider support in subsequent patches?

[nickdesaulniers]

@aaron.ballman should review+accept this before you land it, but I'm satisfied with the result. Thank you @inclyc for working on this!

[aaron.ballman]

Basically LGTM as well, just some nits about comments and a small fix to the c status page.

In D132568#3753512, @inclyc wrote:

Currently this patch has not fully implemented wchar_t related support, this
type seems to be even platform dependent in C language, if possible, maybe we
can consider support in subsequent patches?

I think that's reasonable.

clang/lib/AST/FormatString.cpp

360

371

404

Yeah, let's leave the structure be for now, we can always clarify it further in an NFC follow-up if it turns out to be a reasonable suggestion.

404

452

clang/lib/Sema/SemaChecking.cpp

10123–10125

10131

We should probably have a comment here about why ObjC is special..

clang/www/c_status.html

822

[efriedma]

clang/docs/ReleaseNotes.rst

141

It seems a little weird to say we "implemented" this. The standard doesn't require any warnings for misuse of printf etc., and clang doesn't actually implement printf, so we don't need to do anything to be standard-compliant. I'd just say that we adjusted -Wformat warnings to avoid false positives.

clang/lib/Sema/SemaChecking.cpp

10124

*likely

[inclyc]

Update comments according to review feedback

clang/docs/ReleaseNotes.rst

 ---------------
 
 AIX Support
 -----------
 
 C Language Changes in Clang
 ---------------------------
 
+- Implemented `WG14 N2562 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf>`_.
+  Clang will now consider default argument promotions in printf, and remove unnecessary warnings.
+  Especially ``int`` argument with specifier ``%hhd`` and ``%hd``.
+
 C2x Feature Support
 -------------------
 
 C++ Language Changes in Clang
 -----------------------------
 
 - Implemented DR692, DR1395 and DR1432. Use the ``-fclang-abi-compat=14`` option
   to get the old partial ordering behavior regarding packs.

clang/include/clang/AST/FormatString.h

public:
   enum Kind { UnknownTy, InvalidTy, SpecificTy, ObjCPointerTy, CPointerTy,
               AnyCharTy, CStrTy, WCStrTy, WIntTy };
 
   /// How well a given conversion specifier matches its argument.
   enum MatchKind {
     /// The conversion specifier and the argument types are incompatible. For
     /// instance, "%d" and float.
     NoMatch = 0,
     /// The conversion specifier and the argument type are compatible. For
-    /// instance, "%d" and _Bool.
+    /// instance, "%d" and int.
     Match = 1,
+    /// The conversion specifier and the argument type are compatible because of
+    /// default argument promotions. For instance, "%hhd" and int.
+    MatchPromotion,
+    /// The conversion specifier and the argument type are compatible but still
+    /// seems likely to be an error. For instanace, "%hhd" and short.
+    NoMatchPromotionTypeConfusion,
     /// The conversion specifier and the argument type are disallowed by the C
     /// standard, but are in practice harmless. For instance, "%p" and int*.
     NoMatchPedantic,
     /// The conversion specifier and the argument type are compatible, but still
     /// seems likely to be an error. For instance, "%hd" and _Bool.
     NoMatchTypeConfusion,
   };
 

clang/lib/AST/FormatString.cpp

ArgType::matchesType(ASTContext &C, QualType argTy) const {
   switch (K) {
     case InvalidTy:
       llvm_unreachable("ArgType must be valid");
 
     case UnknownTy:
       return Match;
 
     case AnyCharTy: {
-      if (const EnumType *ETy = argTy->getAs<EnumType>()) {
+      if (const auto *ETy = argTy->getAs<EnumType>()) {
         // If the enum is incomplete we know nothing about the underlying type.
         // Assume that it's 'int'.
         if (!ETy->getDecl()->isComplete())
           return NoMatch;
         argTy = ETy->getDecl()->getIntegerType();
       }
 
-      if (const BuiltinType *BT = argTy->getAs<BuiltinType>())
+      if (const auto *BT = argTy->getAs<BuiltinType>()) {
+        // the types are perfectly matched?
         switch (BT->getKind()) {
-          default:
-            break;
-          case BuiltinType::Char_S:
-          case BuiltinType::SChar:
-          case BuiltinType::UChar:
-          case BuiltinType::Char_U:
-          case BuiltinType::Bool:
-            return Match;
         }
+        // "partially matched" because of promotions?
+        if (!Ptr) {
+          switch (BT->getKind()) {
+          default:
+            break;
+          case BuiltinType::Int:
+          case BuiltinType::UInt:
+            return MatchPromotion;
+          case BuiltinType::Short:
+          case BuiltinType::UShort:
+          case BuiltinType::WChar_S:
+          case BuiltinType::WChar_U:
+            return NoMatchPromotionTypeConfusion;
+          }
+        }
+      }
       return NoMatch;
     }
 
     case SpecificTy: {
       if (const EnumType *ETy = argTy->getAs<EnumType>()) {
         // If the enum is incomplete we know nothing about the underlying type.
         // Assume that it's 'int'.
         if (!ETy->getDecl()->isComplete())
           argTy = C.IntTy;
         else
           argTy = ETy->getDecl()->getIntegerType();
       }
       argTy = C.getCanonicalType(argTy).getUnqualifiedType();
 
       if (T == argTy)
         return Match;
-      // Check for "compatible types".
-      if (const BuiltinType *BT = argTy->getAs<BuiltinType>())
+      if (const auto *BT = argTy->getAs<BuiltinType>()) {
+        // check if the only difference between them is signed vs unsigned
+        // if true, we consider they are compatible.
         switch (BT->getKind()) {
           default:
             break;
           case BuiltinType::Char_S:
           case BuiltinType::SChar:
           case BuiltinType::Char_U:
           case BuiltinType::UChar:
           case BuiltinType::Bool:
             if (T == C.UnsignedShortTy || T == C.ShortTy)
               return NoMatchTypeConfusion;
-            return T == C.UnsignedCharTy || T == C.SignedCharTy ? Match
-                                                                : NoMatch;
+            if (T == C.UnsignedCharTy || T == C.SignedCharTy)
+              return Match;
+            break;
           case BuiltinType::Short:
-            return T == C.UnsignedShortTy ? Match : NoMatch;
+            if (T == C.UnsignedShortTy)
+              return Match;
+            break;
           case BuiltinType::UShort:
-            return T == C.ShortTy ? Match : NoMatch;
+            if (T == C.ShortTy)
+              return Match;
+            break;
           case BuiltinType::Int:
-            return T == C.UnsignedIntTy ? Match : NoMatch;
+            if (T == C.UnsignedIntTy)
+              return Match;
+            break;
           case BuiltinType::UInt:
-            return T == C.IntTy ? Match : NoMatch;
+            if (T == C.IntTy)
+              return Match;
+            break;
           case BuiltinType::Long:
-            return T == C.UnsignedLongTy ? Match : NoMatch;
+            if (T == C.UnsignedLongTy)
+              return Match;
+            break;
           case BuiltinType::ULong:
-            return T == C.LongTy ? Match : NoMatch;
+            if (T == C.LongTy)
+              return Match;
+            break;
           case BuiltinType::LongLong:
-            return T == C.UnsignedLongLongTy ? Match : NoMatch;
+            if (T == C.UnsignedLongLongTy)
+              return Match;
+            break;
           case BuiltinType::ULongLong:
-            return T == C.LongLongTy ? Match : NoMatch;
-        }
+            if (T == C.LongLongTy)
+              return Match;
+            break;
+          }
+          // "partially matched" because of promotions?
+          if (!Ptr) {
+            switch (BT->getKind()) {
+            default:
+              break;
+            case BuiltinType::Int:
+            case BuiltinType::UInt:
+              if (T == C.SignedCharTy || T == C.UnsignedCharTy ||
+                  T == C.ShortTy || T == C.UnsignedShortTy || T == C.WCharTy ||
+                  T == C.WideCharTy)
+                return MatchPromotion;
+              break;
+            case BuiltinType::Short:
+            case BuiltinType::UShort:
+              if (T == C.SignedCharTy || T == C.UnsignedCharTy)
+                return NoMatchPromotionTypeConfusion;
+              break;
+            case BuiltinType::WChar_U:
+            case BuiltinType::WChar_S:
+              if (T != C.WCharTy && T != C.WideCharTy)
+                return NoMatchPromotionTypeConfusion;
+            }
+          }
       return NoMatch;
     }
 
     case CStrTy: {
       const PointerType *PT = argTy->getAs<PointerType>();
       if (!PT)
         return NoMatch;
       QualType pointeeTy = PT->getPointeeType();

clang/lib/Sema/SemaChecking.cpp

if (FS.getConversionSpecifier().getKind() == ConversionSpecifier::cArg &&
     llvm::raw_svector_ostream os(FSString);
     FS.toString(os);
     EmitFormatDiagnostic(S.PDiag(diag::warn_format_bool_as_character)
                              << FSString,
                          E->getExprLoc(), false, CSR);
     return true;
   }
 
-  analyze_printf::ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy);
-  if (Match == analyze_printf::ArgType::Match)
+  ArgType::MatchKind ImplicitMatch = ArgType::NoMatch;
+  ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy);
+  if (Match == ArgType::Match)
     return true;
 
+  // NoMatchPromotionTypeConfusion should be only returned in ImplictCastExpr
+  assert(Match != ArgType::NoMatchPromotionTypeConfusion);
+
   // Look through argument promotions for our error message's reported type.
   // This includes the integral and floating promotions, but excludes array
   // and function pointer decay (seeing that an argument intended to be a
   // string has type 'char [6]' is probably more confusing than 'char *') and
   // certain bitfield promotions (bitfields can be 'demoted' to a lesser type).
   if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
     if (isArithmeticArgumentPromotion(S, ICE)) {
       E = ICE->getSubExpr();
       ExprTy = E->getType();
 
       // Check if we didn't match because of an implicit cast from a 'char'
       // or 'short' to an 'int'.  This is done because printf is a varargs
       // function.
       if (ICE->getType() == S.Context.IntTy ||
           ICE->getType() == S.Context.UnsignedIntTy) {
         // All further checking is done on the subexpression
-        const analyze_printf::ArgType::MatchKind ImplicitMatch =
-            AT.matchesType(S.Context, ExprTy);
-        if (ImplicitMatch == analyze_printf::ArgType::Match)
+        ImplicitMatch = AT.matchesType(S.Context, ExprTy);
+        if (ImplicitMatch == ArgType::Match)
           return true;
-        if (ImplicitMatch == ArgType::NoMatchPedantic ||
-            ImplicitMatch == ArgType::NoMatchTypeConfusion)
-          Match = ImplicitMatch;
       }
     }
   } else if (const CharacterLiteral *CL = dyn_cast<CharacterLiteral>(E)) {
     // Special case for 'a', which has type 'int' in C.
     // Note, however, that we do /not/ want to treat multibyte constants like
     // 'MooV' as characters! This form is deprecated but still exists. In
     // addition, don't treat expressions as of type 'char' if one byte length
     // modifier is provided.
     if (ExprTy == S.Context.IntTy &&
         FS.getLengthModifier().getKind() != LengthModifier::AsChar)
-      if (llvm::isUIntN(S.Context.getCharWidth(), CL->getValue()))
+      if (llvm::isUIntN(S.Context.getCharWidth(), CL->getValue())) {
         ExprTy = S.Context.CharTy;
+        // Consider character literal is a 'char' in C
+        // printf("%hd", 'a'); is more likey a type confusion situation
+        // We will suggest our users to use %hhd by discarding MatchPromotion
+        if (Match == ArgType::MatchPromotion)
+          Match = ArgType::NoMatch;
       }
-
+  }
+  if (Match == ArgType::MatchPromotion) {
+    if (!S.getLangOpts().ObjC &&
+        ImplicitMatch != ArgType::NoMatchPromotionTypeConfusion &&
+        ImplicitMatch != ArgType::NoMatchTypeConfusion)
+      return true;
+    Match = ArgType::NoMatch;
+  }
+  if (ImplicitMatch == ArgType::NoMatchPedantic ||
+      ImplicitMatch == ArgType::NoMatchTypeConfusion)
+    Match = ImplicitMatch;
+  assert(Match != ArgType::MatchPromotion);
   // Look through enums to their underlying type.
   bool IsEnum = false;
   if (auto EnumTy = ExprTy->getAs<EnumType>()) {
     ExprTy = EnumTy->getDecl()->getIntegerType();
     IsEnum = true;
   }
 
   // %C in an Objective-C context prints a unichar, not a wchar_t.
if (Success) {
     llvm::raw_svector_ostream os(buf);
     fixedFS.toString(os);
 
     CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen);
 
     if (IntendedTy == ExprTy && !ShouldNotPrintDirectly) {
       unsigned Diag;
       switch (Match) {
-      case ArgType::Match: llvm_unreachable("expected non-matching");
+      case ArgType::Match:
+      case ArgType::MatchPromotion:
+      case ArgType::NoMatchPromotionTypeConfusion:
+        llvm_unreachable("expected non-matching");
       case ArgType::NoMatchPedantic:
         Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
         break;
       case ArgType::NoMatchTypeConfusion:
         Diag = diag::warn_format_conversion_argument_type_mismatch_confusion;
         break;
       case ArgType::NoMatch:
         Diag = diag::warn_format_conversion_argument_type_mismatch;
if (Success) {
     // was deferred until now, we emit a warning for non-POD
     // arguments here.
     bool EmitTypeMismatch = false;
     switch (S.isValidVarArgType(ExprTy)) {
     case Sema::VAK_Valid:
     case Sema::VAK_ValidInCXX11: {
       unsigned Diag;
       switch (Match) {
-      case ArgType::Match: llvm_unreachable("expected non-matching");
+      case ArgType::Match:
+      case ArgType::MatchPromotion:
+      case ArgType::NoMatchPromotionTypeConfusion:
+        llvm_unreachable("expected non-matching");
       case ArgType::NoMatchPedantic:
         Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
         break;
       case ArgType::NoMatchTypeConfusion:
         Diag = diag::warn_format_conversion_argument_type_mismatch_confusion;
         break;
       case ArgType::NoMatch:
         Diag = diag::warn_format_conversion_argument_type_mismatch;

clang/test/Sema/format-strings-freebsd.c

void check_freebsd_kernel_extensions(int i, long l, char *s, short h)
 
   // %r expects an int
   freebsd_kernel_printf("%r", i); // no-warning
   freebsd_kernel_printf("%r", l); // expected-warning{{format specifies type 'int' but the argument has type 'long'}}
   freebsd_kernel_printf("%lr", i); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
   freebsd_kernel_printf("%lr", l); // no-warning
 
   // h modifier expects a short
-  freebsd_kernel_printf("%hr", i); // expected-warning{{format specifies type 'short' but the argument has type 'int'}}
+  freebsd_kernel_printf("%hr", i); // no-warning
   freebsd_kernel_printf("%hr", h); // no-warning
-  freebsd_kernel_printf("%hy", i); // expected-warning{{format specifies type 'short' but the argument has type 'int'}}
+  freebsd_kernel_printf("%hy", i); // no-warning
   freebsd_kernel_printf("%hy", h); // no-warning
 
   // %y expects an int
   freebsd_kernel_printf("%y", i); // no-warning
   freebsd_kernel_printf("%y", l); // expected-warning{{format specifies type 'int' but the argument has type 'long'}}
   freebsd_kernel_printf("%ly", i); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
   freebsd_kernel_printf("%ly", l); // no-warning
 }

clang/test/Sema/format-strings-scanf.c

 // RUN: %clang_cc1 -std=c11 -fsyntax-only -verify -Wformat-nonliteral %s
 
 // Test that -Wformat=0 works:
 // RUN: %clang_cc1 -std=c11 -fsyntax-only -Werror -Wformat=0 %s
 
 #include <stdarg.h>
 typedef __SIZE_TYPE__ size_t;
 #define __SSIZE_TYPE__                                                         \
   __typeof__(_Generic((__SIZE_TYPE__)0,                                        \
                       unsigned long long int : (long long int)0,               \
                       unsigned long int : (long int)0,                         \
                       unsigned int : (int)0,                                   \
                       unsigned short : (short)0,                               \
                       unsigned char : (signed char)0))
 typedef __SSIZE_TYPE__ ssize_t;                         
 
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 #define __UNSIGNED_PTRDIFF_TYPE__                                              \
   __typeof__(_Generic((__PTRDIFF_TYPE__)0,                                     \
                       long long int : (unsigned long long int)0,               \
                       long int : (unsigned long int)0,                         \
                       int : (unsigned int)0,                                   \
                       short : (unsigned short)0,                               \
void check_conditional_literal(char *s, int *i) {
   scanf(0 ? "%d %d" : "%d", i); // no warning
   scanf(1 ? "%d %d" : "%d", i); // expected-warning{{more '%' conversions than data arguments}}
   scanf(0 ? "%d %d" : "%d", i, s); // expected-warning{{data argument not used}}
   scanf(1 ? "%d %s" : "%d", i, s); // no warning
   scanf(i ? "%d %s" : "%d", i, s); // no warning
   scanf(i ? "%d" : "%d", i, s); // expected-warning{{data argument not used}}
   scanf(i ? "%s" : "%d", s); // expected-warning{{format specifies type 'int *'}}
 }
+
+void test_promotion(void) {
+  // No promotions for *scanf pointers clarified in N2562
+  // https://github.com/llvm/llvm-project/issues/57102
+  // N2562: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
+  int i;
+  signed char sc;
+  unsigned char uc;
+  short ss;
+  unsigned short us;
+
+  // pointers could not be "promoted"
+  scanf("%hhd", &i); // expected-warning{{format specifies type 'char *' but the argument has type 'int *'}}
+  scanf("%hd", &i); // expected-warning{{format specifies type 'short *' but the argument has type 'int *'}}
+  scanf("%d", &i); // no-warning
+  // char & uchar
+  scanf("%hhd", &sc); // no-warning
+  scanf("%hhd", &uc); // no-warning
+  scanf("%hd", &sc); // expected-warning{{format specifies type 'short *' but the argument has type 'signed char *'}}
+  scanf("%hd", &uc); // expected-warning{{format specifies type 'short *' but the argument has type 'unsigned char *'}}
+  scanf("%d", &sc); // expected-warning{{format specifies type 'int *' but the argument has type 'signed char *'}}
+  scanf("%d", &uc); // expected-warning{{format specifies type 'int *' but the argument has type 'unsigned char *'}}
+  // short & ushort
+  scanf("%hhd", &ss); // expected-warning{{format specifies type 'char *' but the argument has type 'short *'}}
+  scanf("%hhd", &us); // expected-warning{{format specifies type 'char *' but the argument has type 'unsigned short *'}}
+  scanf("%hd", &ss); // no-warning
+  scanf("%hd", &us); // no-warning
+  scanf("%d", &ss); // expected-warning{{format specifies type 'int *' but the argument has type 'short *'}}
+  scanf("%d", &us); // expected-warning{{format specifies type 'int *' but the argument has type 'unsigned short *'}}
+
+  // long types
+  scanf("%ld", &i); // expected-warning{{format specifies type 'long *' but the argument has type 'int *'}}
+  scanf("%lld", &i); // expected-warning{{format specifies type 'long long *' but the argument has type 'int *'}}
+  scanf("%ld", &sc); // expected-warning{{format specifies type 'long *' but the argument has type 'signed char *'}}
+  scanf("%lld", &sc); // expected-warning{{format specifies type 'long long *' but the argument has type 'signed char *'}}
+  scanf("%ld", &uc); // expected-warning{{format specifies type 'long *' but the argument has type 'unsigned char *'}}
+  scanf("%lld", &uc); // expected-warning{{format specifies type 'long long *' but the argument has type 'unsigned char *'}}
+  scanf("%llx", &i); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'int *'}}
+
+  // ill-formed floats
+  scanf("%hf", // expected-warning{{length modifier 'h' results in undefined behavior or no effect with 'f' conversion specifier}}
+  &sc);
+
+  // pointers in scanf
+  scanf("%s", i); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+
+  // FIXME: does this match what the C committee allows or should it be pedantically warned on?
+  char c;
+  void *vp;
+  scanf("%hhd", &c); // Pedantic warning?
+  scanf("%hhd", vp); // expected-warning{{format specifies type 'char *' but the argument has type 'void *'}}
+}

clang/test/Sema/format-strings.c

void __attribute__((__format__(__printf__, 2, 3))) (^printf_arg2)(
     vprintf(fmt, ap);
     vprintf(not_fmt, ap); // expected-warning{{format string is not a string literal}}
     va_end(ap);
   };
 
   printf_arg2("foo", "%s string %i\n", "aaa", 123);
   printf_arg2("%s string\n", "foo", "bar"); // expected-warning{{data argument not used by format string}}
 }
+
+void test_promotion(void) {
+  // Default argument promotions for *printf in N2562
+  // https://github.com/llvm/llvm-project/issues/57102
+  // N2562: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
+  int i;
+  signed char sc;
+  unsigned char uc;
+  char c;
+  short ss;
+  unsigned short us;
+
+  printf("%hhd %hd %d %hhd %hd %d", i, i, i, sc, sc, sc); // no-warning
+  printf("%hhd %hd %d %hhd %hd %d", uc, uc, uc, c, c, c); // no-warning
+
+  // %ld %lld %llx
+  printf("%ld", i); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
+  printf("%lld", i); // expected-warning{{format specifies type 'long long' but the argument has type 'int'}}
+  printf("%ld", sc); // expected-warning{{format specifies type 'long' but the argument has type 'signed char'}}
+  printf("%lld", sc); // expected-warning{{format specifies type 'long long' but the argument has type 'signed char'}}
+  printf("%ld", uc); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned char'}}
+  printf("%lld", uc); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned char'}}
+  printf("%llx", i); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'int'}}
+
+  // ill formed spec for floats
+  printf("%hf", // expected-warning{{length modifier 'h' results in undefined behavior or no effect with 'f' conversion specifier}}
+  sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}
+
+  // for %hhd and `short` they are compatible by promotions but more likely misuse
+  printf("%hd", ss); // no-warning
+  printf("%hhd", ss); // expected-warning{{format specifies type 'char' but the argument has type 'short'}}
+  printf("%hu", us); // no-warning
+  printf("%hhu", ss); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'short'}}
+
+  // floats & integers are not compatible
+  printf("%f", i); // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
+  printf("%f", sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}
+  printf("%f", uc); // expected-warning{{format specifies type 'double' but the argument has type 'unsigned char'}}
+  printf("%f", c); // expected-warning{{format specifies type 'double' but the argument has type 'char'}}
+  printf("%f", ss); // expected-warning{{format specifies type 'double' but the argument has type 'short'}}
+  printf("%f", us); // expected-warning{{format specifies type 'double' but the argument has type 'unsigned short'}}
+
+  // character literals
+  // In C language engineering practice, printing a character literal with %hhd or %d is common, but %hd may be misuse.
+  printf("%hhu", 'a'); // no-warning
+  printf("%hhd", 'a'); // no-warning
+  printf("%hd", 'a'); // expected-warning{{format specifies type 'short' but the argument has type 'char'}}
+  printf("%hu", 'a'); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'char'}}
+  printf("%d", 'a'); // no-warning
+  printf("%u", 'a'); // no-warning
+  
+  // pointers
+  printf("%s", i); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+}

clang/www/c_status.html

</tr>
     <tr>
       <td>Compatibility of Pointers to Arrays with Qualifiers</td>
       <td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2607.pdf">N2607</a></td>
       <td class="partial" align="center">Partial</td>
     </tr>
     <tr>
       <td>Unclear type relationship between a format specifier and its argument</td>
       <td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf">N2562</a></td>
-      <td class="partial" align="center">
-        <details><summary>Partial</summary>
-          Clang supports diagnostics checking format specifier validity, but
-          does not yet account for all of the changes in this paper, especially
-          regarding length modifiers like <code>h</code> and <code>hh</code>.
-        </details>
-      </td>
+      <td class="full" align="center">Clang 16</td>
     </tr>
     <!-- Apr 2021 Papers -->
     <tr>
       <td>String functions for freestanding implementations</td>
       <td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2524.htm">N2524</a></td>
       <td class="none" align="center">No</td>
     </tr>
     <tr>