Revert r158887. This fixes pr13168.

Revert "If an object (such as a std::string) with an appropriate c_str() member function"

This reverts commit 7d96f6106bfbd85b1af06f34fdbf2834aad0e47e.

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

1 files changed, 120 insertions, 271 deletions

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index ef7dc8819f..b8ed7b52f6 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -16,7 +16,6 @@
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Initialization.h"
-#include "clang/Sema/Lookup.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Analysis/Analyses/FormatString.h"
 #include "clang/AST/ASTContext.h"
@@ -419,91 +418,34 @@ bool Sema::CheckARMBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
   return false;
 }
 
-/// Given a FunctionDecl's FormatAttr, attempts to populate the FomatStringInfo
-/// parameter with the FormatAttr's correct format_idx and firstDataArg.
-/// Returns true when the format fits the function and the FormatStringInfo has
-/// been populated.
-bool Sema::getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember,
-                               FormatStringInfo *FSI) {
-  FSI->HasVAListArg = Format->getFirstArg() == 0;
-  FSI->FormatIdx = Format->getFormatIdx() - 1;
-  FSI->FirstDataArg = FSI->HasVAListArg ? 0 : Format->getFirstArg() - 1;
+/// CheckFunctionCall - Check a direct function call for various correctness
+/// and safety properties not strictly enforced by the C type system.
+bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) {
+  // Get the IdentifierInfo* for the called function.
+  IdentifierInfo *FnInfo = FDecl->getIdentifier();
 
-  // The way the format attribute works in GCC, the implicit this argument
-  // of member functions is counted. However, it doesn't appear in our own
-  // lists, so decrement format_idx in that case.
-  if (IsCXXMember) {
-    if(FSI->FormatIdx == 0)
-      return false;
-    --FSI->FormatIdx;
-    if (FSI->FirstDataArg != 0)
-      --FSI->FirstDataArg;
-  }
-  return true;
-}
+  // None of the checks below are needed for functions that don't have
+  // simple names (e.g., C++ conversion functions).
+  if (!FnInfo)
+    return false;
 
-/// Handles the checks for format strings, non-POD arguments to vararg
-/// functions, and NULL arguments passed to non-NULL parameters.
-void Sema::checkCall(NamedDecl *FDecl, Expr **Args,
-                     unsigned NumArgs,
-                     unsigned NumProtoArgs,
-                     bool IsMemberFunction,
-                     SourceLocation Loc,
-                     SourceRange Range,
-                     VariadicCallType CallType) {
   // FIXME: This mechanism should be abstracted to be less fragile and
   // more efficient. For example, just map function ids to custom
   // handlers.
 
   // Printf and scanf checking.
-  bool HandledFormatString = false;
   for (specific_attr_iterator<FormatAttr>
-         I = FDecl->specific_attr_begin<FormatAttr>(),
-         E = FDecl->specific_attr_end<FormatAttr>(); I != E ; ++I)
-    if (CheckFormatArguments(*I, Args, NumArgs, IsMemberFunction, Loc, Range))
-        HandledFormatString = true;
-
-  // Refuse POD arguments that weren't caught by the format string
-  // checks above.
-  if (!HandledFormatString && CallType != VariadicDoesNotApply)
-    for (unsigned ArgIdx = NumProtoArgs; ArgIdx < NumArgs; ++ArgIdx)
-      variadicArgumentPODCheck(Args[ArgIdx], CallType);
+         i = FDecl->specific_attr_begin<FormatAttr>(),
+         e = FDecl->specific_attr_end<FormatAttr>(); i != e ; ++i) {
+    CheckFormatArguments(*i, TheCall);
+  }
 
   for (specific_attr_iterator<NonNullAttr>
-         I = FDecl->specific_attr_begin<NonNullAttr>(),
-         E = FDecl->specific_attr_end<NonNullAttr>(); I != E; ++I)
-    CheckNonNullArguments(*I, Args, Loc);
-}
-
-/// CheckConstructorCall - Check a constructor call for correctness and safety
-/// properties not enforced by the C type system.
-void Sema::CheckConstructorCall(FunctionDecl *FDecl, Expr **Args,
-                                unsigned NumArgs,
-                                const FunctionProtoType *Proto,
-                                SourceLocation Loc) {
-  VariadicCallType CallType =
-    Proto->isVariadic() ? VariadicConstructor : VariadicDoesNotApply;
-  checkCall(FDecl, Args, NumArgs, Proto->getNumArgs(),
-            /*IsMemberFunction=*/true, Loc, SourceRange(), CallType);
-}
-
-/// CheckFunctionCall - Check a direct function call for various correctness
-/// and safety properties not strictly enforced by the C type system.
-bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
-                             const FunctionProtoType *Proto) {
-  bool IsMemberFunction = isa<CXXMemberCallExpr>(TheCall);
-  VariadicCallType CallType = getVariadicCallType(FDecl, Proto,
-                                                  TheCall->getCallee());
-  unsigned NumProtoArgs = Proto ? Proto->getNumArgs() : 0;
-  checkCall(FDecl, TheCall->getArgs(), TheCall->getNumArgs(), NumProtoArgs,
-            IsMemberFunction, TheCall->getRParenLoc(),
-            TheCall->getCallee()->getSourceRange(), CallType);
-
-  IdentifierInfo *FnInfo = FDecl->getIdentifier();
-  // None of the checks below are needed for functions that don't have
-  // simple names (e.g., C++ conversion functions).
-  if (!FnInfo)
-    return false;
+         i = FDecl->specific_attr_begin<NonNullAttr>(),
+         e = FDecl->specific_attr_end<NonNullAttr>(); i != e; ++i) {
+    CheckNonNullArguments(*i, TheCall->getArgs(),
+                          TheCall->getCallee()->getLocStart());
+  }
 
   unsigned CMId = FDecl->getMemoryFunctionKind();
   if (CMId == 0)
