Format strings: check against an enum's underlying type.

This allows us to be more careful when dealing with enums whose fixed
underlying type requires special handling in a format string, like
NSInteger.

A refinement of r163266 from a year and a half ago, which added the
special handling for NSInteger and friends in the first place.

<rdar://problem/16616623>

llvm-svn: 209966

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6295,12 +6295,13 @@ def warn_missing_format_string : Warning<
 def warn_scanf_nonzero_width : Warning<
   "zero field width in scanf format string is unused">,
   InGroup<Format>;
-def warn_printf_conversion_argument_type_mismatch : Warning<
+def warn_format_conversion_argument_type_mismatch : Warning<
-  "format specifies type %0 but the argument has type %1">,
+  "format specifies type %0 but the argument has "
+  "%select{type|underlying type}2 %1">,
   InGroup<Format>;
 def warn_format_argument_needs_cast : Warning<
-  "values of type '%0' should not be used as format arguments; add an explicit "
+  "%select{values of type|enum values with underlying type}2 '%0' should not "
-  "cast to %1 instead">,
+  "be used as format arguments; add an explicit cast to %1 instead">,
   InGroup<Format>;
 def warn_printf_positional_arg_exceeds_data_args : Warning <
   "data argument position '%0' exceeds the number of data arguments (%1)">,

clang/lib/Sema/SemaChecking.cpp

@@ -3110,6 +3110,13 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
         ExprTy = S.Context.CharTy;
   }
 
+  // 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 the argument is an integer of some kind, believe the %C and suggest
   // a cast instead of changing the conversion specifier.
