r180890 - [analyzer] Don't inline the [cd]tors of C++ iterators.

[Jordan Rose]

Author: jrose
Date: Wed May  1 17:39:31 2013
New Revision: 180890

URL: http://llvm.org/viewvc/llvm-project?rev=180890&view=rev
Log:
[analyzer] Don't inline the [cd]tors of C++ iterators.

This goes with r178516, which instructed the analyzer not to inline the
constructors and destructors of C++ container classes. This goes a step
further and does the same thing for iterators, so that the analyzer won't
falsely decide we're trying to construct an iterator pointing to a
nonexistent element.

The heuristic for determining whether something is an iterator is the
presence of an 'iterator_category' member. This is controlled under the
same -analyzer-config option as container constructor/destructor inlining:
'c++-container-inlining'.

<rdar://problem/13770187>

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
    cfe/trunk/test/Analysis/inlining/containers.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=180890&r1=180889&r2=180890&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Wed May  1 17:39:31 2013
@@ -676,46 +676,40 @@ static CallInlinePolicy mayInlineCallKin
   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())
+/// Returns true if the given C++ class contains a member with the given name.
+static bool hasMember(const ASTContext &Ctx, const CXXRecordDecl *RD,
+                      StringRef Name) {
+  const IdentifierInfo &II = Ctx.Idents.get(Name);
+  DeclarationName DeclName = Ctx.DeclarationNames.getIdentifier(&II);
+  if (!RD->lookup(DeclName).empty())
     return true;
+
+  CXXBasePaths Paths(false, false, false);
   if (RD->lookupInBases(&CXXRecordDecl::FindOrdinaryMember,
-                        IteratorName.getAsOpaquePtr(),
+                        DeclName.getAsOpaquePtr(),
                         Paths))
     return true;
 
   return false;
 }
 
+/// Returns true if the given C++ class is a container or iterator.
+///
+/// Our heuristic for this is whether it contains a method named 'begin()' or a
+/// nested type named 'iterator' or 'iterator_category'.
+static bool isContainerClass(const ASTContext &Ctx, const CXXRecordDecl *RD) {
+  return hasMember(Ctx, RD, "begin") ||
+         hasMember(Ctx, RD, "iterator") ||
+         hasMember(Ctx, RD, "iterator_category");
+}
+
 /// Returns true if the given function refers to a constructor or destructor of
-/// a C++ container.
+/// a C++ container or iterator.
 ///
 /// We generally do a poor job modeling most containers right now, and would
-/// prefer not to inline their methods.
+/// prefer not to inline their setup and teardown.
 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;
 

Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h?rev=180890&r1=180889&r2=180890&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h Wed May  1 17:39:31 2013
@@ -80,6 +80,12 @@ namespace std {
       *OI++ = *II++;
     return OI;
   }
+
+  struct input_iterator_tag { };
+  struct output_iterator_tag { };
+  struct forward_iterator_tag : public input_iterator_tag { };
+  struct bidirectional_iterator_tag : public forward_iterator_tag { };
+  struct random_access_iterator_tag : public bidirectional_iterator_tag { };
 }
 
 void* operator new(std::size_t, const std::nothrow_t&) throw();

Modified: cfe/trunk/test/Analysis/inlining/containers.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/containers.cpp?rev=180890&r1=180889&r2=180890&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/containers.cpp (original)
+++ cfe/trunk/test/Analysis/inlining/containers.cpp Wed May  1 17:39:31 2013
@@ -78,7 +78,9 @@ void testWrappers(BeginOnlySet &w1, Iter
 }
 
 
-#else
+#else // HEADER
+
+#include "../Inputs/system-header-simulator-cxx.h"
 
 class MySet {
   int *storage;
@@ -152,8 +154,13 @@ class BeginOnlySet {
 public:
   struct IterImpl {
     MySet::iterator impl;
+    typedef std::forward_iterator_tag iterator_category;
+
     IterImpl(MySet::iterator i) : impl(i) {
-      clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+      clang_analyzer_checkInlined(true);
+#if INLINE
+      // expected-warning at -2 {{TRUE}}
+#endif
     }
   };
 
@@ -231,4 +238,4 @@ public:
   }
 };
 
-#endif
+#endif // HEADER