@@ -522,18 +464,25 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
 
 bool Sema::CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation lbrac, 
                                Expr **Args, unsigned NumArgs) {
-  VariadicCallType CallType =
-      Method->isVariadic() ? VariadicMethod : VariadicDoesNotApply;
+  for (specific_attr_iterator<FormatAttr>
+       i = Method->specific_attr_begin<FormatAttr>(),
+       e = Method->specific_attr_end<FormatAttr>(); i != e ; ++i) {
+
+    CheckFormatArguments(*i, Args, NumArgs, false, lbrac, 
+                         Method->getSourceRange());
+  }
 
-  checkCall(Method, Args, NumArgs, Method->param_size(),
-            /*IsMemberFunction=*/false,
-            lbrac, Method->getSourceRange(), CallType);
+  // diagnose nonnull arguments.
+  for (specific_attr_iterator<NonNullAttr>
+       i = Method->specific_attr_begin<NonNullAttr>(),
+       e = Method->specific_attr_end<NonNullAttr>(); i != e; ++i) {
+    CheckNonNullArguments(*i, Args, lbrac);
+  }
 
   return false;
 }
 
-bool Sema::CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall,
-                          const FunctionProtoType *Proto) {
+bool Sema::CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall) {
   const VarDecl *V = dyn_cast<VarDecl>(NDecl);
   if (!V)
     return false;
@@ -542,15 +491,13 @@ bool Sema::CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall,
   if (!Ty->isBlockPointerType())
     return false;
 
-  VariadicCallType CallType = 
-      Proto && Proto->isVariadic() ? VariadicBlock : VariadicDoesNotApply ;
-  unsigned NumProtoArgs = Proto ? Proto->getNumArgs() : 0;
+  // format string checking.
+  for (specific_attr_iterator<FormatAttr>
+       i = NDecl->specific_attr_begin<FormatAttr>(),
+       e = NDecl->specific_attr_end<FormatAttr>(); i != e ; ++i) {
+    CheckFormatArguments(*i, TheCall);
+  }
 
-  checkCall(NDecl, TheCall->getArgs(), TheCall->getNumArgs(),
-            NumProtoArgs, /*IsMemberFunction=*/false,
-            TheCall->getRParenLoc(),
-            TheCall->getCallee()->getSourceRange(), CallType);
-  
   return false;
 }
 
@@ -1554,18 +1501,14 @@ bool Sema::SemaBuiltinLongjmp(CallExpr *TheCall) {
   return false;
 }
 
