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".

llvm-svn: 150578

3 files changed, 22 insertions, 14 deletions

diff --git a/clang/lib/Analysis/PrintfFormatString.cpp b/clang/lib/Analysis/PrintfFormatString.cpp
index 6da37fc44ab..ff0174e3c3e 100644
--- a/clang/lib/Analysis/PrintfFormatString.cpp
+++ b/clang/lib/Analysis/PrintfFormatString.cpp
@@ -336,7 +336,8 @@ ArgTypeResult PrintfSpecifier::getArgType(ASTContext &Ctx,
   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 @@ bool PrintfSpecifier::fixType(QualType QT, const LangOptions &LangOpt) {
     }
   }
 
+  // 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 @@ bool PrintfSpecifier::fixType(QualType QT, const LangOptions &LangOpt) {
     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 {
diff --git a/clang/lib/Analysis/ScanfFormatString.cpp b/clang/lib/Analysis/ScanfFormatString.cpp
index c1cdef86327..5990a56c35c 100644
--- a/clang/lib/Analysis/ScanfFormatString.cpp
+++ b/clang/lib/Analysis/ScanfFormatString.cpp
@@ -307,8 +307,8 @@ ScanfArgTypeResult ScanfSpecifier::getArgType(ASTContext &Ctx) const {
   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 @@ bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt)
     }
   }
 
+  // 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;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3393cf73f1c..1d75ef6e6f5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2181,7 +2181,8 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
 
     // 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 @@ bool CheckScanfHandler::HandleScanfSpecifier(
   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.