@@ -3182,8 +3189,8 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
       // In this case, the specifier is wrong and should be changed to match
       // the argument.
       EmitFormatDiagnostic(
-        S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
+        S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
-          << AT.getRepresentativeTypeName(S.Context) << IntendedTy
+          << AT.getRepresentativeTypeName(S.Context) << IntendedTy << IsEnum
           << E->getSourceRange(),
         E->getLocStart(),
         /*IsStringLocation*/false,
@@ -3235,7 +3242,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
         StringRef Name = cast<TypedefType>(ExprTy)->getDecl()->getName();
 
         EmitFormatDiagnostic(S.PDiag(diag::warn_format_argument_needs_cast)
-                               << Name << IntendedTy
+                               << Name << IntendedTy << IsEnum
                                << E->getSourceRange(),
                              E->getLocStart(), /*IsStringLocation=*/false,
                              SpecRange, Hints);
@@ -3244,8 +3251,8 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
         // specifier, but we've decided that the specifier is probably correct 
         // and we should cast instead. Just use the normal warning message.
         EmitFormatDiagnostic(
-          S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
+          S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
-            << AT.getRepresentativeTypeName(S.Context) << ExprTy
+            << AT.getRepresentativeTypeName(S.Context) << ExprTy << IsEnum
             << E->getSourceRange(),
           E->getLocStart(), /*IsStringLocation*/false,
           SpecRange, Hints);
@@ -3261,8 +3268,8 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
     case Sema::VAK_Valid:
     case Sema::VAK_ValidInCXX11:
       EmitFormatDiagnostic(
-        S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
+        S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
-          << AT.getRepresentativeTypeName(S.Context) << ExprTy
+          << AT.getRepresentativeTypeName(S.Context) << ExprTy << IsEnum
           << CSR
           << E->getSourceRange(),
         E->getLocStart(), /*IsStringLocation*/false, CSR);
@@ -3453,8 +3460,8 @@ bool CheckScanfHandler::HandleScanfSpecifier(
       fixedFS.toString(os);
 
       EmitFormatDiagnostic(
-        S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
+        S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
-          << AT.getRepresentativeTypeName(S.Context) << Ex->getType()
+          << AT.getRepresentativeTypeName(S.Context) << Ex->getType() << false
           << Ex->getSourceRange(),
         Ex->getLocStart(),
         /*IsStringLocation*/false,
@@ -3464,8 +3471,8 @@ bool CheckScanfHandler::HandleScanfSpecifier(
           os.str()));
     } else {
       EmitFormatDiagnostic(
-        S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
+        S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
-          << AT.getRepresentativeTypeName(S.Context) << Ex->getType()
+          << AT.getRepresentativeTypeName(S.Context) << Ex->getType() << false
           << Ex->getSourceRange(),
         Ex->getLocStart(),
         /*IsStringLocation*/false,

clang/test/FixIt/format-darwin.m

@@ -1,11 +1,8 @@
 // RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -fblocks -Wformat-non-iso -verify %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -fblocks -Wformat-non-iso -verify %s
 
-// RUN: %clang_cc1 -triple i386-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks -Wformat-non-iso %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks -Wformat-non-iso %s 2>&1 | FileCheck -check-prefix=CHECK -check-prefix=CHECK-32 %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks -Wformat-non-iso %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks -Wformat-non-iso %s 2>&1 | FileCheck -check-prefix=CHECK -check-prefix=CHECK-64 %s
-
-// RUN: %clang_cc1 -triple i386-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks -Wformat-non-iso %s 2>&1 | FileCheck -check-prefix=CHECK-32 %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks -Wformat-non-iso %s 2>&1 | FileCheck -check-prefix=CHECK-64 %s
 
 int printf(const char * restrict, ...);
 
@@ -25,10 +22,16 @@ typedef unsigned long UInt32;
 
 typedef SInt32 OSStatus;
 
+typedef enum NSIntegerEnum : NSInteger {
+  EnumValueA,
+  EnumValueB
+} NSIntegerEnum;
+
 NSInteger getNSInteger();
 NSUInteger getNSUInteger();
 SInt32 getSInt32();
 UInt32 getUInt32();
+NSIntegerEnum getNSIntegerEnum();
 
 void testCorrectionInAllCases() {
   printf("%s", getNSInteger()); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
@@ -47,6 +50,11 @@ void testCorrectionInAllCases() {
 
   // CHECK: fix-it:"{{.*}}":{[[@LINE-11]]:11-[[@LINE-11]]:13}:"%u"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-12]]:16-[[@LINE-12]]:16}:"(unsigned int)"
+
+  printf("%s", getNSIntegerEnum()); // expected-warning{{enum values with underlying type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:13}:"%ld"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:16-[[@LINE-3]]:16}:"(long)"
 }
 
 @interface Foo {
@@ -107,6 +115,11 @@ void testWarn() {
 
   // CHECK-64: fix-it:"{{.*}}":{[[@LINE-11]]:11-[[@LINE-11]]:14}:"%u"
   // CHECK-64: fix-it:"{{.*}}":{[[@LINE-12]]:17-[[@LINE-12]]:17}:"(unsigned int)"
+
+  printf("%d", getNSIntegerEnum()); // expected-warning{{enum values with underlying type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+
+  // CHECK-64: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:13}:"%ld"
+  // CHECK-64: fix-it:"{{.*}}":{[[@LINE-3]]:16-[[@LINE-3]]:16}:"(long)"
 }
 
 void testPreserveHex() {
@@ -135,6 +148,7 @@ void testNoWarn() {
   printf("%lu", getNSUInteger()); // no-warning
   printf("%d", getSInt32()); // no-warning
   printf("%u", getUInt32()); // no-warning
+  printf("%ld", getNSIntegerEnum()); // no-warning
 }
 
 #else
@@ -149,6 +163,10 @@ void testWarn() {
   // CHECK-32: fix-it:"{{.*}}":{[[@LINE-5]]:17-[[@LINE-5]]:17}:"(unsigned long)"
   // CHECK-32: fix-it:"{{.*}}":{[[@LINE-5]]:16-[[@LINE-5]]:16}:"(int)"
   // CHECK-32: fix-it:"{{.*}}":{[[@LINE-5]]:16-[[@LINE-5]]:16}:"(unsigned int)"
+
+  printf("%ld", getNSIntegerEnum()); // expected-warning{{enum values with underlying type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+
+  // CHECK-32: fix-it:"{{.*}}":{[[@LINE-2]]:17-[[@LINE-2]]:17}:"(long)"
 }
 
 void testPreserveHex() {
@@ -164,6 +182,7 @@ void testNoWarn() {
   printf("%u", getNSUInteger()); // no-warning
   printf("%ld", getSInt32()); // no-warning
   printf("%lu", getUInt32()); // no-warning
+  printf("%d", getNSIntegerEnum()); // no-warning
 }
 
 void testSignedness(NSInteger i, NSUInteger u) {
@@ -194,6 +213,11 @@ void testCasts() {
 
   // CHECK: fix-it:"{{.*}}":{[[@LINE-11]]:11-[[@LINE-11]]:13}:"%u"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-12]]:16-[[@LINE-12]]:24}:"(unsigned int)"
+
+  printf("%s", (NSIntegerEnum)0); // expected-warning{{enum values with underlying type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:13}:"%ld"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:16-[[@LINE-3]]:31}:"(long)"
 }
 
 void testCapitals() {

clang/test/FixIt/format.m

@@ -82,14 +82,14 @@ void test_class_correction (Class x) {
 
 typedef enum : int { NSUTF8StringEncoding = 8 } NSStringEncoding;
 void test_fixed_enum_correction(NSStringEncoding x) {
-  NSLog(@"%@", x); // expected-warning{{format specifies type 'id' but the argument has type 'NSStringEncoding'}}
+  NSLog(@"%@", x); // expected-warning{{format specifies type 'id' but the argument has underlying type 'int'}}
   // CHECK: fix-it:"{{.*}}":{85:11-85:13}:"%d"
 }
 
 typedef __SIZE_TYPE__ size_t;
 enum SomeSize : size_t { IntegerSize = sizeof(int) };
 void test_named_fixed_enum_correction(enum SomeSize x) {
-  NSLog(@"%@", x); // expected-warning{{format specifies type 'id' but the argument has type 'enum SomeSize'}}
+  NSLog(@"%@", x); // expected-warning{{format specifies type 'id' but the argument has underlying type 'size_t' (aka 'unsigned long')}}
   // CHECK: fix-it:"{{.*}}":{92:11-92:13}:"%zu"
 }
 
@@ -228,3 +228,29 @@ void testSignedness(long i, unsigned long u) {
 
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:14}:"%+ld"
 }
+
+void testEnum() {
+  typedef enum {
+    ImplicitA = 1,
+    ImplicitB = 2
+  } Implicit;
+
+  typedef enum {
+    ImplicitLLA = 0,
+    ImplicitLLB = ~0ULL
+  } ImplicitLongLong;
+
+  typedef enum : short {
+    ExplicitA = 0,
+    ExplicitB
+  } ExplicitShort;
+
+  printf("%f", (Implicit)0); // expected-warning{{format specifies type 'double' but the argument has underlying type}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%{{[du]}}"
+
+  printf("%f", (ImplicitLongLong)0); // expected-warning{{format specifies type 'double' but the argument has underlying type}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%{{l*[du]}}"
+
+  printf("%f", (ExplicitShort)0); // expected-warning{{format specifies type 'double' but the argument has underlying type 'short'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%hd"
+}

clang/test/Sema/format-strings-enum-fixed-type.cpp

@@ -16,7 +16,7 @@ typedef enum : short { Constant = 0 } TestEnum;
 // This is why we don't check for that in the expected output.
 
 void test(TestEnum input) {
-  printf("%hhd", input); // expected-warning{{format specifies type 'char' but the argument has type 'TestEnum'}}
+  printf("%hhd", input); // expected-warning{{format specifies type 'char' but the argument has underlying type 'short'}}
   printf("%hhd", Constant); // expected-warning{{format specifies type 'char'}}
   
   printf("%hd", input); // no-warning
@@ -26,7 +26,7 @@ void test(TestEnum input) {
   printf("%d", input); // no-warning
   printf("%d", Constant); // no-warning
   
-  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has type 'TestEnum'}}
+  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has underlying type 'short'}}
   printf("%lld", Constant); // expected-warning{{format specifies type 'long long'}}
 }
 
@@ -34,7 +34,7 @@ void test(TestEnum input) {
 typedef enum : unsigned long { LongConstant = ~0UL } LongEnum;
 
 void testLong(LongEnum input) {
-  printf("%u", input); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'LongEnum'}}
+  printf("%u", input); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'unsigned long'}}
   printf("%u", LongConstant); // expected-warning{{format specifies type 'unsigned int'}}
   
   printf("%lu", input);
@@ -46,7 +46,7 @@ typedef short short_t;
 typedef enum : short_t { ShortConstant = 0 } ShortEnum;
 
 void testUnderlyingTypedef(ShortEnum input) {
-  printf("%hhd", input); // expected-warning{{format specifies type 'char' but the argument has type 'ShortEnum'}}
+  printf("%hhd", input); // expected-warning{{format specifies type 'char' but the argument has underlying type 'short_t' (aka 'short')}}
   printf("%hhd", ShortConstant); // expected-warning{{format specifies type 'char'}}
   
   printf("%hd", input); // no-warning
@@ -56,7 +56,7 @@ void testUnderlyingTypedef(ShortEnum input) {
   printf("%d", input); // no-warning
   printf("%d", ShortConstant); // no-warning
   
-  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has type 'ShortEnum'}}
+  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has underlying type 'short_t' (aka 'short')}}
   printf("%lld", ShortConstant); // expected-warning{{format specifies type 'long long'}}
 }
 
@@ -64,10 +64,10 @@ void testUnderlyingTypedef(ShortEnum input) {
 typedef ShortEnum ShortEnum2;
 
 void testTypedefChain(ShortEnum2 input) {
-  printf("%hhd", input); // expected-warning{{format specifies type 'char' but the argument has type 'ShortEnum2' (aka 'ShortEnum')}}
+  printf("%hhd", input); // expected-warning{{format specifies type 'char' but the argument has underlying type 'short_t' (aka 'short')}}
   printf("%hd", input); // no-warning
   printf("%d", input); // no-warning
-  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has type 'ShortEnum2' (aka 'ShortEnum')}}
+  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has underlying type 'short_t' (aka 'short')}}
 }
 
 