-// Determine if an expression is a string literal or constant string.
-// If this function returns false on the arguments to a function expecting a
-// format string, we will usually need to emit a warning.
-// True string literals are then checked by CheckFormatString.
-Sema::StringLiteralCheckType
-Sema::checkFormatStringExpr(const Expr *E, Expr **Args,
-                            unsigned NumArgs, bool HasVAListArg,
-                            unsigned format_idx, unsigned firstDataArg,
-                            FormatStringType Type, bool inFunctionCall) {
+// Handle i > 1 ? "x" : "y", recursively.
+bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args,
+                                  unsigned NumArgs, bool HasVAListArg,
+                                  unsigned format_idx, unsigned firstDataArg,
+                                  FormatStringType Type, bool inFunctionCall) {
  tryAgain:
   if (E->isTypeDependent() || E->isValueDependent())
-    return SLCT_NotALiteral;
+    return false;
 
   E = E->IgnoreParenCasts();
 
@@ -1574,26 +1517,18 @@ Sema::checkFormatStringExpr(const Expr *E, Expr **Args,
     // The behavior of printf and friends in this case is implementation
     // dependent.  Ideally if the format string cannot be null then
     // it should have a 'nonnull' attribute in the function prototype.
-    return SLCT_CheckedLiteral;
+    return true;
 
   switch (E->getStmtClass()) {
   case Stmt::BinaryConditionalOperatorClass:
   case Stmt::ConditionalOperatorClass: {
-    // The expression is a literal if both sub-expressions were, and it was
-    // completely checked only if both sub-expressions were checked.
-    const AbstractConditionalOperator *C =
-        cast<AbstractConditionalOperator>(E);
-    StringLiteralCheckType Left =
-        checkFormatStringExpr(C->getTrueExpr(), Args, NumArgs,
-                              HasVAListArg, format_idx, firstDataArg,
-                              Type, inFunctionCall);
-    if (Left == SLCT_NotALiteral)
-      return SLCT_NotALiteral;
-    StringLiteralCheckType Right =
-        checkFormatStringExpr(C->getFalseExpr(), Args, NumArgs,
-                              HasVAListArg, format_idx, firstDataArg,
-                              Type, inFunctionCall);
-    return Left < Right ? Left : Right;
+    const AbstractConditionalOperator *C = cast<AbstractConditionalOperator>(E);
+    return SemaCheckStringLiteral(C->getTrueExpr(), Args, NumArgs, HasVAListArg,
+                                  format_idx, firstDataArg, Type,
+                                  inFunctionCall)
+       && SemaCheckStringLiteral(C->getFalseExpr(), Args, NumArgs, HasVAListArg,
+                                 format_idx, firstDataArg, Type,
+                                 inFunctionCall);
   }
 
   case Stmt::ImplicitCastExprClass: {
@@ -1606,13 +1541,13 @@ Sema::checkFormatStringExpr(const Expr *E, Expr **Args,
       E = src;
       goto tryAgain;
     }
-    return SLCT_NotALiteral;
+    return false;
 
   case Stmt::PredefinedExprClass:
     // While __func__, etc., are technically not string literals, they
     // cannot contain format specifiers and thus are not a security
     // liability.
-    return SLCT_UncheckedLiteral;
+    return true;
       
   case Stmt::DeclRefExprClass: {
     const DeclRefExpr *DR = cast<DeclRefExpr>(E);
@@ -1641,10 +1576,9 @@ Sema::checkFormatStringExpr(const Expr *E, Expr **Args,
             if (InitList->isStringLiteralInit())
               Init = InitList->getInit(0)->IgnoreParenImpCasts();
           }
-          return checkFormatStringExpr(Init, Args, NumArgs,
-                                       HasVAListArg, format_idx,
-                                       firstDataArg, Type,
-                                       /*inFunctionCall*/false);
+          return SemaCheckStringLiteral(Init, Args, NumArgs,
+                                        HasVAListArg, format_idx, firstDataArg,
+                                        Type, /*inFunctionCall*/false);
         }
       }
 
@@ -1678,14 +1612,14 @@ Sema::checkFormatStringExpr(const Expr *E, Expr **Args,
               // We can't pass a 'scanf' string to a 'printf' function.
               if (PVIndex == PVFormat->getFormatIdx() &&
                   Type == GetFormatStringType(PVFormat))
-                return SLCT_UncheckedLiteral;
+                return true;
             }
           }
         }
       }
     }
 
-    return SLCT_NotALiteral;
+    return false;
   }
 
   case Stmt::CallExprClass:
@@ -1699,22 +1633,22 @@ Sema::checkFormatStringExpr(const Expr *E, Expr **Args,
             --ArgIndex;
         const Expr *Arg = CE->getArg(ArgIndex - 1);
 
-        return checkFormatStringExpr(Arg, Args, NumArgs,
-                                     HasVAListArg, format_idx, firstDataArg,
-                                     Type, inFunctionCall);
+        return SemaCheckStringLiteral(Arg, Args, NumArgs, HasVAListArg,
+                                      format_idx, firstDataArg, Type,
+                                      inFunctionCall);
       } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
         unsigned BuiltinID = FD->getBuiltinID();
         if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
             BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
           const Expr *Arg = CE->getArg(0);
