From 8e1be7bf688dc282408aa0403e9896fa5a142ec4 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 24 Nov 2015 11:44:57 -0500 Subject: [PATCH 1/9] cmMakefile: Clarify purpose of method that pops a scope snapshot The `PopPolicyBarrier` method is actually responsible for closing any scope opened by creating a snapshot. Rename it to `PopSnapshot` and add a comment explaining the purpose of the poilcy-scope-specific part of the method. --- Source/cmMakefile.cxx | 17 ++++++++++------- Source/cmMakefile.h | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index cb66a75a2..2c056280d 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -435,7 +435,7 @@ cmMakefile::IncludeScope::~IncludeScope() this->EnforceCMP0011(); } } - this->Makefile->PopPolicyBarrier(this->ReportError); + this->Makefile->PopSnapshot(this->ReportError); this->Makefile->PopFunctionBlockerBarrier(this->ReportError); } @@ -549,7 +549,7 @@ public: ~ListFileScope() { - this->Makefile->PopPolicyBarrier(this->ReportError); + this->Makefile->PopSnapshot(this->ReportError); this->Makefile->PopFunctionBlockerBarrier(this->ReportError); } @@ -1551,7 +1551,7 @@ void cmMakefile::PopFunctionScope(bool reportError) { this->PopPolicy(); - this->PopPolicyBarrier(reportError); + this->PopSnapshot(reportError); this->PopFunctionBlockerBarrier(reportError); @@ -1582,7 +1582,7 @@ void cmMakefile::PushMacroScope(std::string const& fileName, void cmMakefile::PopMacroScope(bool reportError) { this->PopPolicy(); - this->PopPolicyBarrier(reportError); + this->PopSnapshot(reportError); this->PopFunctionBlockerBarrier(reportError); } @@ -1619,7 +1619,7 @@ public: ~BuildsystemFileScope() { this->Makefile->PopFunctionBlockerBarrier(this->ReportError); - this->Makefile->PopPolicyBarrier(this->ReportError); + this->Makefile->PopSnapshot(this->ReportError); #if defined(CMAKE_BUILD_WITH_CMAKE) this->GG->GetFileLockPool().PopFileScope(); #endif @@ -4614,7 +4614,7 @@ cmMakefile::PolicyPushPop::PolicyPushPop(cmMakefile* m, bool weak, cmMakefile::PolicyPushPop::~PolicyPushPop() { this->Makefile->PopPolicy(); - this->Makefile->PopPolicyBarrier(this->ReportError); + this->Makefile->PopSnapshot(this->ReportError); } //---------------------------------------------------------------------------- @@ -4634,8 +4634,11 @@ void cmMakefile::PopPolicy() } //---------------------------------------------------------------------------- -void cmMakefile::PopPolicyBarrier(bool reportError) +void cmMakefile::PopSnapshot(bool reportError) { + // cmState::Snapshot manages nested policy scopes within it. + // Since the scope corresponding to the snapshot is closing, + // reject any still-open nested policy scopes with an error. while (!this->StateSnapshot.CanPopPolicyScope()) { if(reportError) diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 111f07496..a112d57f6 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -904,7 +904,7 @@ private: void PushPolicy(bool weak = false, cmPolicies::PolicyMap const& pm = cmPolicies::PolicyMap()); void PopPolicy(); - void PopPolicyBarrier(bool reportError = true); + void PopSnapshot(bool reportError = true); friend class cmCMakePolicyCommand; class IncludeScope; friend class IncludeScope; From d85c9176ae15f4fb203e501d777cfce8304bf256 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 24 Nov 2015 13:06:42 -0500 Subject: [PATCH 2/9] cmMakefile: Remove unused PolicyPushPop interfaces The PolicyPushPop constructor arguments and Quiet method were used to pass non-default arguments to PushPolicy and PopSnapshot, but no clients use them anymore. --- Source/cmMakefile.cxx | 8 +++----- Source/cmMakefile.h | 6 +----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 2c056280d..688888250 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4601,20 +4601,18 @@ bool cmMakefile::SetPolicy(cmPolicies::PolicyID id, } //---------------------------------------------------------------------------- -cmMakefile::PolicyPushPop::PolicyPushPop(cmMakefile* m, bool weak, - cmPolicies::PolicyMap const& pm): - Makefile(m), ReportError(true) +cmMakefile::PolicyPushPop::PolicyPushPop(cmMakefile* m): Makefile(m) { this->Makefile->StateSnapshot = this->Makefile->StateSnapshot.GetState() ->CreatePolicyScopeSnapshot(this->Makefile->StateSnapshot); - this->Makefile->PushPolicy(weak, pm); + this->Makefile->PushPolicy(); } //---------------------------------------------------------------------------- cmMakefile::PolicyPushPop::~PolicyPushPop() { this->Makefile->PopPolicy(); - this->Makefile->PopSnapshot(this->ReportError); + this->Makefile->PopSnapshot(); } //---------------------------------------------------------------------------- diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index a112d57f6..1edffdcd7 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -321,14 +321,10 @@ public: class PolicyPushPop { public: - PolicyPushPop(cmMakefile* m, - bool weak = false, - cmPolicies::PolicyMap const& pm = cmPolicies::PolicyMap()); + PolicyPushPop(cmMakefile* m); ~PolicyPushPop(); - void Quiet() { this->ReportError = false; } private: cmMakefile* Makefile; - bool ReportError; }; friend class PolicyPushPop; From 0fa7f143a08b62459900bd811c2c0674562bb8be Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 24 Nov 2015 13:11:09 -0500 Subject: [PATCH 3/9] cmLocalGenerator: Use ScopePushPop RAII class to manage local variable scopes --- Source/cmLocalGenerator.cxx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx index 32304032d..233e7fe5f 100644 --- a/Source/cmLocalGenerator.cxx +++ b/Source/cmLocalGenerator.cxx @@ -3003,7 +3003,7 @@ void cmLocalGenerator::GenerateAppleInfoPList(cmTarget* target, // override user make variables. If not the configuration will fall // back to the directory-level values set by the user. cmMakefile* mf = this->Makefile; - mf->PushScope(); + cmMakefile::ScopePushPop varScope(mf); mf->AddDefinition("MACOSX_BUNDLE_EXECUTABLE_NAME", targetName.c_str()); cmLGInfoProp(mf, target, "MACOSX_BUNDLE_INFO_STRING"); cmLGInfoProp(mf, target, "MACOSX_BUNDLE_ICON_FILE"); @@ -3014,7 +3014,6 @@ void cmLocalGenerator::GenerateAppleInfoPList(cmTarget* target, cmLGInfoProp(mf, target, "MACOSX_BUNDLE_BUNDLE_VERSION"); cmLGInfoProp(mf, target, "MACOSX_BUNDLE_COPYRIGHT"); mf->ConfigureFile(inFile.c_str(), fname, false, false, false); - mf->PopScope(); } //---------------------------------------------------------------------------- @@ -3047,12 +3046,11 @@ void cmLocalGenerator::GenerateFrameworkInfoPList(cmTarget* target, // override user make variables. If not the configuration will fall // back to the directory-level values set by the user. cmMakefile* mf = this->Makefile; - mf->PushScope(); + cmMakefile::ScopePushPop varScope(mf); mf->AddDefinition("MACOSX_FRAMEWORK_NAME", targetName.c_str()); cmLGInfoProp(mf, target, "MACOSX_FRAMEWORK_ICON_FILE"); cmLGInfoProp(mf, target, "MACOSX_FRAMEWORK_IDENTIFIER"); cmLGInfoProp(mf, target, "MACOSX_FRAMEWORK_SHORT_VERSION_STRING"); cmLGInfoProp(mf, target, "MACOSX_FRAMEWORK_BUNDLE_VERSION"); mf->ConfigureFile(inFile.c_str(), fname, false, false, false); - mf->PopScope(); } From 32edac6fddfbe91e47b34506cda855232d5a9e2c Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 24 Nov 2015 13:37:34 -0500 Subject: [PATCH 4/9] cmState: Enforce policy scope balancing around variable scopes Everywhere we use cmMakefile::ScopePushPop to manage variable scopes also expects policy scopes to be balanced. There is no place that we use cmMakefile::PolicyPushPop without also using ScopePushPop. Relieve PolicyPushPop of responsibility for policy scope balance checks by moving it to ScopePushPop. --- Source/cmMakefile.cxx | 7 +------ Source/cmState.cxx | 1 + 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 688888250..3c19f550d 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4233,9 +4233,7 @@ void cmMakefile::PopScope() this->CheckForUnusedVariables(); - this->StateSnapshot = - this->GetState()->Pop(this->StateSnapshot); - assert(this->StateSnapshot.IsValid()); + this->PopSnapshot(); } void cmMakefile::RaiseScope(const std::string& var, const char *varDef) @@ -4603,8 +4601,6 @@ bool cmMakefile::SetPolicy(cmPolicies::PolicyID id, //---------------------------------------------------------------------------- cmMakefile::PolicyPushPop::PolicyPushPop(cmMakefile* m): Makefile(m) { - this->Makefile->StateSnapshot = this->Makefile->StateSnapshot.GetState() - ->CreatePolicyScopeSnapshot(this->Makefile->StateSnapshot); this->Makefile->PushPolicy(); } @@ -4612,7 +4608,6 @@ cmMakefile::PolicyPushPop::PolicyPushPop(cmMakefile* m): Makefile(m) cmMakefile::PolicyPushPop::~PolicyPushPop() { this->Makefile->PopPolicy(); - this->Makefile->PopSnapshot(); } //---------------------------------------------------------------------------- diff --git a/Source/cmState.cxx b/Source/cmState.cxx index 363d2bf5d..e20ac89fe 100644 --- a/Source/cmState.cxx +++ b/Source/cmState.cxx @@ -884,6 +884,7 @@ cmState::CreateVariableScopeSnapshot(cmState::Snapshot originSnapshot, pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = VariableScopeType; + pos->PolicyScope = originSnapshot.Position->Policies; assert(originSnapshot.Position->Vars.IsValid()); cmLinkedTree::iterator origin = From 518d6b22f6705c4747c713031587705641540364 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 25 Nov 2015 10:23:05 -0500 Subject: [PATCH 5/9] cmLinkedTree: Rename 'Extend' method to 'Push' Logically the method pushes a nested scope on top of a given scope because the "up" pointer sequence forms a stack independent of any other branches of the tree. --- Source/cmLinkedTree.h | 12 ++++----- Source/cmState.cxx | 58 +++++++++++++++++++++---------------------- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/Source/cmLinkedTree.h b/Source/cmLinkedTree.h index 721a24606..93d801e26 100644 --- a/Source/cmLinkedTree.h +++ b/Source/cmLinkedTree.h @@ -24,7 +24,7 @@ needs of the cmState. For example, the Truncate() method is a specific requirement of the cmState. - An empty cmLinkedTree provides a Root() method, and an Extend() method, + An empty cmLinkedTree provides a Root() method, and an Push() method, each of which return iterators. A Tree can be built up by extending from the root, and then extending from any other iterator. @@ -142,14 +142,14 @@ public: return iterator(const_cast(this), 0); } - iterator Extend(iterator it) + iterator Push(iterator it) { - return Extend_impl(it, T()); + return Push_impl(it, T()); } - iterator Extend(iterator it, T t) + iterator Push(iterator it, T t) { - return Extend_impl(it, t); + return Push_impl(it, t); } iterator Truncate() @@ -179,7 +179,7 @@ private: return &this->Data[pos]; } - iterator Extend_impl(iterator it, T t) + iterator Push_impl(iterator it, T t) { assert(this->UpPositions.size() == this->Data.size()); assert(it.Position <= this->UpPositions.size()); diff --git a/Source/cmState.cxx b/Source/cmState.cxx index e20ac89fe..3c96d465d 100644 --- a/Source/cmState.cxx +++ b/Source/cmState.cxx @@ -288,7 +288,7 @@ cmState::Snapshot cmState::Reset() assert(pos->Policies.IsValid()); assert(pos->PolicyRoot.IsValid()); this->VarTree.Clear(); - pos->Vars = this->VarTree.Extend(this->VarTree.Root()); + pos->Vars = this->VarTree.Push(this->VarTree.Root()); pos->Parent = this->VarTree.Root(); pos->Root = this->VarTree.Root(); @@ -751,14 +751,14 @@ void cmState::Directory::ComputeRelativePathTopBinary() cmState::Snapshot cmState::CreateBaseSnapshot() { - PositionType pos = this->SnapshotData.Extend(this->SnapshotData.Root()); + PositionType pos = this->SnapshotData.Push(this->SnapshotData.Root()); pos->DirectoryParent = this->SnapshotData.Root(); pos->ScopeParent = this->SnapshotData.Root(); pos->SnapshotType = BaseType; pos->BuildSystemDirectory = - this->BuildsystemDirectory.Extend(this->BuildsystemDirectory.Root()); + this->BuildsystemDirectory.Push(this->BuildsystemDirectory.Root()); pos->ExecutionListFile = - this->ExecutionListFiles.Extend(this->ExecutionListFiles.Root()); + this->ExecutionListFiles.Push(this->ExecutionListFiles.Root()); pos->IncludeDirectoryPosition = 0; pos->CompileDefinitionsPosition = 0; pos->CompileOptionsPosition = 0; @@ -768,7 +768,7 @@ cmState::Snapshot cmState::CreateBaseSnapshot() pos->PolicyScope = this->PolicyStack.Root(); assert(pos->Policies.IsValid()); assert(pos->PolicyRoot.IsValid()); - pos->Vars = this->VarTree.Extend(this->VarTree.Root()); + pos->Vars = this->VarTree.Push(this->VarTree.Root()); assert(pos->Vars.IsValid()); pos->Parent = this->VarTree.Root(); pos->Root = this->VarTree.Root(); @@ -781,17 +781,17 @@ cmState::CreateBuildsystemDirectorySnapshot(Snapshot originSnapshot, long entryPointLine) { assert(originSnapshot.IsValid()); - PositionType pos = this->SnapshotData.Extend(originSnapshot.Position); + PositionType pos = this->SnapshotData.Push(originSnapshot.Position); pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->DirectoryParent = originSnapshot.Position; pos->ScopeParent = originSnapshot.Position; pos->SnapshotType = BuildsystemDirectoryType; pos->BuildSystemDirectory = - this->BuildsystemDirectory.Extend( + this->BuildsystemDirectory.Push( originSnapshot.Position->BuildSystemDirectory); pos->ExecutionListFile = - this->ExecutionListFiles.Extend( + this->ExecutionListFiles.Push( originSnapshot.Position->ExecutionListFile); pos->BuildSystemDirectory->DirectoryEnd = pos; pos->Policies = originSnapshot.Position->Policies; @@ -804,7 +804,7 @@ cmState::CreateBuildsystemDirectorySnapshot(Snapshot originSnapshot, originSnapshot.Position->Vars; pos->Parent = origin; pos->Root = origin; - pos->Vars = this->VarTree.Extend(origin); + pos->Vars = this->VarTree.Push(origin); cmState::Snapshot snapshot = cmState::Snapshot(this, pos); originSnapshot.Position->BuildSystemDirectory->Children.push_back(snapshot); return snapshot; @@ -816,13 +816,13 @@ cmState::CreateFunctionCallSnapshot(cmState::Snapshot originSnapshot, long entryPointLine, std::string const& fileName) { - PositionType pos = this->SnapshotData.Extend(originSnapshot.Position, - *originSnapshot.Position); + PositionType pos = this->SnapshotData.Push(originSnapshot.Position, + *originSnapshot.Position); pos->ScopeParent = originSnapshot.Position; pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = FunctionCallType; - pos->ExecutionListFile = this->ExecutionListFiles.Extend( + pos->ExecutionListFile = this->ExecutionListFiles.Push( originSnapshot.Position->ExecutionListFile, fileName); pos->BuildSystemDirectory->DirectoryEnd = pos; pos->PolicyScope = originSnapshot.Position->Policies; @@ -830,7 +830,7 @@ cmState::CreateFunctionCallSnapshot(cmState::Snapshot originSnapshot, cmLinkedTree::iterator origin = originSnapshot.Position->Vars; pos->Parent = origin; - pos->Vars = this->VarTree.Extend(origin); + pos->Vars = this->VarTree.Push(origin); return cmState::Snapshot(this, pos); } @@ -841,12 +841,12 @@ cmState::CreateMacroCallSnapshot(cmState::Snapshot originSnapshot, long entryPointLine, std::string const& fileName) { - PositionType pos = this->SnapshotData.Extend(originSnapshot.Position, - *originSnapshot.Position); + PositionType pos = this->SnapshotData.Push(originSnapshot.Position, + *originSnapshot.Position); pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = MacroCallType; - pos->ExecutionListFile = this->ExecutionListFiles.Extend( + pos->ExecutionListFile = this->ExecutionListFiles.Push( originSnapshot.Position->ExecutionListFile, fileName); assert(originSnapshot.Position->Vars.IsValid()); pos->BuildSystemDirectory->DirectoryEnd = pos; @@ -860,12 +860,12 @@ cmState::CreateCallStackSnapshot(cmState::Snapshot originSnapshot, long entryPointLine, const std::string& fileName) { - PositionType pos = this->SnapshotData.Extend(originSnapshot.Position, - *originSnapshot.Position); + PositionType pos = this->SnapshotData.Push(originSnapshot.Position, + *originSnapshot.Position); pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = CallStackType; - pos->ExecutionListFile = this->ExecutionListFiles.Extend( + pos->ExecutionListFile = this->ExecutionListFiles.Push( originSnapshot.Position->ExecutionListFile, fileName); assert(originSnapshot.Position->Vars.IsValid()); pos->BuildSystemDirectory->DirectoryEnd = pos; @@ -878,8 +878,8 @@ cmState::CreateVariableScopeSnapshot(cmState::Snapshot originSnapshot, std::string const& entryPointCommand, long entryPointLine) { - PositionType pos = this->SnapshotData.Extend(originSnapshot.Position, - *originSnapshot.Position); + PositionType pos = this->SnapshotData.Push(originSnapshot.Position, + *originSnapshot.Position); pos->ScopeParent = originSnapshot.Position; pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; @@ -890,7 +890,7 @@ cmState::CreateVariableScopeSnapshot(cmState::Snapshot originSnapshot, cmLinkedTree::iterator origin = originSnapshot.Position->Vars; pos->Parent = origin; - pos->Vars = this->VarTree.Extend(origin); + pos->Vars = this->VarTree.Push(origin); assert(pos->Vars.IsValid()); return cmState::Snapshot(this, pos); } @@ -901,12 +901,12 @@ cmState::CreateInlineListFileSnapshot(cmState::Snapshot originSnapshot, long entryPointLine, const std::string& fileName) { - PositionType pos = this->SnapshotData.Extend(originSnapshot.Position, - *originSnapshot.Position); + PositionType pos = this->SnapshotData.Push(originSnapshot.Position, + *originSnapshot.Position); pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = InlineListFileType; - pos->ExecutionListFile = this->ExecutionListFiles.Extend( + pos->ExecutionListFile = this->ExecutionListFiles.Push( originSnapshot.Position->ExecutionListFile, fileName); pos->BuildSystemDirectory->DirectoryEnd = pos; pos->PolicyScope = originSnapshot.Position->Policies; @@ -916,8 +916,8 @@ cmState::CreateInlineListFileSnapshot(cmState::Snapshot originSnapshot, cmState::Snapshot cmState::CreatePolicyScopeSnapshot(cmState::Snapshot originSnapshot) { - PositionType pos = this->SnapshotData.Extend(originSnapshot.Position, - *originSnapshot.Position); + PositionType pos = this->SnapshotData.Push(originSnapshot.Position, + *originSnapshot.Position); pos->SnapshotType = PolicyScopeType; pos->BuildSystemDirectory->DirectoryEnd = pos; pos->PolicyScope = originSnapshot.Position->Policies; @@ -1113,8 +1113,8 @@ void cmState::Snapshot::PushPolicy(cmPolicies::PolicyMap entry, bool weak) { PositionType pos = this->Position; pos->Policies = - this->State->PolicyStack.Extend(pos->Policies, - PolicyStackEntry(entry, weak)); + this->State->PolicyStack.Push(pos->Policies, + PolicyStackEntry(entry, weak)); } bool cmState::Snapshot::PopPolicy() From 85fe26b5f742b704b51a7e15b4806366feab3a23 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 24 Nov 2015 14:42:20 -0500 Subject: [PATCH 6/9] cmLinkedTree: Add Pop method Add a method to increment an iterator (follow the "up" pointer) to the previous level in the stack of scopes and free storage of the top of the stack if possible. This will allow short-lived scopes to be created and destroyed by matching Push/Pop pairs without accumulating storage. --- Source/cmLinkedTree.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Source/cmLinkedTree.h b/Source/cmLinkedTree.h index 93d801e26..3b41459c2 100644 --- a/Source/cmLinkedTree.h +++ b/Source/cmLinkedTree.h @@ -152,6 +152,27 @@ public: return Push_impl(it, t); } + bool IsLast(iterator it) + { + return it.Position == this->Data.size(); + } + + iterator Pop(iterator it) + { + assert(!this->Data.empty()); + assert(this->UpPositions.size() == this->Data.size()); + bool const isLast = this->IsLast(it); + ++it; + // If this is the last entry then no other entry can refer + // to it so we can drop its storage. + if (isLast) + { + this->Data.pop_back(); + this->UpPositions.pop_back(); + } + return it; + } + iterator Truncate() { assert(this->UpPositions.size() > 0); From bc1d3a8a8783848016ef12044a02a28b620c41a0 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 24 Nov 2015 19:43:04 -0500 Subject: [PATCH 7/9] cmListFileCache: Implement cmListFileBacktrace ctor/dtor out-of-line --- Source/cmListFileCache.cxx | 11 +++++++++++ Source/cmListFileCache.h | 6 ++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index bff298615..676074fdd 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -398,6 +398,17 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token, } } +cmListFileBacktrace::cmListFileBacktrace(cmState::Snapshot snapshot, + cmCommandContext const& cc) + : Context(cc) + , Snapshot(snapshot) +{ +} + +cmListFileBacktrace::~cmListFileBacktrace() +{ +} + void cmListFileBacktrace::PrintTitle(std::ostream& out) const { if (!this->Snapshot.IsValid()) diff --git a/Source/cmListFileCache.h b/Source/cmListFileCache.h index 0afd7f5d3..17ee10fed 100644 --- a/Source/cmListFileCache.h +++ b/Source/cmListFileCache.h @@ -90,10 +90,8 @@ class cmListFileBacktrace { public: cmListFileBacktrace(cmState::Snapshot snapshot = cmState::Snapshot(), - cmCommandContext const& cc = cmCommandContext()) - : Context(cc), Snapshot(snapshot) - { - } + cmCommandContext const& cc = cmCommandContext()); + ~cmListFileBacktrace(); void PrintTitle(std::ostream& out) const; void PrintCallStack(std::ostream& out) const; From f21dc4a81c05c79b873c9918f6fe8aff4bf02133 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 24 Nov 2015 14:44:19 -0500 Subject: [PATCH 8/9] cmState: Avoid accumulating policy stack storage for short-lived scopes We enforce policy push/pop balance around any scope that pushes/pops a snapshot. Therefore a snapshot may never reference entries of PolicyStack that were created in nested scopes. Free storage of short-lived policy stack entries when they are popped. --- Source/cmState.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmState.cxx b/Source/cmState.cxx index 3c96d465d..f9e96f102 100644 --- a/Source/cmState.cxx +++ b/Source/cmState.cxx @@ -1124,7 +1124,7 @@ bool cmState::Snapshot::PopPolicy() { return false; } - ++pos->Policies; + pos->Policies = this->State->PolicyStack.Pop(pos->Policies); return true; } From 5f860ebb67e86e0aa407e26ddf79652f73742211 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 24 Nov 2015 15:00:47 -0500 Subject: [PATCH 9/9] cmState: Avoid accumulating snapshot storage for short-lived scopes We need to keep only certain snapshot types and their ancestors. Also keep those needed for backtraces. --- Source/cmListFileCache.cxx | 4 ++++ Source/cmState.cxx | 29 +++++++++++++++++++++++++++++ Source/cmState.h | 1 + 3 files changed, 34 insertions(+) diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index 676074fdd..1465f907b 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -403,6 +403,10 @@ cmListFileBacktrace::cmListFileBacktrace(cmState::Snapshot snapshot, : Context(cc) , Snapshot(snapshot) { + if (this->Snapshot.IsValid()) + { + this->Snapshot.Keep(); + } } cmListFileBacktrace::~cmListFileBacktrace() diff --git a/Source/cmState.cxx b/Source/cmState.cxx index f9e96f102..c1ead6c32 100644 --- a/Source/cmState.cxx +++ b/Source/cmState.cxx @@ -27,6 +27,7 @@ struct cmState::SnapshotDataType cmLinkedTree::iterator PolicyRoot; cmLinkedTree::iterator PolicyScope; cmState::SnapshotType SnapshotType; + bool Keep; cmLinkedTree::iterator ExecutionListFile; cmLinkedTree::iterator BuildSystemDirectory; @@ -755,6 +756,7 @@ cmState::Snapshot cmState::CreateBaseSnapshot() pos->DirectoryParent = this->SnapshotData.Root(); pos->ScopeParent = this->SnapshotData.Root(); pos->SnapshotType = BaseType; + pos->Keep = true; pos->BuildSystemDirectory = this->BuildsystemDirectory.Push(this->BuildsystemDirectory.Root()); pos->ExecutionListFile = @@ -787,6 +789,7 @@ cmState::CreateBuildsystemDirectorySnapshot(Snapshot originSnapshot, pos->DirectoryParent = originSnapshot.Position; pos->ScopeParent = originSnapshot.Position; pos->SnapshotType = BuildsystemDirectoryType; + pos->Keep = true; pos->BuildSystemDirectory = this->BuildsystemDirectory.Push( originSnapshot.Position->BuildSystemDirectory); @@ -822,6 +825,7 @@ cmState::CreateFunctionCallSnapshot(cmState::Snapshot originSnapshot, pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = FunctionCallType; + pos->Keep = false; pos->ExecutionListFile = this->ExecutionListFiles.Push( originSnapshot.Position->ExecutionListFile, fileName); pos->BuildSystemDirectory->DirectoryEnd = pos; @@ -846,6 +850,7 @@ cmState::CreateMacroCallSnapshot(cmState::Snapshot originSnapshot, pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = MacroCallType; + pos->Keep = false; pos->ExecutionListFile = this->ExecutionListFiles.Push( originSnapshot.Position->ExecutionListFile, fileName); assert(originSnapshot.Position->Vars.IsValid()); @@ -865,6 +870,7 @@ cmState::CreateCallStackSnapshot(cmState::Snapshot originSnapshot, pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = CallStackType; + pos->Keep = true; pos->ExecutionListFile = this->ExecutionListFiles.Push( originSnapshot.Position->ExecutionListFile, fileName); assert(originSnapshot.Position->Vars.IsValid()); @@ -884,6 +890,7 @@ cmState::CreateVariableScopeSnapshot(cmState::Snapshot originSnapshot, pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = VariableScopeType; + pos->Keep = false; pos->PolicyScope = originSnapshot.Position->Policies; assert(originSnapshot.Position->Vars.IsValid()); @@ -906,6 +913,7 @@ cmState::CreateInlineListFileSnapshot(cmState::Snapshot originSnapshot, pos->EntryPointLine = entryPointLine; pos->EntryPointCommand = entryPointCommand; pos->SnapshotType = InlineListFileType; + pos->Keep = true; pos->ExecutionListFile = this->ExecutionListFiles.Push( originSnapshot.Position->ExecutionListFile, fileName); pos->BuildSystemDirectory->DirectoryEnd = pos; @@ -919,6 +927,7 @@ cmState::CreatePolicyScopeSnapshot(cmState::Snapshot originSnapshot) PositionType pos = this->SnapshotData.Push(originSnapshot.Position, *originSnapshot.Position); pos->SnapshotType = PolicyScopeType; + pos->Keep = false; pos->BuildSystemDirectory->DirectoryEnd = pos; pos->PolicyScope = originSnapshot.Position->Policies; return cmState::Snapshot(this, pos); @@ -937,6 +946,21 @@ cmState::Snapshot cmState::Pop(cmState::Snapshot originSnapshot) prevPos->BuildSystemDirectory->CompileOptions.size(); prevPos->BuildSystemDirectory->DirectoryEnd = prevPos; + if (!pos->Keep && this->SnapshotData.IsLast(pos)) + { + if (pos->Vars != prevPos->Vars) + { + assert(this->VarTree.IsLast(pos->Vars)); + this->VarTree.Pop(pos->Vars); + } + if (pos->ExecutionListFile != prevPos->ExecutionListFile) + { + assert(this->ExecutionListFiles.IsLast(pos->ExecutionListFile)); + this->ExecutionListFiles.Pop(pos->ExecutionListFile); + } + this->SnapshotData.Pop(pos); + } + return Snapshot(this, prevPos); } @@ -999,6 +1023,11 @@ void cmState::Directory::SetCurrentBinary(std::string const& dir) this->ComputeRelativePathTopBinary(); } +void cmState::Snapshot::Keep() +{ + this->Position->Keep = true; +} + void cmState::Snapshot::SetListFile(const std::string& listfile) { *this->Position->ExecutionListFile = listfile; diff --git a/Source/cmState.h b/Source/cmState.h index 99e537c07..a66603f62 100644 --- a/Source/cmState.h +++ b/Source/cmState.h @@ -62,6 +62,7 @@ public: std::vector ClosureKeys() const; bool RaiseScope(std::string const& var, const char* varDef); + void Keep(); void SetListFile(std::string const& listfile); std::string GetExecutionListFile() const;