From: Jordan Rose Date: Sat, 8 Sep 2012 04:00:12 +0000 (+0000) Subject: Format strings: suggest %lld instead of %qd and %Ld with -Wformat-non-iso. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8be066e6733364cd34f25c4f7b7344f72aa23369;p=clang Format strings: suggest %lld instead of %qd and %Ld with -Wformat-non-iso. As a corollary to the previous commit, even when an extension is available, we can still offer a fixit to the standard modifier. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@163453 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/Analyses/FormatString.h b/include/clang/Analysis/Analyses/FormatString.h index c69f17a928..0579265db4 100644 --- a/include/clang/Analysis/Analyses/FormatString.h +++ b/include/clang/Analysis/Analyses/FormatString.h @@ -118,7 +118,7 @@ public: cArg, dArg, iArg, - IntArgBeg = cArg, IntArgEnd = iArg, + IntArgBeg = dArg, IntArgEnd = iArg, oArg, uArg, @@ -191,7 +191,9 @@ public: return EndScanList ? EndScanList - Position : 1; } + bool isIntArg() const { return kind >= IntArgBeg && kind <= IntArgEnd; } bool isUIntArg() const { return kind >= UIntArgBeg && kind <= UIntArgEnd; } + bool isAnyIntArg() const { return kind >= IntArgBeg && kind <= UIntArgEnd; } const char *toString() const; bool isPrintfKind() const { return IsPrintf; } @@ -382,7 +384,6 @@ public: : ConversionSpecifier(true, pos, k) {} bool isObjCArg() const { return kind >= ObjCBeg && kind <= ObjCEnd; } - bool isIntArg() const { return kind >= IntArgBeg && kind <= IntArgEnd; } bool isDoubleArg() const { return kind >= DoubleArgBeg && kind <= DoubleArgEnd; } unsigned getLength() const { diff --git a/lib/Analysis/FormatString.cpp b/lib/Analysis/FormatString.cpp index 45c722a5d5..c6ba6fab07 100644 --- a/lib/Analysis/FormatString.cpp +++ b/lib/Analysis/FormatString.cpp @@ -723,21 +723,13 @@ bool FormatSpecifier::hasStandardLengthConversionCombination() const { llvm::Optional FormatSpecifier::getCorrectedLengthModifier() const { - if (LM.getKind() == LengthModifier::AsLongDouble) { - switch (CS.getKind()) { - case ConversionSpecifier::dArg: - case ConversionSpecifier::iArg: - case ConversionSpecifier::oArg: - case ConversionSpecifier::uArg: - case ConversionSpecifier::xArg: - case ConversionSpecifier::XArg: { + if (CS.isAnyIntArg() || CS.getKind() == ConversionSpecifier::nArg) { + if (LM.getKind() == LengthModifier::AsLongDouble || + LM.getKind() == LengthModifier::AsQuad) { LengthModifier FixedLM(LM); FixedLM.setKind(LengthModifier::AsLongLong); return FixedLM; } - default: - break; - } } return llvm::Optional(); diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index c477de8531..dd1f9e109b 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -1941,21 +1941,16 @@ public: void HandleInvalidLengthModifier( const analyze_format_string::FormatSpecifier &FS, const analyze_format_string::ConversionSpecifier &CS, - const char *startSpecifier, unsigned specifierLen); + const char *startSpecifier, unsigned specifierLen, unsigned DiagID); void HandleNonStandardLengthModifier( - const analyze_format_string::LengthModifier &LM, + const analyze_format_string::FormatSpecifier &FS, const char *startSpecifier, unsigned specifierLen); void HandleNonStandardConversionSpecifier( const analyze_format_string::ConversionSpecifier &CS, const char *startSpecifier, unsigned specifierLen); - void HandleNonStandardConversionSpecification( - const analyze_format_string::LengthModifier &LM, - const analyze_format_string::ConversionSpecifier &CS, - const char *startSpecifier, unsigned specifierLen); - virtual void HandlePosition(const char *startPos, unsigned posLen); virtual void HandleInvalidPosition(const char *startSpecifier, @@ -2036,7 +2031,7 @@ void CheckFormatHandler::HandleIncompleteSpecifier(const char *startSpecifier, void CheckFormatHandler::HandleInvalidLengthModifier( const analyze_format_string::FormatSpecifier &FS, const analyze_format_string::ConversionSpecifier &CS, - const char *startSpecifier, unsigned specifierLen) { + const char *startSpecifier, unsigned specifierLen, unsigned DiagID) { using namespace analyze_format_string; const LengthModifier &LM = FS.getLengthModifier(); @@ -2045,8 +2040,7 @@ void CheckFormatHandler::HandleInvalidLengthModifier( // See if we know how to fix this length modifier. llvm::Optional FixedLM = FS.getCorrectedLengthModifier(); if (FixedLM) { - EmitFormatDiagnostic(S.PDiag(diag::warn_format_nonsensical_length) - << LM.toString() << CS.toString(), + EmitFormatDiagnostic(S.PDiag(DiagID) << LM.toString() << CS.toString(), getLocationOfByte(LM.getStart()), /*IsStringLocation*/true, getSpecifierRange(startSpecifier, specifierLen)); @@ -2056,23 +2050,46 @@ void CheckFormatHandler::HandleInvalidLengthModifier( << FixItHint::CreateReplacement(LMRange, FixedLM->toString()); } else { - EmitFormatDiagnostic(S.PDiag(diag::warn_format_nonsensical_length) - << LM.toString() << CS.toString(), + FixItHint Hint; + if (DiagID == diag::warn_format_nonsensical_length) + Hint = FixItHint::CreateRemoval(LMRange); + + EmitFormatDiagnostic(S.PDiag(DiagID) << LM.toString() << CS.toString(), getLocationOfByte(LM.getStart()), /*IsStringLocation*/true, getSpecifierRange(startSpecifier, specifierLen), - FixItHint::CreateRemoval(LMRange)); + Hint); } } void CheckFormatHandler::HandleNonStandardLengthModifier( - const analyze_format_string::LengthModifier &LM, + const analyze_format_string::FormatSpecifier &FS, const char *startSpecifier, unsigned specifierLen) { - EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard) << LM.toString() - << 0, - getLocationOfByte(LM.getStart()), - /*IsStringLocation*/true, - getSpecifierRange(startSpecifier, specifierLen)); + using namespace analyze_format_string; + + const LengthModifier &LM = FS.getLengthModifier(); + CharSourceRange LMRange = getSpecifierRange(LM.getStart(), LM.getLength()); + + // See if we know how to fix this length modifier. + llvm::Optional FixedLM = FS.getCorrectedLengthModifier(); + if (FixedLM) { + EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard) + << LM.toString() << 0, + getLocationOfByte(LM.getStart()), + /*IsStringLocation*/true, + getSpecifierRange(startSpecifier, specifierLen)); + + S.Diag(getLocationOfByte(LM.getStart()), diag::note_format_fix_specifier) + << FixedLM->toString() + << FixItHint::CreateReplacement(LMRange, FixedLM->toString()); + + } else { + EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard) + << LM.toString() << 0, + getLocationOfByte(LM.getStart()), + /*IsStringLocation*/true, + getSpecifierRange(startSpecifier, specifierLen)); + } } void CheckFormatHandler::HandleNonStandardConversionSpecifier( @@ -2085,17 +2102,6 @@ void CheckFormatHandler::HandleNonStandardConversionSpecifier( getSpecifierRange(startSpecifier, specifierLen)); } -void CheckFormatHandler::HandleNonStandardConversionSpecification( - const analyze_format_string::LengthModifier &LM, - const analyze_format_string::ConversionSpecifier &CS, - const char *startSpecifier, unsigned specifierLen) { - EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard_conversion_spec) - << LM.toString() << CS.toString(), - getLocationOfByte(LM.getStart()), - /*IsStringLocation*/true, - getSpecifierRange(startSpecifier, specifierLen)); -} - void CheckFormatHandler::HandlePosition(const char *startPos, unsigned posLen) { EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard_positional_arg), @@ -2602,14 +2608,14 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier startSpecifier, specifierLen); // Check the length modifier is valid with the given conversion specifier. - const LengthModifier &LM = FS.getLengthModifier(); if (!FS.hasValidLengthModifier(S.getASTContext().getTargetInfo())) - HandleInvalidLengthModifier(FS, CS, startSpecifier, specifierLen); + HandleInvalidLengthModifier(FS, CS, startSpecifier, specifierLen, + diag::warn_format_nonsensical_length); else if (!FS.hasStandardLengthModifier()) - HandleNonStandardLengthModifier(LM, startSpecifier, specifierLen); + HandleNonStandardLengthModifier(FS, startSpecifier, specifierLen); else if (!FS.hasStandardLengthConversionCombination()) - HandleNonStandardConversionSpecification(LM, CS, startSpecifier, - specifierLen); + HandleInvalidLengthModifier(FS, CS, startSpecifier, specifierLen, + diag::warn_format_non_standard_conversion_spec); if (!FS.hasStandardConversionSpecifier(S.getLangOpts())) HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen); @@ -2916,14 +2922,14 @@ bool CheckScanfHandler::HandleScanfSpecifier( } // Check the length modifier is valid with the given conversion specifier. - const LengthModifier &LM = FS.getLengthModifier(); if (!FS.hasValidLengthModifier(S.getASTContext().getTargetInfo())) - HandleInvalidLengthModifier(FS, CS, startSpecifier, specifierLen); + HandleInvalidLengthModifier(FS, CS, startSpecifier, specifierLen, + diag::warn_format_nonsensical_length); else if (!FS.hasStandardLengthModifier()) - HandleNonStandardLengthModifier(LM, startSpecifier, specifierLen); + HandleNonStandardLengthModifier(FS, startSpecifier, specifierLen); else if (!FS.hasStandardLengthConversionCombination()) - HandleNonStandardConversionSpecification(LM, CS, startSpecifier, - specifierLen); + HandleInvalidLengthModifier(FS, CS, startSpecifier, specifierLen, + diag::warn_format_non_standard_conversion_spec); if (!FS.hasStandardConversionSpecifier(S.getLangOpts())) HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen); diff --git a/test/Sema/format-strings-non-iso.c b/test/Sema/format-strings-non-iso.c index e5ff8798f6..ee45946961 100644 --- a/test/Sema/format-strings-non-iso.c +++ b/test/Sema/format-strings-non-iso.c @@ -7,8 +7,8 @@ void f(void) { char *cp; // The 'q' length modifier. - printf("%qd", (long long)42); // expected-warning{{'q' length modifier is not supported by ISO C}} - scanf("%qd", (long long *)0); // expected-warning{{'q' length modifier is not supported by ISO C}} + printf("%qd", (long long)42); // expected-warning{{'q' length modifier is not supported by ISO C}} expected-note{{did you mean to use 'll'?}} + scanf("%qd", (long long *)0); // expected-warning{{'q' length modifier is not supported by ISO C}} expected-note{{did you mean to use 'll'?}} // The 'm' length modifier. scanf("%ms", &cp); // expected-warning{{'m' length modifier is not supported by ISO C}} @@ -18,11 +18,11 @@ void f(void) { printf("%C", L'x'); // expected-warning{{'C' conversion specifier is not supported by ISO C}} // Combining 'L' with an integer conversion specifier. - printf("%Li", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'i' is not supported by ISO C}} - printf("%Lo", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'o' is not supported by ISO C}} - printf("%Lu", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'u' is not supported by ISO C}} - printf("%Lx", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'x' is not supported by ISO C}} - printf("%LX", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'X' is not supported by ISO C}} + printf("%Li", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'i' is not supported by ISO C}} expected-note{{did you mean to use 'll'?}} + printf("%Lo", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'o' is not supported by ISO C}} expected-note{{did you mean to use 'll'?}} + printf("%Lu", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'u' is not supported by ISO C}} expected-note{{did you mean to use 'll'?}} + printf("%Lx", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'x' is not supported by ISO C}} expected-note{{did you mean to use 'll'?}} + printf("%LX", (long long)42); // expected-warning{{using length modifier 'L' with conversion specifier 'X' is not supported by ISO C}} expected-note{{did you mean to use 'll'?}} // Positional arguments. printf("%1$d", 42); // expected-warning{{positional arguments are not supported by ISO C}} diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c index 2e980a59a2..8fb1218b99 100644 --- a/test/Sema/format-strings.c +++ b/test/Sema/format-strings.c @@ -117,6 +117,7 @@ void check_writeback_specifier() printf("%qn", (int*)0); // expected-warning{{format specifies type 'long long *' but the argument has type 'int *'}} printf("%Ln", 0); // expected-warning{{length modifier 'L' results in undefined behavior or no effect with 'n' conversion specifier}} + // expected-note@-1{{did you mean to use 'll'?}} } void check_invalid_specifier(FILE* fp, char *buf)