-          return checkFormatStringExpr(Arg, Args, NumArgs,
-                                       HasVAListArg, format_idx,
-                                       firstDataArg, Type, inFunctionCall);
+          return SemaCheckStringLiteral(Arg, Args, NumArgs, HasVAListArg,
+                                        format_idx, firstDataArg, Type,
+                                        inFunctionCall);
         }
       }
     }
 
-    return SLCT_NotALiteral;
+    return false;
   }
   case Stmt::ObjCStringLiteralClass:
   case Stmt::StringLiteralClass: {
@@ -1728,14 +1662,14 @@ Sema::checkFormatStringExpr(const Expr *E, Expr **Args,
     if (StrE) {
       CheckFormatString(StrE, E, Args, NumArgs, HasVAListArg, format_idx,
                         firstDataArg, Type, inFunctionCall);
-      return SLCT_CheckedLiteral;
+      return true;
     }
 
-    return SLCT_NotALiteral;
+    return false;
   }
 
   default:
-    return SLCT_NotALiteral;
+    return false;
   }
 }
 
@@ -1766,34 +1700,42 @@ Sema::FormatStringType Sema::GetFormatStringType(const FormatAttr *Format) {
 
 /// CheckPrintfScanfArguments - Check calls to printf and scanf (and similar
 /// functions) for correct use of format strings.
-/// Returns true if a format string has been fully checked.
-bool Sema::CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall) {
-  bool IsCXXMember = isa<CXXMemberCallExpr>(TheCall);
-  return CheckFormatArguments(Format, TheCall->getArgs(),
-                              TheCall->getNumArgs(),
-                              IsCXXMember, TheCall->getRParenLoc(), 
-                              TheCall->getCallee()->getSourceRange());
+void Sema::CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall) {
+  bool IsCXXMember = false;
+  // The way the format attribute works in GCC, the implicit this argument
+  // of member functions is counted. However, it doesn't appear in our own
+  // lists, so decrement format_idx in that case.
+  IsCXXMember = isa<CXXMemberCallExpr>(TheCall);
+  CheckFormatArguments(Format, TheCall->getArgs(), TheCall->getNumArgs(),
+                       IsCXXMember, TheCall->getRParenLoc(), 
+                       TheCall->getCallee()->getSourceRange());
 }
 
-bool Sema::CheckFormatArguments(const FormatAttr *Format, Expr **Args,
+void Sema::CheckFormatArguments(const FormatAttr *Format, Expr **Args,
                                 unsigned NumArgs, bool IsCXXMember,
                                 SourceLocation Loc, SourceRange Range) {
-  FormatStringInfo FSI;
-  if (getFormatStringInfo(Format, IsCXXMember, &FSI))
-    return CheckFormatArguments(Args, NumArgs, FSI.HasVAListArg, FSI.FormatIdx,
-                                FSI.FirstDataArg, GetFormatStringType(Format),
-                                Loc, Range);
-  return false;
+  bool HasVAListArg = Format->getFirstArg() == 0;
+  unsigned format_idx = Format->getFormatIdx() - 1;
+  unsigned firstDataArg = HasVAListArg ? 0 : Format->getFirstArg() - 1;
+  if (IsCXXMember) {
+    if (format_idx == 0)
+      return;
+    --format_idx;
+    if(firstDataArg != 0)
+      --firstDataArg;
+  }
+  CheckFormatArguments(Args, NumArgs, HasVAListArg, format_idx,
+                       firstDataArg, GetFormatStringType(Format), Loc, Range);
 }
 
-bool Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
+void Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
                                 bool HasVAListArg, unsigned format_idx,
                                 unsigned firstDataArg, FormatStringType Type,
                                 SourceLocation Loc, SourceRange Range) {
   // CHECK: printf/scanf-like function is called with no format string.
   if (format_idx >= NumArgs) {
     Diag(Loc, diag::warn_missing_format_string) << Range;
-    return false;
+    return;
   }
 
   const Expr *OrigFormatExpr = Args[format_idx]->IgnoreParenCasts();
@@ -1810,17 +1752,14 @@ bool Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
   // C string (e.g. "%d")
   // ObjC string uses the same format specifiers as C string, so we can use
   // the same format string checking logic for both ObjC and C strings.
-  StringLiteralCheckType CT =
-      checkFormatStringExpr(OrigFormatExpr, Args, NumArgs, HasVAListArg,
-                            format_idx, firstDataArg, Type);
-  if (CT != SLCT_NotALiteral)
-    // Literal format string found, check done!
-    return CT == SLCT_CheckedLiteral;
+  if (SemaCheckStringLiteral(OrigFormatExpr, Args, NumArgs, HasVAListArg,
+                             format_idx, firstDataArg, Type))
+    return;  // Literal format string found, check done!
 
   // Strftime is particular as it always uses a single 'time' argument,
   // so it is safe to pass a non-literal string.
   if (Type == FST_Strftime)
-    return false;
+    return;
 
   // Do not emit diag when the string param is a macro expansion and the
   // format is either NSString or CFString. This is a hack to prevent
@@ -1828,7 +1767,7 @@ bool Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
   // which are usually used in place of NS and CF string literals.
   if (Type == FST_NSString &&
       SourceMgr.isInSystemMacro(Args[format_idx]->getLocStart()))
-    return false;
+    return;
 
   // If there are no arguments specified, warn with -Wformat-security, otherwise
   // warn only with -Wformat-nonliteral.
@@ -1840,7 +1779,6 @@ bool Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
     Diag(Args[format_idx]->getLocStart(),
          diag::warn_format_nonliteral)
            << OrigFormatExpr->getSourceRange();
-  return false;
 }
 
 namespace {
@@ -2200,11 +2138,7 @@ public:
   bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
                              const char *startSpecifier,
                              unsigned specifierLen);
