[cfe-commits] r150578 - in /cfe/trunk: include/clang/Analysis/Analyses/FormatString.h lib/Analysis/PrintfFormatString.cpp lib/Analysis/ScanfFormatString.cpp lib/Sema/SemaChecking.cpp test/Sema/format-strings-fixit.c

Hans Wennborg hans at hanshq.net
Wed Feb 15 01:59:46 PST 2012


Author: hans
Date: Wed Feb 15 03:59:46 2012
New Revision: 150578

URL: http://llvm.org/viewvc/llvm-project?rev=150578&view=rev
Log:
Make -Wformat fix-its preserve original conversion specifiers.

This commit makes PrintfSpecifier::fixType() and ScanfSpecifier::fixType()
only fix a conversion specification enough that Clang wouldn't warn about it,
as opposed to always changing it to use the "canonical" conversion specifier.
(PR11975)

This preserves the user's choice of conversion specifier in cases like:

printf("%a", (long double)1);
where we previously suggested "%Lf", we now suggest "%La"

printf("%x", (long)1);
where we previously suggested "%ld", we now suggest "%lx".

Modified:
    cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
    cfe/trunk/lib/Analysis/PrintfFormatString.cpp
    cfe/trunk/lib/Analysis/ScanfFormatString.cpp
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/Sema/format-strings-fixit.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=150578&r1=150577&r2=150578&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/FormatString.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/FormatString.h Wed Feb 15 03:59:46 2012
@@ -467,10 +467,11 @@
   const OptionalFlag &hasSpacePrefix() const { return HasSpacePrefix; }
   bool usesPositionalArg() const { return UsesPositionalArg; }
 
-    /// Changes the specifier and length according to a QualType, retaining any
-    /// flags or options. Returns true on success, or false when a conversion
-    /// was not successful.
-  bool fixType(QualType QT, const LangOptions &LangOpt);
+  /// Changes the specifier and length according to a QualType, retaining any
+  /// flags or options. Returns true on success, or false when a conversion
+  /// was not successful.
+  bool fixType(QualType QT, const LangOptions &LangOpt, ASTContext &Ctx,
+               bool IsObjCLiteral);
 
   void toString(raw_ostream &os) const;
 
@@ -567,7 +568,7 @@
 
   ScanfArgTypeResult getArgType(ASTContext &Ctx) const;
 
-  bool fixType(QualType QT, const LangOptions &LangOpt);
+  bool fixType(QualType QT, const LangOptions &LangOpt, ASTContext &Ctx);
 
   void toString(raw_ostream &os) const;
 

Modified: cfe/trunk/lib/Analysis/PrintfFormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PrintfFormatString.cpp?rev=150578&r1=150577&r2=150578&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/PrintfFormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/PrintfFormatString.cpp Wed Feb 15 03:59:46 2012
@@ -336,7 +336,8 @@
   return ArgTypeResult();
 }
 
-bool PrintfSpecifier::fixType(QualType QT, const LangOptions &LangOpt) {
+bool PrintfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
+                              ASTContext &Ctx, bool IsObjCLiteral) {
   // Handle strings first (char *, wchar_t *)
   if (QT->isPointerType() && (QT->getPointeeType()->isAnyCharacterType())) {
     CS.setKind(ConversionSpecifier::sArg);
@@ -432,6 +433,11 @@
     }
   }
 
+  // If fixing the length modifier was enough, we are done.
+  const analyze_printf::ArgTypeResult &ATR = getArgType(Ctx, IsObjCLiteral);
+  if (hasValidLengthModifier() && ATR.isValid() && ATR.matchesType(Ctx, QT))
+    return true;
+
   // Set conversion specifier and disable any flags which do not apply to it.
   // Let typedefs to char fall through to int, as %c is silly for uint8_t.
   if (isa<TypedefType>(QT) && QT->isAnyCharacterType()) {
@@ -451,9 +457,7 @@
     HasAlternativeForm = 0;
   }
   else if (QT->isUnsignedIntegerType()) {
-    // Preserve the original formatting, e.g. 'X', 'o'.
-    if (!cast<PrintfConversionSpecifier>(CS).isUIntArg())
-      CS.setKind(ConversionSpecifier::uArg);
+    CS.setKind(ConversionSpecifier::uArg);
     HasAlternativeForm = 0;
     HasPlusPrefix = 0;
   } else {

Modified: cfe/trunk/lib/Analysis/ScanfFormatString.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ScanfFormatString.cpp?rev=150578&r1=150577&r2=150578&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ScanfFormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/ScanfFormatString.cpp Wed Feb 15 03:59:46 2012
@@ -307,8 +307,8 @@
   return ScanfArgTypeResult();
 }
 
-bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt)
-{
+bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
+                             ASTContext &Ctx) {
   if (!QT->isPointerType())
     return false;
 
@@ -390,17 +390,19 @@
     }
   }
 
+  // If fixing the length modifier was enough, we are done.
+  const analyze_scanf::ScanfArgTypeResult &ATR = getArgType(Ctx);
+  if (hasValidLengthModifier() && ATR.isValid() && ATR.matchesType(Ctx, QT))
+    return true;
+
   // Figure out the conversion specifier.
   if (PT->isRealFloatingType())
     CS.setKind(ConversionSpecifier::fArg);
   else if (PT->isSignedIntegerType())
     CS.setKind(ConversionSpecifier::dArg);
-  else if (PT->isUnsignedIntegerType()) {
-    // Preserve the original formatting, e.g. 'X', 'o'.
-    if (!CS.isUIntArg()) {
-      CS.setKind(ConversionSpecifier::uArg);
-    }
-  } else
+  else if (PT->isUnsignedIntegerType())
+    CS.setKind(ConversionSpecifier::uArg);
+  else
     llvm_unreachable("Unexpected type");
 
   return true;

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=150578&r1=150577&r2=150578&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Feb 15 03:59:46 2012
@@ -2181,7 +2181,8 @@
 
     // We may be able to offer a FixItHint if it is a supported type.
     PrintfSpecifier fixedFS = FS;
-    bool success = fixedFS.fixType(Ex->getType(), S.getLangOptions());
+    bool success = fixedFS.fixType(Ex->getType(), S.getLangOptions(),
+                                   S.Context, IsObjCLiteral);
 
     if (success) {
       // Get the fix string from the fixed format specifier
@@ -2340,7 +2341,8 @@
   const analyze_scanf::ScanfArgTypeResult &ATR = FS.getArgType(S.Context);
   if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) {
     ScanfSpecifier fixedFS = FS;
-    bool success = fixedFS.fixType(Ex->getType(), S.getLangOptions());
+    bool success = fixedFS.fixType(Ex->getType(), S.getLangOptions(),
+                                   S.Context);
 
     if (success) {
       // Get the fix string from the fixed format specifier.

Modified: cfe/trunk/test/Sema/format-strings-fixit.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-fixit.c?rev=150578&r1=150577&r2=150578&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings-fixit.c (original)
+++ cfe/trunk/test/Sema/format-strings-fixit.c Wed Feb 15 03:59:46 2012
@@ -48,7 +48,7 @@
   printf("%hhs", "foo");
   printf("%1$zp", (void *)0);
 
-  // Perserve the original formatting for unsigned integers.
+  // Preserve the original formatting for unsigned integers.
   unsigned long val = 42;
   printf("%X", val);
 
@@ -60,6 +60,21 @@
 
   // string
   printf("%ld", "foo");
+
+  // Preserve the original choice of conversion specifier.
+  printf("%o", (long) 42);
+  printf("%u", (long) 42);
+  printf("%x", (long) 42);
+  printf("%X", (long) 42);
+  printf("%i", (unsigned long) 42);
+  printf("%d", (unsigned long) 42);
+  printf("%F", (long double) 42);
+  printf("%e", (long double) 42);
+  printf("%E", (long double) 42);
+  printf("%g", (long double) 42);
+  printf("%G", (long double) 42);
+  printf("%a", (long double) 42);
+  printf("%A", (long double) 42);
 }
 
 int scanf(char const *, ...);
@@ -101,20 +116,30 @@
   scanf("%f", &uIntmaxVar);
   scanf("%f", &ptrdiffVar);
 
-  // Perserve the original formatting for unsigned integers.
-  scanf("%o", &uLongVar);
-  scanf("%x", &uLongVar);
-  scanf("%X", &uLongVar);
+  // Preserve the original formatting.
+  scanf("%o", &longVar);
+  scanf("%u", &longVar);
+  scanf("%x", &longVar);
+  scanf("%X", &longVar);
+  scanf("%i", &uLongVar);
+  scanf("%d", &uLongVar);
+  scanf("%F", &longDoubleVar);
+  scanf("%e", &longDoubleVar);
+  scanf("%E", &longDoubleVar);
+  scanf("%g", &longDoubleVar);
+  scanf("%G", &longDoubleVar);
+  scanf("%a", &longDoubleVar);
+  scanf("%A", &longDoubleVar);
 }
 
