[cfe-commits] r163453 - in /cfe/trunk: include/clang/Analysis/Analyses/FormatString.h lib/Analysis/FormatString.cpp lib/Sema/SemaChecking.cpp test/Sema/format-strings-non-iso.c test/Sema/format-strings.c

Jordan Rose jordan_rose at apple.com
Fri Sep 7 21:00:12 PDT 2012


Author: jrose
Date: Fri Sep  7 23:00:12 2012
New Revision: 163453

URL: http://llvm.org/viewvc/llvm-project?rev=163453&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
    cfe/trunk/lib/Analysis/FormatString.cpp
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/Sema/format-strings-non-iso.c
    cfe/trunk/test/Sema/format-strings.c

Modified: cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/FormatString.h?rev=163453&r1=163452&r2=163453&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/FormatString.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/FormatString.h Fri Sep  7 23:00:12 2012
@@ -118,7 +118,7 @@
     cArg,
     dArg,
     iArg,
-    IntArgBeg = cArg, IntArgEnd = iArg,
+    IntArgBeg = dArg, IntArgEnd = iArg,
 
     oArg,
     uArg,
@@ -191,7 +191,9 @@
     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 @@
     : 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 {

Modified: cfe/trunk/lib/Analysis/FormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/FormatString.cpp?rev=163453&r1=163452&r2=163453&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/FormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/FormatString.cpp Fri Sep  7 23:00:12 2012
@@ -723,21 +723,13 @@
 
 llvm::Optional<LengthModifier>
 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<LengthModifier>();

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=163453&r1=163452&r2=163453&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Sep  7 23:00:12 2012
@@ -1941,21 +1941,16 @@
   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::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 @@
   // See if we know how to fix this length modifier.
   llvm::Optional<LengthModifier> 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 @@
       << 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<LengthModifier> 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 @@
                        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 @@
             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 @@
   }
   
   // 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);

Modified: cfe/trunk/test/Sema/format-strings-non-iso.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-non-iso.c?rev=163453&r1=163452&r2=163453&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings-non-iso.c (original)
+++ cfe/trunk/test/Sema/format-strings-non-iso.c Fri Sep  7 23:00:12 2012
@@ -7,8 +7,8 @@
   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 @@
   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}}

Modified: cfe/trunk/test/Sema/format-strings.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=163453&r1=163452&r2=163453&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Fri Sep  7 23:00:12 2012
@@ -117,6 +117,7 @@
   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 at -1{{did you mean to use 'll'?}}
 }
 
 void check_invalid_specifier(FILE* fp, char *buf)





More information about the cfe-commits mailing list