-  bool checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
-                       const char *StartSpecifier,
-                       unsigned SpecifierLen,
-                       const Expr *E);
-
+  
   bool HandleAmount(const analyze_format_string::OptionalAmount &Amt, unsigned k,
                     const char *startSpecifier, unsigned specifierLen);
   void HandleInvalidAmount(const analyze_printf::PrintfSpecifier &FS,
@@ -2218,9 +2152,6 @@ public:
                          const analyze_printf::OptionalFlag &ignoredFlag,
                          const analyze_printf::OptionalFlag &flag,
                          const char *startSpecifier, unsigned specifierLen);
-  bool checkForCStrMembers(const analyze_printf::ArgTypeResult &ATR,
-                           const Expr *E, const CharSourceRange &CSR);
-
 };  
 }
 
@@ -2338,64 +2269,6 @@ void CheckPrintfHandler::HandleIgnoredFlag(
                          getSpecifierRange(ignoredFlag.getPosition(), 1)));
 }
 
-// Determines if the specified is a C++ class or struct containing
-// a member with the specified name and kind (e.g. a CXXMethodDecl named
-// "c_str()").
-template<typename MemberKind>
-static llvm::SmallPtrSet<MemberKind*, 1>
-CXXRecordMembersNamed(StringRef Name, Sema &S, QualType Ty) {
-  const RecordType *RT = Ty->getAs<RecordType>();
-  llvm::SmallPtrSet<MemberKind*, 1> Results;
-
-  if (!RT)
-    return Results;
-  const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl());
-  if (!RD)
-    return Results;
-
-  LookupResult R(S, &S.PP.getIdentifierTable().get(Name), SourceLocation(),
-                 Sema::LookupMemberName);
-
-  // We just need to include all members of the right kind turned up by the
-  // filter, at this point.
-  if (S.LookupQualifiedName(R, RT->getDecl()))
-    for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) {
-      NamedDecl *decl = (*I)->getUnderlyingDecl();
-      if (MemberKind *FK = dyn_cast<MemberKind>(decl))
-        Results.insert(FK);
-    }
-  return Results;
-}
-
-// Check if a (w)string was passed when a (w)char* was needed, and offer a
-// better diagnostic if so. ATR is assumed to be valid.
-// Returns true when a c_str() conversion method is found.
-bool CheckPrintfHandler::checkForCStrMembers(
-    const analyze_printf::ArgTypeResult &ATR, const Expr *E,
-    const CharSourceRange &CSR) {
-  typedef llvm::SmallPtrSet<CXXMethodDecl*, 1> MethodSet;
-
-  MethodSet Results =
-      CXXRecordMembersNamed<CXXMethodDecl>("c_str", S, E->getType());
-
-  for (MethodSet::iterator MI = Results.begin(), ME = Results.end();
-       MI != ME; ++MI) {
-    const CXXMethodDecl *Method = *MI;
-    if (Method->getNumParams() == 0 &&
-          ATR.matchesType(S.Context, Method->getResultType())) {
-      // FIXME: Suggest parens if the expression needs them.
-      SourceLocation EndLoc =
-          S.getPreprocessor().getLocForEndOfToken(E->getLocEnd());
-      S.Diag(E->getLocStart(), diag::note_printf_c_str)
-          << "c_str()"
-          << FixItHint::CreateInsertion(EndLoc, ".c_str()");
-      return true;
-    }
-  }
-
-  return false;
-}
-
 bool
 CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
                                             &FS,
