Format strings: offer a cast to 'unichar' for %C in Objective-C contexts.

For most cases where a conversion specifier doesn't match an argument,
we usually guess that the conversion specifier is wrong. However, if
the argument is an integer type and the specifier is %C, it's likely
the user really did mean to print the integer as a character.

(This is more common than %c because there is no way to specify a unichar
literal -- you have to write an integer literal, such as '0x2603',
and then cast it to unichar.)

This does not change the behavior of %S, since there are fewer cases
where printing a literal Unicode *string* is necessary, but this could
easily be changed in the future.

<rdar://problem/11982013>

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@169400 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Analysis/PrintfFormatString.cpp b/lib/Analysis/PrintfFormatString.cpp
index 08207910b831a5762d58388761d2e93dc2f5d4de..176aaedeaa10c3881df6245dcc28c9f76f8dc1ea 100644 (file)
--- a/lib/Analysis/PrintfFormatString.cpp
+++ b/lib/Analysis/PrintfFormatString.cpp
@@ -359,17 +359,19 @@ ArgType PrintfSpecifier::getArgType(ASTContext &Ctx,
     case ConversionSpecifier::sArg:
       if (LM.getKind() == LengthModifier::AsWideChar) {
         if (IsObjCLiteral)
-          return Ctx.getPointerType(Ctx.UnsignedShortTy.withConst());
+          return ArgType(Ctx.getPointerType(Ctx.UnsignedShortTy.withConst()),
+                         "const unichar *");
         return ArgType(ArgType::WCStrTy, "wchar_t *");
       }
       return ArgType::CStrTy;
     case ConversionSpecifier::SArg:
       if (IsObjCLiteral)
-        return Ctx.getPointerType(Ctx.UnsignedShortTy.withConst());
+        return ArgType(Ctx.getPointerType(Ctx.UnsignedShortTy.withConst()),
+                       "const unichar *");
       return ArgType(ArgType::WCStrTy, "wchar_t *");
     case ConversionSpecifier::CArg:
       if (IsObjCLiteral)
-        return Ctx.UnsignedShortTy;
+        return ArgType(Ctx.UnsignedShortTy, "unichar");
       return ArgType(Ctx.WCharTy, "wchar_t");
     case ConversionSpecifier::pArg:
       return ArgType::CPointerTy;

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index d91f03416b63c36702a9ee1aebf566a79415c05f..095d25aa027f9d23395085bcc6bd870f6057df1d 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -2762,17 +2762,46 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
         ExprTy = S.Context.CharTy;
   }
 
+  // %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.
   QualType IntendedTy = ExprTy;
+  if (ObjCContext &&
+      FS.getConversionSpecifier().getKind() == ConversionSpecifier::CArg) {
+    if (ExprTy->isIntegralOrUnscopedEnumerationType() &&
+        !ExprTy->isCharType()) {
+      // 'unichar' is defined as a typedef of unsigned short, but we should
+      // prefer using the typedef if it is visible.
+      IntendedTy = S.Context.UnsignedShortTy;
+      
+      LookupResult Result(S, &S.Context.Idents.get("unichar"), E->getLocStart(),
+                          Sema::LookupOrdinaryName);
+      if (S.LookupName(Result, S.getCurScope())) {
+        NamedDecl *ND = Result.getFoundDecl();
+        if (TypedefNameDecl *TD = dyn_cast<TypedefNameDecl>(ND))
+          if (TD->getUnderlyingType() == IntendedTy)
+            IntendedTy = S.Context.getTypedefType(TD);
+      }
+    }
+  }
+
+  // Special-case some of Darwin's platform-independence types by suggesting
+  // casts to primitive types that are known to be large enough.
+  bool ShouldNotPrintDirectly = false;
   if (S.Context.getTargetInfo().getTriple().isOSDarwin()) {
-    // Special-case some of Darwin's platform-independence types.
     if (const TypedefType *UserTy = IntendedTy->getAs<TypedefType>()) {
       StringRef Name = UserTy->getDecl()->getName();
-      IntendedTy = llvm::StringSwitch<QualType>(Name)
+      QualType CastTy = llvm::StringSwitch<QualType>(Name)
         .Case("NSInteger", S.Context.LongTy)
         .Case("NSUInteger", S.Context.UnsignedLongTy)
         .Case("SInt32", S.Context.IntTy)
         .Case("UInt32", S.Context.UnsignedIntTy)
-        .Default(IntendedTy);
+        .Default(QualType());
+
+      if (!CastTy.isNull()) {
+        ShouldNotPrintDirectly = true;
+        IntendedTy = CastTy;
+      }
     }
   }
 
