Fix Clang bug that -Wformat-signedness is not reported properly.

clang/docs/ReleaseNotes.rst

@@ -155,6 +155,17 @@ Improvements to Clang's diagnostics
 - Fixed fix-it hint for fold expressions. Clang now correctly places the suggested right
   parenthesis when diagnosing malformed fold expressions. (#GH151787)
 
+- Fixed an issue where emitted format-signedness diagnostics were not associated with an appropriate
+  diagnostic id. Besides being incorrect from an API standpoint, this was user visible, e.g.:
+  "format specifies type 'unsigned int' but the argument has type 'int' [-Wformat]"
+  "signedness of format specifier 'u' is incompatible with 'c' [-Wformat]"
+  This was misleading, because even though -Wformat is required in order to emit the diagnostics,
+  the warning flag the user needs to concerned with here is -Wformat-signedness, which is also
+  required and is not enabled by default. With the change you'll now see:
+  "format specifies type 'unsigned int' but the argument has type 'int', which differs in signedness [-Wformat-signedness]"
+  "signedness of format specifier 'u' is incompatible with 'c' [-Wformat-signedness]"
+  and the API-visible diagnostic id will be appropriate.
+
 Improvements to Clang's time-trace
 ----------------------------------
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10423,9 +10423,10 @@ def warn_format_conversion_argument_type_mismatch : Warning<
 def warn_format_conversion_argument_type_mismatch_pedantic : Extension<
   warn_format_conversion_argument_type_mismatch.Summary>,
   InGroup<FormatPedantic>;
-def warn_format_conversion_argument_type_mismatch_signedness : Warning<
-  warn_format_conversion_argument_type_mismatch.Summary>,
-  InGroup<FormatSignedness>, DefaultIgnore;
+def warn_format_conversion_argument_type_mismatch_signedness: Warning<
+      "format specifies type %0 but the argument has %select{type|underlying "
+      "type}2 %1, which differs in signedness" >
+    , InGroup<FormatSignedness>, DefaultIgnore;
 def warn_format_conversion_argument_type_mismatch_confusion : Warning<
   warn_format_conversion_argument_type_mismatch.Summary>,
   InGroup<FormatTypeConfusion>, DefaultIgnore;
@@ -10537,8 +10538,10 @@ def warn_format_cmp_sensitivity_mismatch : Warning<
   "it should be %select{unspecified|private|public|sensitive}1">, InGroup<Format>;
 def warn_format_cmp_specifier_mismatch : Warning<
   "format specifier '%0' is incompatible with '%1'">, InGroup<Format>;
-def warn_format_cmp_specifier_sign_mismatch : Warning<
-  "signedness of format specifier '%0' is incompatible with '%1'">, InGroup<Format>;
+def warn_format_cmp_specifier_sign_mismatch
+    : Warning<"signedness of format specifier '%0' is incompatible with '%1'">,
+      InGroup<FormatSignedness>,
+      DefaultIgnore;
 def warn_format_cmp_specifier_mismatch_pedantic : Extension<
   warn_format_cmp_specifier_sign_mismatch.Summary>, InGroup<FormatPedantic>;
 def note_format_cmp_with : Note<

clang/lib/Sema/SemaChecking.cpp

@@ -41,6 +41,7 @@
 #include "clang/AST/UnresolvedSet.h"
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
@@ -7656,17 +7657,13 @@ bool EquatableFormatArgument::VerifyCompatible(
     break;
 
   case MK::NoMatchSignedness:
-    if (!S.getDiagnostics().isIgnored(
-            diag::warn_format_conversion_argument_type_mismatch_signedness,
-            ElementLoc)) {
-      EmitDiagnostic(S,
-                     S.PDiag(diag::warn_format_cmp_specifier_sign_mismatch)
-                         << buildFormatSpecifier()
-                         << Other.buildFormatSpecifier(),
-                     FmtExpr, InFunctionCall);
-      HadError = S.Diag(Other.ElementLoc, diag::note_format_cmp_with)
-                 << 0 << Other.Range;
-    }
+    EmitDiagnostic(S,
+                   S.PDiag(diag::warn_format_cmp_specifier_sign_mismatch)
+                       << buildFormatSpecifier()
+                       << Other.buildFormatSpecifier(),
+                   FmtExpr, InFunctionCall);
+    HadError = S.Diag(Other.ElementLoc, diag::note_format_cmp_with)
+               << 0 << Other.Range;
     break;
   }
   return !HadError;
@@ -8203,11 +8200,14 @@ static analyze_format_string::ArgType::MatchKind
 handleFormatSignedness(analyze_format_string::ArgType::MatchKind Match,
                        DiagnosticsEngine &Diags, SourceLocation Loc) {
   if (Match == analyze_format_string::ArgType::NoMatchSignedness) {
-    Match =
+    if (Diags.isIgnored(
+            diag::warn_format_conversion_argument_type_mismatch_signedness,
+            Loc) ||
         Diags.isIgnored(
-            diag::warn_format_conversion_argument_type_mismatch_signedness, Loc)
-            ? analyze_format_string::ArgType::Match
-            : analyze_format_string::ArgType::NoMatch;
+            // Arbitrary -Wformat diagnostic to detect -Wno-format:
+            diag::warn_format_conversion_argument_type_mismatch, Loc)) {
+      return analyze_format_string::ArgType::Match;
+    }
   }
   return Match;
 }
