From 8b1745a1c5ed992e632bd4865c0f6a34b136041d Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 30 Apr 2015 00:19:55 +0200 Subject: [PATCH 1/5] cmDefinitions: Accept varStack iterators in Get API. --- Source/cmDefinitions.cxx | 21 ++++++++++++++------- Source/cmDefinitions.h | 8 ++++++-- Source/cmMakefile.cxx | 3 ++- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 61328be60..94d27c29c 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -11,6 +11,8 @@ ============================================================================*/ #include "cmDefinitions.h" +#include + //---------------------------------------------------------------------------- cmDefinitions::Def cmDefinitions::NoDef; @@ -21,28 +23,33 @@ cmDefinitions::cmDefinitions(cmDefinitions* parent) } //---------------------------------------------------------------------------- -cmDefinitions::Def const& -cmDefinitions::GetInternal(const std::string& key) +cmDefinitions::Def const& cmDefinitions::GetInternal( + const std::string& key, + std::list::reverse_iterator rbegin, + std::list::reverse_iterator rend) { + assert(&*rbegin == this); MapType::const_iterator i = this->Map.find(key); if(i != this->Map.end()) { return i->second; } - cmDefinitions* up = this->Up; - if(!up) + ++rbegin; + if(rbegin == rend) { return this->NoDef; } // Query the parent scope and store the result locally. - Def def = up->GetInternal(key); + Def def = rbegin->GetInternal(key, rbegin, rend); return this->Map.insert(MapType::value_type(key, def)).first->second; } //---------------------------------------------------------------------------- -const char* cmDefinitions::Get(const std::string& key) +const char* cmDefinitions::Get(const std::string& key, + std::list::reverse_iterator rbegin, + std::list::reverse_iterator rend) { - Def const& def = this->GetInternal(key); + Def const& def = this->GetInternal(key, rbegin, rend); return def.Exists? def.c_str() : 0; } diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 4c7f11f60..2bcfcb0b2 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -37,7 +37,9 @@ public: /** Get the value associated with a key; null if none. Store the result locally if it came from a parent. */ - const char* Get(const std::string& key); + const char* Get(const std::string& key, + std::list::reverse_iterator rbegin, + std::list::reverse_iterator rend); /** Set (or unset if null) a value associated with a key. */ void Set(const std::string& key, const char* value); @@ -81,7 +83,9 @@ private: MapType Map; // Internal query and update methods. - Def const& GetInternal(const std::string& key); + Def const& GetInternal(const std::string& key, + std::list::reverse_iterator rbegin, + std::list::reverse_iterator rend); void MakeClosure(std::set& undefined, std::list::const_reverse_iterator rbegin, diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 5686b1b0d..ae7e56454 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -70,7 +70,8 @@ public: const char* GetDefinition(std::string const& name) { - return this->VarStack.back().Get(name); + return this->VarStack.back().Get(name, this->VarStack.rbegin(), + this->VarStack.rend()); } void SetDefinition(std::string const& name, std::string const& value) From 191573f7922aea3c1abdba325f5e65e4ba03d7c8 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 16:19:11 +0200 Subject: [PATCH 2/5] cmDefinitions: Remove Parent pointer. All structural knowledge of the stack of scopes is now external. --- Source/cmDefinitions.cxx | 6 ------ Source/cmDefinitions.h | 9 --------- Source/cmMakefile.cxx | 7 +------ 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 94d27c29c..6e2665665 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -16,12 +16,6 @@ //---------------------------------------------------------------------------- cmDefinitions::Def cmDefinitions::NoDef; -//---------------------------------------------------------------------------- -cmDefinitions::cmDefinitions(cmDefinitions* parent) - : Up(parent) -{ -} - //---------------------------------------------------------------------------- cmDefinitions::Def const& cmDefinitions::GetInternal( const std::string& key, diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 2bcfcb0b2..33f468b4b 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -29,12 +29,6 @@ class cmDefinitions { public: - /** Construct with the given parent scope. */ - cmDefinitions(cmDefinitions* parent = 0); - - /** Returns the parent scope, if any. */ - cmDefinitions* GetParent() const { return this->Up; } - /** Get the value associated with a key; null if none. Store the result locally if it came from a parent. */ const char* Get(const std::string& key, @@ -71,9 +65,6 @@ private: }; static Def NoDef; - // Parent scope, if any. - cmDefinitions* Up; - // Local definitions, set or unset. #if defined(CMAKE_BUILD_WITH_CMAKE) typedef cmsys::hash_map MapType; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index ae7e56454..b16d7e6de 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -53,12 +53,7 @@ public: void PushDefinitions() { - cmDefinitions* parent = 0; - if (!this->VarStack.empty()) - { - parent = &this->VarStack.back(); - } - this->VarStack.push_back(cmDefinitions(parent)); + this->VarStack.push_back(cmDefinitions()); } void InitializeDefinitions(cmMakefile* parent) From 7a5039fa6c0d84d1f4062211afcf0318db77f325 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Fri, 1 May 2015 19:35:47 +0200 Subject: [PATCH 3/5] cmDefinitions: Use static member without this->. --- Source/cmDefinitions.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 6e2665665..a429b6d33 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -31,7 +31,7 @@ cmDefinitions::Def const& cmDefinitions::GetInternal( ++rbegin; if(rbegin == rend) { - return this->NoDef; + return cmDefinitions::NoDef; } // Query the parent scope and store the result locally. Def def = rbegin->GetInternal(key, rbegin, rend); From a7ce0c7bc03efcfff6bf9fecc52663409262d73d Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Fri, 1 May 2015 01:13:38 +0200 Subject: [PATCH 4/5] cmDefinitions: Make GetInternal method static. For some reason, using recursion here is faster to configure ParaView than using a loop. Probably some compiler optimization is inhibited by using a loop. Co-Author: Brad King --- Source/cmDefinitions.cxx | 18 +++++++++--------- Source/cmDefinitions.h | 8 +++----- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index a429b6d33..f54bc4d51 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -22,20 +22,20 @@ cmDefinitions::Def const& cmDefinitions::GetInternal( std::list::reverse_iterator rbegin, std::list::reverse_iterator rend) { - assert(&*rbegin == this); - MapType::const_iterator i = this->Map.find(key); - if(i != this->Map.end()) + assert(rbegin != rend); + MapType::const_iterator i = rbegin->Map.find(key); + if (i != rbegin->Map.end()) { return i->second; } - ++rbegin; - if(rbegin == rend) + std::list::reverse_iterator rit = rbegin; + ++rit; + if (rit == rend) { return cmDefinitions::NoDef; } - // Query the parent scope and store the result locally. - Def def = rbegin->GetInternal(key, rbegin, rend); - return this->Map.insert(MapType::value_type(key, def)).first->second; + Def const& def = cmDefinitions::GetInternal(key, rit, rend); + return rbegin->Map.insert(MapType::value_type(key, def)).first->second; } //---------------------------------------------------------------------------- @@ -43,7 +43,7 @@ const char* cmDefinitions::Get(const std::string& key, std::list::reverse_iterator rbegin, std::list::reverse_iterator rend) { - Def const& def = this->GetInternal(key, rbegin, rend); + Def const& def = cmDefinitions::GetInternal(key, rbegin, rend); return def.Exists? def.c_str() : 0; } diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 33f468b4b..77edbd057 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -73,11 +73,9 @@ private: #endif MapType Map; - // Internal query and update methods. - Def const& GetInternal(const std::string& key, - std::list::reverse_iterator rbegin, - std::list::reverse_iterator rend); - + static Def const& GetInternal(const std::string& key, + std::list::reverse_iterator rbegin, + std::list::reverse_iterator rend); void MakeClosure(std::set& undefined, std::list::const_reverse_iterator rbegin, std::list::const_reverse_iterator rend); From 6c7dad41d9f5e9b6ab6bdd6ddee837aa5509e19e Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Fri, 1 May 2015 01:50:45 +0200 Subject: [PATCH 5/5] cmDefinitions: Make Get method static. --- Source/cmDefinitions.h | 6 +++--- Source/cmMakefile.cxx | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 77edbd057..b24479352 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -31,9 +31,9 @@ class cmDefinitions public: /** Get the value associated with a key; null if none. Store the result locally if it came from a parent. */ - const char* Get(const std::string& key, - std::list::reverse_iterator rbegin, - std::list::reverse_iterator rend); + static const char* Get(const std::string& key, + std::list::reverse_iterator rbegin, + std::list::reverse_iterator rend); /** Set (or unset if null) a value associated with a key. */ void Set(const std::string& key, const char* value); diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index b16d7e6de..4ed241986 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -65,8 +65,8 @@ public: const char* GetDefinition(std::string const& name) { - return this->VarStack.back().Get(name, this->VarStack.rbegin(), - this->VarStack.rend()); + return cmDefinitions::Get(name, this->VarStack.rbegin(), + this->VarStack.rend()); } void SetDefinition(std::string const& name, std::string const& value)