@@ -2789,7 +2818,19 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
 
     CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen);
 
-    if (IntendedTy != ExprTy) {
+    if (IntendedTy == ExprTy) {
+      // 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)
+          << AT.getRepresentativeTypeName(S.Context) << IntendedTy
+          << E->getSourceRange(),
+        E->getLocStart(),
+        /*IsStringLocation*/false,
+        SpecRange,
+        FixItHint::CreateReplacement(SpecRange, os.str()));
+
+    } else {
       // The canonical type for formatting this value is different from the
       // actual type of the expression. (This occurs, for example, with Darwin's
       // NSInteger on 32-bit platforms, where it is typedef'd as 'int', but
@@ -2827,26 +2868,28 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
         Hints.push_back(FixItHint::CreateInsertion(After, ")"));
       }
 
-      // We extract the name from the typedef because we don't want to show
-      // the underlying type in the diagnostic.
-      const TypedefType *UserTy = cast<TypedefType>(ExprTy);
-      StringRef Name = UserTy->getDecl()->getName();
-
-      // Finally, emit the diagnostic.
-      EmitFormatDiagnostic(S.PDiag(diag::warn_format_argument_needs_cast)
-                             << Name << IntendedTy
-                             << E->getSourceRange(),
-                           E->getLocStart(), /*IsStringLocation=*/false,
-                           SpecRange, Hints);
-    } else {
-      EmitFormatDiagnostic(
-        S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
-          << AT.getRepresentativeTypeName(S.Context) << IntendedTy
-          << E->getSourceRange(),
-        E->getLocStart(),
-        /*IsStringLocation*/false,
-        SpecRange,
-        FixItHint::CreateReplacement(SpecRange, os.str()));
+      if (ShouldNotPrintDirectly) {
+        // The expression has a type that should not be printed directly.
+        // We extract the name from the typedef because we don't want to show
+        // the underlying type in the diagnostic.
+        StringRef Name = cast<TypedefType>(ExprTy)->getDecl()->getName();
+
+        EmitFormatDiagnostic(S.PDiag(diag::warn_format_argument_needs_cast)
+                               << Name << IntendedTy
+                               << E->getSourceRange(),
+                             E->getLocStart(), /*IsStringLocation=*/false,
+                             SpecRange, Hints);
+      } else {
+        // In this case, the expression could be printed using a different
+        // 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)
+            << AT.getRepresentativeTypeName(S.Context) << ExprTy
+            << E->getSourceRange(),
+          E->getLocStart(), /*IsStringLocation*/false,
+          SpecRange, Hints);
+      }
     }
   } else {
     const CharSourceRange &CSR = getSpecifierRange(StartSpecifier,

diff --git a/test/FixIt/format.m b/test/FixIt/format.m
index 7957e91ba1e1365ecde05bdf5ea03b65ed7acb3d..58c553a01094d0bc99bdda3c378ddfa14aa40fdc 100644 (file)
--- a/test/FixIt/format.m
+++ b/test/FixIt/format.m
@@ -180,3 +180,36 @@ void multichar_constants_false_negative() {
   NSLog(@"%c", 'abcd'); // missing-warning{{format specifies type 'char' but the argument has type 'int'}}
   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
 }
+
+
+void test_percent_C() {
+  const unsigned short data = 'a';
+  NSLog(@"%C", data);  // no-warning
+
+  NSLog(@"%C", 0x2603);  // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unsigned short)"
+
+  typedef unsigned short unichar;
+  
+  NSLog(@"%C", 0x2603);  // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)"
+  
+  NSLog(@"%C", data ? 0x2F : 0x2603); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:36-[[@LINE-3]]:36}:")"
+
+  NSLog(@"%C", 0.0); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'double'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%f"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)"
+
+  NSLog(@"%C", (char)0x2603); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'char'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:22}:"(unichar)"
+
+  NSLog(@"%C", 'a'); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'char'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:22}:"(unichar)"
+}