@@ -8424,8 +8424,10 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
       case ArgType::Match:
       case ArgType::MatchPromotion:
       case ArgType::NoMatchPromotionTypeConfusion:
-      case ArgType::NoMatchSignedness:
         llvm_unreachable("expected non-matching");
+      case ArgType::NoMatchSignedness:
+        Diag = diag::warn_format_conversion_argument_type_mismatch_signedness;
+        break;
       case ArgType::NoMatchPedantic:
         Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
         break;
@@ -8750,17 +8752,20 @@ bool CheckScanfHandler::HandleScanfSpecifier(
   analyze_format_string::ArgType::MatchKind Match =
       AT.matchesType(S.Context, Ex->getType());
   Match = handleFormatSignedness(Match, S.getDiagnostics(), Ex->getExprLoc());
-  bool Pedantic = Match == analyze_format_string::ArgType::NoMatchPedantic;
   if (Match == analyze_format_string::ArgType::Match)
     return true;
+  bool Pedantic = Match == analyze_format_string::ArgType::NoMatchPedantic;
+  bool Signedness = Match == analyze_format_string::ArgType::NoMatchSignedness;
 
   ScanfSpecifier fixedFS = FS;
   bool Success = fixedFS.fixType(Ex->getType(), Ex->IgnoreImpCasts()->getType(),
                                  S.getLangOpts(), S.Context);
 
   unsigned Diag =
       Pedantic ? diag::warn_format_conversion_argument_type_mismatch_pedantic
-               : diag::warn_format_conversion_argument_type_mismatch;
+      : Signedness
+          ? diag::warn_format_conversion_argument_type_mismatch_signedness
+          : diag::warn_format_conversion_argument_type_mismatch;
 
   if (Success) {
     // Get the fix string from the fixed format specifier.

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

@@ -39,41 +39,41 @@ void test_printf_unsigned_char(unsigned char x)
 void test_printf_int(int x)
 {
     printf("%d", x); // no-warning
-    printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'int'}}
-    printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'int'}}
+    printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'int', which differs in signedness}}
+    printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'int', which differs in signedness}}
 }
 
 void test_printf_unsigned(unsigned x)
 {
-    printf("%d", x); // expected-warning{{format specifies type 'int' but the argument has type 'unsigned int'}}
+    printf("%d", x); // expected-warning{{format specifies type 'int' but the argument has type 'unsigned int', which differs in signedness}}
     printf("%u", x); // no-warning
     printf("%x", x); // no-warning
 }
 
 void test_printf_long(long x)
 {
     printf("%ld", x); // no-warning
-    printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has type 'long'}}
-    printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has type 'long'}}
+    printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has type 'long', which differs in signedness}}
+    printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has type 'long', which differs in signedness}}
 }
 
 void test_printf_unsigned_long(unsigned long x)
 {
-    printf("%ld", x); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned long'}}
+    printf("%ld", x); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned long', which differs in signedness}}
     printf("%lu", x); // no-warning
     printf("%lx", x); // no-warning
 }
 
 void test_printf_long_long(long long x)
 {
     printf("%lld", x); // no-warning
-    printf("%llu", x); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'long long'}}
-    printf("%llx", x); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'long long'}}
+    printf("%llu", x); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'long long', which differs in signedness}}
+    printf("%llx", x); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'long long', which differs in signedness}}
 }
 
 void test_printf_unsigned_long_long(unsigned long long x)
 {
-    printf("%lld", x); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned long long'}}
+    printf("%lld", x); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned long long', which differs in signedness}}
     printf("%llu", x); // no-warning
     printf("%llx", x); // no-warning
 }
@@ -85,8 +85,8 @@ enum enum_int {
 void test_printf_enum_int(enum enum_int x)
 {
     printf("%d", x); // no-warning
-    printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'int'}}
-    printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'int'}}
+    printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'int', which differs in signedness}}
+    printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'int', which differs in signedness}}
 }
 
 #ifndef _WIN32 // Disabled due to enums have different underlying type on _WIN32
