[analyzer] For now, don't inline [cd]tors of C++ containers.

This is a heuristic to make up for the fact that the analyzer doesn't
model C++ containers very well. One example is modeling that
'std::distance(I, E) == 0' implies 'I == E'. In the future, it would be
nice to model this explicitly, but for now it just results in a lot of
false positives.

The actual heuristic checks if the base type has a member named 'begin' or
'iterator'. If so, we treat the constructors and destructors of that type
as opaque, rather than inlining them.

This is intended to drastically reduce the number of false positives
reported with experimental destructor support turned on. We can tweak the
heuristic in the future, but we'd rather err on the side of false negatives
for now.

<rdar://problem/13497258>

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

2 files changed, 63 insertions, 4 deletions

diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 465f408e7f..64e1bae8b9 100644
--- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -134,6 +134,13 @@ bool AnalyzerOptions::mayInlineTemplateFunctions() {
                           /*Default=*/true);
 }
 
+bool AnalyzerOptions::mayInlineCXXContainerCtorsAndDtors() {
+  return getBooleanOption(InlineCXXContainerCtorsAndDtors,
+                          "c++-container-inlining",
+                          /*Default=*/false);
+}
+
+
 bool AnalyzerOptions::mayInlineObjCMethod() {
   return getBooleanOption(ObjCInliningMode,
                           "objc-inlining",
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 67b249c5e9..ee603d728e 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -674,6 +674,53 @@ static CallInlinePolicy mayInlineCallKind(const CallEvent &Call,
   return CIP_Allowed;
 }
 
+/// Returns true if the given C++ class is a container.
+///
+/// Our heuristic for this is whether it contains a method named 'begin()' or a
+/// nested type named 'iterator'.
+static bool isContainerClass(const ASTContext &Ctx, const CXXRecordDecl *RD) {
+  // Don't record any path information.
+  CXXBasePaths Paths(false, false, false);
+
+  const IdentifierInfo &BeginII = Ctx.Idents.get("begin");
+  DeclarationName BeginName = Ctx.DeclarationNames.getIdentifier(&BeginII);
+  DeclContext::lookup_const_result BeginDecls = RD->lookup(BeginName);
+  if (!BeginDecls.empty())
+    return true;
+  if (RD->lookupInBases(&CXXRecordDecl::FindOrdinaryMember,
+                        BeginName.getAsOpaquePtr(),
+                        Paths))
+    return true;
+  
+  const IdentifierInfo &IterII = Ctx.Idents.get("iterator");
+  DeclarationName IteratorName = Ctx.DeclarationNames.getIdentifier(&IterII);
+  DeclContext::lookup_const_result IterDecls = RD->lookup(IteratorName);
+  if (!IterDecls.empty())
+    return true;
+  if (RD->lookupInBases(&CXXRecordDecl::FindOrdinaryMember,
+                        IteratorName.getAsOpaquePtr(),
+                        Paths))
+    return true;
+
+  return false;
+}
+
+/// Returns true if the given function refers to a constructor or destructor of
+/// a C++ container.
+///
+/// We generally do a poor job modeling most containers right now, and would
+/// prefer not to inline their methods.
+static bool isContainerCtorOrDtor(const ASTContext &Ctx,
+                                  const FunctionDecl *FD) {
+  // Heuristic: a type is a container if it contains a "begin()" method
+  // or a type named "iterator".
+  if (!(isa<CXXConstructorDecl>(FD) || isa<CXXDestructorDecl>(FD)))
+    return false;
+
+  const CXXRecordDecl *RD = cast<CXXMethodDecl>(FD)->getParent();
+  return isContainerClass(Ctx, RD);
+}
+
 /// Returns true if the function in \p CalleeADC may be inlined in general.
 ///
 /// This checks static properties of the function, such as its signature and
@@ -681,16 +728,14 @@ static CallInlinePolicy mayInlineCallKind(const CallEvent &Call,
 /// in any context.
 static bool mayInlineDecl(const CallEvent &Call, AnalysisDeclContext *CalleeADC,
                           AnalyzerOptions &Opts) {
-  const Decl *D = CalleeADC->getDecl();
-
   // FIXME: Do not inline variadic calls.
   if (Call.isVariadic())
     return false;
 
   // Check certain C++-related inlining policies.
-  ASTContext &Ctx = D->getASTContext();
+  ASTContext &Ctx = CalleeADC->getASTContext();
   if (Ctx.getLangOpts().CPlusPlus) {
-    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
+    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CalleeADC->getDecl())) {
       // Conditionally control the inlining of template functions.
       if (!Opts.mayInlineTemplateFunctions())
         if (FD->getTemplatedKind() != FunctionDecl::TK_NonTemplate)
@@ -701,6 +746,13 @@ static bool mayInlineDecl(const CallEvent &Call, AnalysisDeclContext *CalleeADC,
         if (Ctx.getSourceManager().isInSystemHeader(FD->getLocation()))
           if (IsInStdNamespace(FD))
             return false;
+
+      // Conditionally control the inlining of methods on objects that look
+      // like C++ containers.
+      if (!Opts.mayInlineCXXContainerCtorsAndDtors())
+        if (!Ctx.getSourceManager().isFromMainFile(FD->getLocation()))
+          if (isContainerCtorOrDtor(Ctx, FD))
+            return false;
     }
   }