Revert r109428 "Hoist argument type checking into CheckFormatHandler. This is prep for scanf format"

Got errors about ASTContext being undefined with Visual Studio 2010.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@109491 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Analysis/Analyses/FormatString.h

@@ -305,8 +305,6 @@ protected:
 public:
   FormatSpecifier(bool isPrintf)
     : CS(isPrintf), UsesPositionalArg(false), argIndex(0) {}
-  
-  virtual ~FormatSpecifier();
 
   void setLengthModifier(LengthModifier lm) {
     LM = lm;
@@ -341,17 +339,6 @@ public:
   bool usesPositionalArg() const { return UsesPositionalArg; }
   
   bool hasValidLengthModifier() const;
-  
-  /// \brief Returns the type that a data argument
-  /// paired with this format specifier should have.  This method
-  /// will an invalid ArgTypeResult if the format specifier does not have
-  /// a matching data argument or the matching argument matches
-  /// more than one type.
-  virtual ArgTypeResult getArgType(ASTContext &Ctx) const = 0;
-  
-  const ConversionSpecifier &getConversionSpecifier() const {
-    return CS;
-  }
 };
 
 } // end analyze_format_string namespace
@@ -451,9 +438,9 @@ public:
     return getConversionSpecifier().consumesDataArgument();
   }
 
-  /// \brief Returns the type that a data argument
+  /// \brief Returns the builtin type that a data argument
   /// paired with this format specifier should have.  This method
-  /// will an invalid ArgTypeResult if the format specifier does not have
+  /// will return null if the format specifier does not have
   /// a matching data argument or the matching argument matches
   /// more than one type.
   ArgTypeResult getArgType(ASTContext &Ctx) const;
@@ -481,11 +468,6 @@ public:
 
   bool hasValidPrecision() const;
   bool hasValidFieldWidth() const;
-  
-  static bool classof(const analyze_format_string::FormatSpecifier *FS) {
-    return FS->getConversionSpecifier().isPrintfKind();
-  }
-  
 };
 }  // end analyze_printf namespace
 
@@ -510,7 +492,6 @@ public:
   }      
 };
 
-using analyze_format_string::ArgTypeResult;
 using analyze_format_string::LengthModifier;
 using analyze_format_string::OptionalAmount;
 using analyze_format_string::OptionalFlag;
@@ -542,13 +523,6 @@ public:
   bool consumesDataArgument() const {
     return CS.consumesDataArgument() && !SuppressAssignment;
   }
-  
-  /// \brief Returns the type that a data argument
-  /// paired with this format specifier should have.  This method
-  /// will an invalid ArgTypeResult if the format specifier does not have
-  /// a matching data argument or the matching argument matches
-  /// more than one type.
-  ArgTypeResult getArgType(ASTContext &Ctx) const;
 
   static ScanfSpecifier Parse(const char *beg, const char *end);
 };

lib/Analysis/FormatString.cpp

@@ -379,11 +379,9 @@ void OptionalAmount::toString(llvm::raw_ostream &os) const {
 }
 
 //===----------------------------------------------------------------------===//
-// Methods on FormatSpecifier.
+// Methods on ConversionSpecifier.
 //===----------------------------------------------------------------------===//
 
-FormatSpecifier::~FormatSpecifier() {}
-
 bool FormatSpecifier::hasValidLengthModifier() const {
   switch (LM.getKind()) {
     case LengthModifier::None:

lib/Analysis/ScanfFormatString.cpp

@@ -218,10 +218,4 @@ bool clang::analyze_format_string::ParseScanfString(FormatStringHandler &H,
   return false;
 }
 
-ArgTypeResult ScanfSpecifier::getArgType(ASTContext &Ctx) const {
-  // FIXME: Fill in.
-  return ArgTypeResult();
-}
-
-
 

lib/Sema/SemaChecking.cpp

@@ -1174,11 +1174,6 @@ protected:
                     const analyze_format_string::ConversionSpecifier &CS,
                     const char *startSpecifier, unsigned specifierLen,
                     unsigned argIndex);
-  
-  void CheckArgType(const analyze_format_string::FormatSpecifier &FS,
-                    const analyze_format_string::ConversionSpecifier &CS,
-                    const char *startSpecifier, unsigned specifierLen,
-                    unsigned argIndex);
 };
 }
 
