From: Jordan Rose Date: Sat, 31 May 2014 04:12:14 +0000 (+0000) Subject: Format strings: check against an enum's underlying type. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a0ea5b8f25880fa9f07d9b210afccf5e40271a99;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@209966 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index da011e4615..175a5a94a6 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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; -def warn_printf_conversion_argument_type_mismatch : Warning< - "format specifies type %0 but the argument has type %1">, +def warn_format_conversion_argument_type_mismatch : Warning< + "format specifies type %0 but the argument has " + "%select{type|underlying type}2 %1">, InGroup; def warn_format_argument_needs_cast : Warning< - "values of type '%0' should not be used as format arguments; add an explicit " - "cast to %1 instead">, + "%select{values of type|enum values with underlying type}2 '%0' should not " + "be used as format arguments; add an explicit cast to %1 instead">, InGroup; def warn_printf_positional_arg_exceeds_data_args : Warning < "data argument position '%0' exceeds the number of data arguments (%1)">, diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 7c539d530a..60514efd55 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/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()) { + 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) - << AT.getRepresentativeTypeName(S.Context) << IntendedTy + S.PDiag(diag::warn_format_conversion_argument_type_mismatch) + << 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(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) - << AT.getRepresentativeTypeName(S.Context) << ExprTy + S.PDiag(diag::warn_format_conversion_argument_type_mismatch) + << 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) - << AT.getRepresentativeTypeName(S.Context) << ExprTy + S.PDiag(diag::warn_format_conversion_argument_type_mismatch) + << 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) - << AT.getRepresentativeTypeName(S.Context) << Ex->getType() + S.PDiag(diag::warn_format_conversion_argument_type_mismatch) + << 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) - << AT.getRepresentativeTypeName(S.Context) << Ex->getType() + S.PDiag(diag::warn_format_conversion_argument_type_mismatch) + << AT.getRepresentativeTypeName(S.Context) << Ex->getType() << false << Ex->getSourceRange(), Ex->getLocStart(), /*IsStringLocation*/false, diff --git a/test/FixIt/format-darwin.m b/test/FixIt/format-darwin.m index f520564348..170bb09fb9 100644 --- a/test/FixIt/format-darwin.m +++ b/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 x86_64-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-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 +// 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 -check-prefix=CHECK -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() { diff --git a/test/FixIt/format.m b/test/FixIt/format.m index 5e4636047e..fea8465485 100644 --- a/test/FixIt/format.m +++ b/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" +} diff --git a/test/Sema/format-strings-enum-fixed-type.cpp b/test/Sema/format-strings-enum-fixed-type.cpp index 8b6b237645..0022ccbc66 100644 --- a/test/Sema/format-strings-enum-fixed-type.cpp +++ b/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'}} } diff --git a/test/Sema/format-strings-enum.c b/test/Sema/format-strings-enum.c index a6c27d0b6e..e79f8598ab 100644 --- a/test/Sema/format-strings-enum.c +++ b/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);