Format strings: correct signedness if already correcting width (%d,%u).

It is valid to do this:
  printf("%u", (int)x);

But if we see this:
  printf("%lu", (int)x);

...our fixit should suggest %d, not %u.

llvm-svn: 172739

clang/lib/Analysis/PrintfFormatString.cpp

@@ -499,8 +499,26 @@ bool PrintfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
   if (isa<TypedefType>(QT) && (LangOpt.C99 || LangOpt.CPlusPlus11))
     namedTypeToLengthModifier(QT, LM);
 
-  // If fixing the length modifier was enough, we are done.
+  // If fixing the length modifier was enough, we might be done.
   if (hasValidLengthModifier(Ctx.getTargetInfo())) {
+    // If we're going to offer a fix anyway, make sure the sign matches.
+    switch (CS.getKind()) {
+    case ConversionSpecifier::uArg:
+    case ConversionSpecifier::UArg:
+      if (QT->isSignedIntegerType())
+        CS.setKind(clang::analyze_format_string::ConversionSpecifier::dArg);
+      break;
+    case ConversionSpecifier::dArg:
+    case ConversionSpecifier::DArg:
+    case ConversionSpecifier::iArg:
+      if (QT->isUnsignedIntegerType())
+        CS.setKind(clang::analyze_format_string::ConversionSpecifier::uArg);
+      break;
+    default:
+      // Other specifiers do not have signed/unsigned variants.
+      break;
+    }
+
     const analyze_printf::ArgType &ATR = getArgType(Ctx, IsObjCLiteral);
     if (ATR.isValid() && ATR.matchesType(Ctx, QT))
       return true;

clang/test/FixIt/format-darwin.m

@@ -118,6 +118,16 @@ void testPreserveHex() {
   // CHECK-64: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:"(unsigned long)"
 }
 
+void testSignedness(NSInteger i, NSUInteger u) {
+  printf("%d", u); // expected-warning{{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+  printf("%i", u); // expected-warning{{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+  printf("%u", i); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+
+  // CHECK-64: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%lu"
+  // CHECK-64: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%lu"
+  // CHECK-64: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%ld"
+}
+
 void testNoWarn() {
   printf("%ld", getNSInteger()); // no-warning
   printf("%lu", getNSUInteger()); // no-warning
@@ -154,6 +164,14 @@ void testNoWarn() {
   printf("%lu", getUInt32()); // no-warning
 }
 
+void testSignedness(NSInteger i, NSUInteger u) {
+  // It is valid to use a specifier with the opposite signedness as long as
+  // the type is correct.
+  printf("%d", u); // no-warning
+  printf("%i", u); // no-warning
+  printf("%u", i); // no-warning
+}
+
 #endif
 
 

clang/test/FixIt/format.m

@@ -213,3 +213,14 @@ void test_percent_C() {
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:22}:"(unichar)"
 }
+
+
+void testSignedness(long i, unsigned long u) {
+  printf("%d", u); // expected-warning{{format specifies type 'int' but the argument has type 'unsigned long'}}
+  printf("%i", u); // expected-warning{{format specifies type 'int' but the argument has type 'unsigned long'}}
+  printf("%u", i); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'long'}}
+
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%lu"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%lu"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:11-[[@LINE-4]]:13}:"%ld"
+}