@@ -1304,52 +1299,6 @@ CheckFormatHandler::CheckNumArgs(
   return true;
 }
 
-void CheckFormatHandler::CheckArgType(
-  const analyze_format_string::FormatSpecifier &FS,
-  const analyze_format_string::ConversionSpecifier &CS,
-  const char *startSpecifier, unsigned specifierLen, unsigned argIndex) {
-  
-  const Expr *Ex = getDataArg(argIndex);
-  const analyze_format_string::ArgTypeResult &ATR = FS.getArgType(S.Context);
-
-  if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) {
-    // Check if we didn't match because of an implicit cast from a 'char'
-    // or 'short' to an 'int'.  This is done because scanf/printf are varargs
-    // functions.
-    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Ex))
-      if (ICE->getType() == S.Context.IntTy)
-        if (ATR.matchesType(S.Context, ICE->getSubExpr()->getType()))
-          return;
-    
-    if (const analyze_printf::PrintfSpecifier *PFS =
-        dyn_cast<analyze_printf::PrintfSpecifier>(&FS)) {
-      // We may be able to offer a FixItHint if it is a supported type.
-      analyze_printf::PrintfSpecifier fixedFS(*PFS);
-      if (fixedFS.fixType(Ex->getType())) {
-        // Get the fix string from the fixed format specifier
-        llvm::SmallString<128> buf;
-        llvm::raw_svector_ostream os(buf);
-        fixedFS.toString(os);
-      
-        S.Diag(getLocationOfByte(CS.getStart()),
-               diag::warn_printf_conversion_argument_type_mismatch)
-          << ATR.getRepresentativeType(S.Context) << Ex->getType()
-          << getSpecifierRange(startSpecifier, specifierLen)
-        << Ex->getSourceRange()
-        << FixItHint::CreateReplacement(
-                getSpecifierRange(startSpecifier, specifierLen), os.str());
-      }
-      else {
-        S.Diag(getLocationOfByte(CS.getStart()),
-               diag::warn_printf_conversion_argument_type_mismatch)
-          << ATR.getRepresentativeType(S.Context) << Ex->getType()
-          << getSpecifierRange(startSpecifier, specifierLen)
-          << Ex->getSourceRange();
-      }
-    }
-  }
-}
-
 //===--- CHECK: Printf format string checking ------------------------------===//
 
 namespace {
@@ -1621,8 +1570,47 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
   if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex))
     return false;
 
-  CheckArgType(FS, CS, startSpecifier, specifierLen, argIndex);
-  
+  // Now type check the data expression that matches the
+  // format specifier.
+  const Expr *Ex = getDataArg(argIndex);
+  const analyze_printf::ArgTypeResult &ATR = FS.getArgType(S.Context);
+  if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) {
+    // Check if we didn't match because of an implicit cast from a 'char'
+    // or 'short' to an 'int'.  This is done because printf is a varargs
+    // function.
+    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Ex))
+      if (ICE->getType() == S.Context.IntTy)
+        if (ATR.matchesType(S.Context, ICE->getSubExpr()->getType()))
+          return true;
+
+    // We may be able to offer a FixItHint if it is a supported type.
+    PrintfSpecifier fixedFS = FS;
+    bool success = fixedFS.fixType(Ex->getType());
+
+    if (success) {
+      // Get the fix string from the fixed format specifier
+      llvm::SmallString<128> buf;
+      llvm::raw_svector_ostream os(buf);
+      fixedFS.toString(os);
+
+      S.Diag(getLocationOfByte(CS.getStart()),
+          diag::warn_printf_conversion_argument_type_mismatch)
+        << ATR.getRepresentativeType(S.Context) << Ex->getType()
+        << getSpecifierRange(startSpecifier, specifierLen)
+        << Ex->getSourceRange()
+        << FixItHint::CreateReplacement(
+            getSpecifierRange(startSpecifier, specifierLen),
+            os.str());
+    }
+    else {
+      S.Diag(getLocationOfByte(CS.getStart()),
+             diag::warn_printf_conversion_argument_type_mismatch)
+        << ATR.getRepresentativeType(S.Context) << Ex->getType()
+        << getSpecifierRange(startSpecifier, specifierLen)
+        << Ex->getSourceRange();
+    }
+  }
+
   return true;
 }