From 0e5badd93b1c6c67bf29b7808f16b15b8d94549d Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Wed, 5 Dec 2012 18:44:49 +0000 Subject: [PATCH] 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. llvm-svn: 169400 --- clang/lib/Analysis/PrintfFormatString.cpp | 8 ++- clang/lib/Sema/SemaChecking.cpp | 91 +++++++++++++++++++++++-------- clang/test/FixIt/format.m | 33 +++++++++++ clang/test/FixIt/format.mm | 30 ++++++++++ clang/test/SemaObjC/format-strings-objc.m | 6 +- 5 files changed, 138 insertions(+), 30 deletions(-) create mode 100644 clang/test/FixIt/format.mm diff --git a/clang/lib/Analysis/PrintfFormatString.cpp b/clang/lib/Analysis/PrintfFormatString.cpp index 0820791..176aaed 100644 --- a/clang/lib/Analysis/PrintfFormatString.cpp +++ b/clang/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/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index d91f034..095d25a 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/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(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()) { StringRef Name = UserTy->getDecl()->getName(); - IntendedTy = llvm::StringSwitch(Name) + QualType CastTy = llvm::StringSwitch(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(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(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/clang/test/FixIt/format.m b/clang/test/FixIt/format.m index 7957e91..58c553a 100644 --- a/clang/test/FixIt/format.m +++ b/clang/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/clang/test/FixIt/format.mm b/clang/test/FixIt/format.mm new file mode 100644 index 0000000..64c6c47 --- /dev/null +++ b/clang/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/clang/test/SemaObjC/format-strings-objc.m b/clang/test/SemaObjC/format-strings-objc.m index 7faa995..bd33ad4 100644 --- a/clang/test/SemaObjC/format-strings-objc.m +++ b/clang/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 (). -- 2.7.4