@@ -2523,30 +2396,20 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
   if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex))
     return false;
 
-  return checkFormatExpr(FS, startSpecifier, specifierLen,
-                         getDataArg(argIndex));
-}
-
-bool
-CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
-                                    const char *StartSpecifier,
-                                    unsigned SpecifierLen,
-                                    const Expr *E) {
-  using namespace analyze_format_string;
-  using namespace analyze_printf;
   // 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,
                                                            ObjCContext);
-  if (ATR.isValid() && !ATR.matchesType(S.Context, E->getType())) {
+  if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) {
     // Look through argument promotions for our error message's reported type.
     // This includes the integral and floating promotions, but excludes array
     // and function pointer decay; seeing that an argument intended to be a
     // string has type 'char [6]' is probably more confusing than 'char *'.
-    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
+    if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Ex)) {
       if (ICE->getCastKind() == CK_IntegralCast ||
           ICE->getCastKind() == CK_FloatingCast) {
-        E = ICE->getSubExpr();
+        Ex = ICE->getSubExpr();
 
         // 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
@@ -2554,7 +2417,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
         if (ICE->getType() == S.Context.IntTy ||
             ICE->getType() == S.Context.UnsignedIntTy) {
           // All further checking is done on the subexpression.
-          if (ATR.matchesType(S.Context, E->getType()))
+          if (ATR.matchesType(S.Context, Ex->getType()))
             return true;
         }
       }
@@ -2562,7 +2425,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
 
     // We may be able to offer a FixItHint if it is a supported type.
     PrintfSpecifier fixedFS = FS;
-    bool success = fixedFS.fixType(E->getType(), S.getLangOpts(),
+    bool success = fixedFS.fixType(Ex->getType(), S.getLangOpts(),
                                    S.Context, ObjCContext);
 
     if (success) {
@@ -2573,38 +2436,24 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
 
       EmitFormatDiagnostic(
         S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
-          << ATR.getRepresentativeTypeName(S.Context) << E->getType()
-          << E->getSourceRange(),
-        E->getLocStart(),
+          << ATR.getRepresentativeTypeName(S.Context) << Ex->getType()
+          << Ex->getSourceRange(),
+        Ex->getLocStart(),
         /*IsStringLocation*/false,
-        getSpecifierRange(StartSpecifier, SpecifierLen),
+        getSpecifierRange(startSpecifier, specifierLen),
         FixItHint::CreateReplacement(
-          getSpecifierRange(StartSpecifier, SpecifierLen),
+          getSpecifierRange(startSpecifier, specifierLen),
           os.str()));
-    } else {
-      const CharSourceRange &CSR = getSpecifierRange(StartSpecifier,
-                                                     SpecifierLen);
-      // Since the warning for passing non-POD types to variadic functions
-      // was deferred until now, we emit a warning for non-POD
-      // arguments here.
-      if (S.isValidVarArgType(E->getType()) == Sema::VAK_Invalid) {
-        EmitFormatDiagnostic(
-          S.PDiag(diag::warn_non_pod_vararg_with_format_string)
-            << S.getLangOpts().CPlusPlus0x
-            << E->getType()
-            << ATR.getRepresentativeTypeName(S.Context)
-            << CSR
-            << E->getSourceRange(),
-          E->getLocStart(), /*IsStringLocation*/false, CSR);
-
-        checkForCStrMembers(ATR, E, CSR);
-      } else
-        EmitFormatDiagnostic(
-          S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
-            << ATR.getRepresentativeTypeName(S.Context) << E->getType()
-            << CSR
-            << E->getSourceRange(),
-          E->getLocStart(), /*IsStringLocation*/false, CSR);
+    }
+    else {
+      EmitFormatDiagnostic(
+        S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
+          << ATR.getRepresentativeTypeName(S.Context) << Ex->getType()
+          << getSpecifierRange(startSpecifier, specifierLen)
+          << Ex->getSourceRange(),
+        Ex->getLocStart(),
+        /*IsStringLocation*/false,
+        getSpecifierRange(startSpecifier, specifierLen));
     }
   }