@@ -96,7 +96,7 @@ enum enum_unsigned {
 
 void test_printf_enum_unsigned(enum enum_unsigned x)
 {
-    printf("%d", x); // expected-warning{{format specifies type 'int' but the argument has underlying type 'unsigned int'}}
+    printf("%d", x); // expected-warning{{format specifies type 'int' but the argument has underlying type 'unsigned int', which differs in signedness}}
     printf("%u", x); // no-warning
     printf("%x", x); // no-warning
 }
@@ -110,8 +110,8 @@ enum enum_long {
 void test_printf_enum_long(enum enum_long x)
 {
     printf("%ld", x); // no-warning
-    printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long'}}
-    printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long'}}
+    printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long', which differs in signedness}}
+    printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long', which differs in signedness}}
 }
 
 enum enum_unsigned_long {
@@ -120,7 +120,7 @@ enum enum_unsigned_long {
 
 void test_printf_enum_unsigned_long(enum enum_unsigned_long x)
 {
-    printf("%ld", x); // expected-warning{{format specifies type 'long' but the argument has underlying type 'unsigned long'}}
+    printf("%ld", x); // expected-warning{{format specifies type 'long' but the argument has underlying type 'unsigned long', which differs in signedness}}
     printf("%lu", x); // no-warning
     printf("%lx", x); // no-warning
 }
@@ -136,61 +136,61 @@ void test_scanf_unsigned_char(unsigned char *y) {
 
 void test_scanf_int(int *x) {
   scanf("%d", x); // no-warning
-  scanf("%u", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'int *'}}
-  scanf("%x", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'int *'}}
+  scanf("%u", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'int *', which differs in signedness}}
+  scanf("%x", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'int *', which differs in signedness}}
 }
 
 void test_scanf_unsigned(unsigned *x) {
-  scanf("%d", x); // expected-warning{{format specifies type 'int *' but the argument has type 'unsigned int *'}}
+  scanf("%d", x); // expected-warning{{format specifies type 'int *' but the argument has type 'unsigned int *', which differs in signedness}}
   scanf("%u", x); // no-warning
   scanf("%x", x); // no-warning
 }
 
 void test_scanf_long(long *x) {
   scanf("%ld", x); // no-warning
-  scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'long *'}}
-  scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'long *'}}
+  scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'long *', which differs in signedness}}
+  scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'long *', which differs in signedness}}
 }
 
 void test_scanf_unsigned_long(unsigned long *x) {
-  scanf("%ld", x); // expected-warning{{format specifies type 'long *' but the argument has type 'unsigned long *'}}
+  scanf("%ld", x); // expected-warning{{format specifies type 'long *' but the argument has type 'unsigned long *', which differs in signedness}}
   scanf("%lu", x); // no-warning
   scanf("%lx", x); // no-warning
 }
 
 void test_scanf_longlong(long long *x) {
   scanf("%lld", x); // no-warning
-  scanf("%llu", x); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'long long *'}}
-  scanf("%llx", x); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'long long *'}}
+  scanf("%llu", x); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'long long *', which differs in signedness}}
+  scanf("%llx", x); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'long long *', which differs in signedness}}
 }
 
 void test_scanf_unsigned_longlong(unsigned long long *x) {
-  scanf("%lld", x); // expected-warning{{format specifies type 'long long *' but the argument has type 'unsigned long long *'}}
+  scanf("%lld", x); // expected-warning{{format specifies type 'long long *' but the argument has type 'unsigned long long *', which differs in signedness}}
   scanf("%llu", x); // no-warning
   scanf("%llx", x); // no-warning
 }
 
 void test_scanf_enum_int(enum enum_int *x) {
   scanf("%d", x); // no-warning
-  scanf("%u", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'enum enum_int *'}}
-  scanf("%x", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'enum enum_int *'}}
+  scanf("%u", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'enum enum_int *', which differs in signedness}}
+  scanf("%x", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'enum enum_int *', which differs in signedness}}
 }
 
 #ifndef _WIN32 // Disabled due to enums have different underlying type on _WIN32
 void test_scanf_enum_unsigned(enum enum_unsigned *x) {
-  scanf("%d", x); // expected-warning{{format specifies type 'int *' but the argument has type 'enum enum_unsigned *'}}
+  scanf("%d", x); // expected-warning{{format specifies type 'int *' but the argument has type 'enum enum_unsigned *', which differs in signedness}}
   scanf("%u", x); // no-warning
   scanf("%x", x); // no-warning
 }
 
 void test_scanf_enum_long(enum enum_long *x) {
   scanf("%ld", x); // no-warning
-  scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum enum_long *'}}
-  scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum enum_long *'}}
+  scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum enum_long *', which differs in signedness}}
+  scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum enum_long *', which differs in signedness}}
 }
 
 void test_scanf_enum_unsigned_long(enum enum_unsigned_long *x) {
-  scanf("%ld", x); // expected-warning{{format specifies type 'long *' but the argument has type 'enum enum_unsigned_long *'}}
+  scanf("%ld", x); // expected-warning{{format specifies type 'long *' but the argument has type 'enum enum_unsigned_long *', which differs in signedness}}
   scanf("%lu", x); // no-warning
   scanf("%lx", x); // no-warning
 }