From 776d3bb65c90278b9c65544b235d2ac40aea1d6e Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Wed, 1 May 2013 22:39:31 +0000 Subject: [PATCH] [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'. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@180890 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/ExprEngineCallAndReturn.cpp | 48 ++++++++----------- .../Inputs/system-header-simulator-cxx.h | 6 +++ test/Analysis/inlining/containers.cpp | 13 +++-- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index f01e4e7640..06570a4b4a 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -676,46 +676,40 @@ 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()) +/// 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(FD) || isa(FD))) return false; diff --git a/test/Analysis/Inputs/system-header-simulator-cxx.h b/test/Analysis/Inputs/system-header-simulator-cxx.h index eee0e31a6f..6e434a04b2 100644 --- a/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -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(); diff --git a/test/Analysis/inlining/containers.cpp b/test/Analysis/inlining/containers.cpp index 4500dff6dc..73b2957ad6 100644 --- a/test/Analysis/inlining/containers.cpp +++ b/test/Analysis/inlining/containers.cpp @@ -78,7 +78,9 @@ void testWrappers(BeginOnlySet &w1, IteratorStructOnlySet &w2, } -#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@-2 {{TRUE}} +#endif } }; @@ -231,4 +238,4 @@ public: } }; -#endif +#endif // HEADER -- 2.40.0