diff --git a/test/FixIt/format.mm b/test/FixIt/format.mm
new file mode 100644 (file)
index 0000000..64c6c47
--- /dev/null
+++ b/test/FixIt/format.mm
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
+// RUN: %clang_cc1 -fdiagnostics-parseable-fixits -fblocks %s 2>&1 | FileCheck %s
+
+extern "C" void NSLog(id, ...);
+
+void test_percent_C() {
+  const unsigned short data = 'a';
+  NSLog(@"%C", data);  // no-warning
+
+  const wchar_t wchar_data = L'a';
+  NSLog(@"%C", wchar_data);  // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'wchar_t'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"(unsigned short)"
+
+  NSLog(@"%C", 0x2603);  // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unsigned short)"
+
+  typedef unsigned short unichar;
+
+  NSLog(@"%C", wchar_data);  // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'wchar_t'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"(unichar)"
+  
+  NSLog(@"%C", 0x2603);  // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)"
+  
+  NSLog(@"%C", 0.0); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'double'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%f"
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)"
+}

diff --git a/test/SemaObjC/format-strings-objc.m b/test/SemaObjC/format-strings-objc.m
index 7faa995002dbdc3d3869ce25827f5c4dc1979294..bd33ad41a560ce40e73d35ec9a511ba5d8177d05 100644 (file)
--- a/test/SemaObjC/format-strings-objc.m
+++ b/test/SemaObjC/format-strings-objc.m
@@ -145,7 +145,7 @@ void test_percent_S() {
   NSLog(@"%S", ptr);  // no-warning
 
   const wchar_t* wchar_ptr = L"ab";
-  NSLog(@"%S", wchar_ptr);  // expected-warning{{format specifies type 'const unsigned short *' but the argument has type 'const wchar_t *'}}
+  NSLog(@"%S", wchar_ptr);  // expected-warning{{format specifies type 'const unichar *' (aka 'const unsigned short *') but the argument has type 'const wchar_t *'}}
 }
 
 void test_percent_ls() {
@@ -154,7 +154,7 @@ void test_percent_ls() {
   NSLog(@"%ls", ptr);  // no-warning
 
   const wchar_t* wchar_ptr = L"ab";
-  NSLog(@"%ls", wchar_ptr);  // expected-warning{{format specifies type 'const unsigned short *' but the argument has type 'const wchar_t *'}}
+  NSLog(@"%ls", wchar_ptr);  // expected-warning{{format specifies type 'const unichar *' (aka 'const unsigned short *') but the argument has type 'const wchar_t *'}}
 }
 
 void test_percent_C() {
@@ -162,7 +162,7 @@ void test_percent_C() {
   NSLog(@"%C", data);  // no-warning
 
   const wchar_t wchar_data = L'a';
-  NSLog(@"%C", wchar_data);  // expected-warning{{format specifies type 'unsigned short' but the argument has type 'wchar_t'}}
+  NSLog(@"%C", wchar_data);  // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'wchar_t'}}
 }
 
 // Test that %@ works with toll-free bridging (<rdar://problem/10814120>).