-// Validate the fixes...
+// Validate the fixes.
 // CHECK: printf("%d", (int) 123);
 // CHECK: printf("abc%s", "testing testing 123");
-// CHECK: printf("%ld", (long) -12);
+// CHECK: printf("%lu", (long) -12);
 // CHECK: printf("%d", 123);
 // CHECK: printf("%s\n", "x");
 // CHECK: printf("%f\n", 1.23);
-// CHECK: printf("%.2llu", (unsigned long long) 123456);
+// CHECK: printf("%+.2lld", (unsigned long long) 123456);
 // CHECK: printf("%1Lf", (long double) 1.23);
 // CHECK: printf("%0u", (unsigned) 31337);
 // CHECK: printf("%p", (void *) 0);
@@ -132,6 +157,19 @@
 // CHECK: printf("%ju", (uintmax_t) 42);
 // CHECK: printf("%td", (ptrdiff_t) 42);
 // CHECK: printf("%s", "foo");
+// CHECK: printf("%lo", (long) 42);
+// CHECK: printf("%lu", (long) 42);
+// CHECK: printf("%lx", (long) 42);
+// CHECK: printf("%lX", (long) 42);
+// CHECK: printf("%li", (unsigned long) 42);
+// CHECK: printf("%ld", (unsigned long) 42);
+// CHECK: printf("%LF", (long double) 42);
+// CHECK: printf("%Le", (long double) 42);
+// CHECK: printf("%LE", (long double) 42);
+// CHECK: printf("%Lg", (long double) 42);
+// CHECK: printf("%LG", (long double) 42);
+// CHECK: printf("%La", (long double) 42);
+// CHECK: printf("%LA", (long double) 42);
 
 // CHECK: scanf("%s", str);
 // CHECK: scanf("%hd", &shortVar);
@@ -149,6 +187,16 @@
 // CHECK: scanf("%jd", &intmaxVar);
 // CHECK: scanf("%ju", &uIntmaxVar);
 // CHECK: scanf("%td", &ptrdiffVar);
-// CHECK: scanf("%lo", &uLongVar);
-// CHECK: scanf("%lx", &uLongVar);
-// CHECK: scanf("%lX", &uLongVar);
+// CHECK: scanf("%lo", &longVar);
+// CHECK: scanf("%lu", &longVar);
+// CHECK: scanf("%lx", &longVar);
+// CHECK: scanf("%lX", &longVar);
+// CHECK: scanf("%li", &uLongVar);
+// CHECK: scanf("%ld", &uLongVar);
+// CHECK: scanf("%LF", &longDoubleVar);
+// CHECK: scanf("%Le", &longDoubleVar);
+// CHECK: scanf("%LE", &longDoubleVar);
+// CHECK: scanf("%Lg", &longDoubleVar);
+// CHECK: scanf("%LG", &longDoubleVar);
+// CHECK: scanf("%La", &longDoubleVar);
+// CHECK: scanf("%LA", &longDoubleVar);





More information about the cfe-commits mailing list