@@ -80,13 +80,13 @@ void testChar(CharEnum input) {
   printf("%hhd", CharConstant); // no-warning
 
   // This is not correct but it is safe. We warn because '%hd' shows intent.
-  printf("%hd", input); // expected-warning{{format specifies type 'short' but the argument has type 'CharEnum'}}
+  printf("%hd", input); // expected-warning{{format specifies type 'short' but the argument has underlying type 'char'}}
   printf("%hd", CharConstant); // expected-warning{{format specifies type 'short'}}
   
   // This is not correct but it matches the promotion rules (and is safe).
   printf("%d", input); // no-warning
   printf("%d", CharConstant); // no-warning
   
-  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has type 'CharEnum'}}
+  printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has underlying type 'char'}}
   printf("%lld", CharConstant); // expected-warning{{format specifies type 'long long'}}
 }

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

@@ -20,7 +20,7 @@ void test(TestEnum input) {
     printf("%d", input); // no-warning
     printf("%d", Constant); // no-warning
 
-    printf("%lld", input); // expected-warning{{format specifies type 'long long' but the argument has type 'TestEnum'}}
+    printf("%lld", input); // expected-warning-re{{format specifies type 'long long' but the argument has underlying type '{{(unsigned)?}} int'}}
     printf("%lld", Constant); // expected-warning{{format specifies type 'long long'}}
 }
 
@@ -28,7 +28,7 @@ void test(TestEnum input) {
 typedef enum { LongConstant = ~0UL } LongEnum;
 
 void testLong(LongEnum input) {
-  printf("%u", input); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'LongEnum'}}
+  printf("%u", input); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type}}
   printf("%u", LongConstant); // expected-warning{{format specifies type 'unsigned int'}}
   
   printf("%lu", input);