From 7872201bf69610579b6b1fab4b5389b692c82089 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 16 May 2015 05:33:19 +0200 Subject: [PATCH 1/3] cmDefinitions: Remove internal MakeClosure method. There is no need to have a separate method, or to pass an external set to it. --- Source/cmDefinitions.cxx | 17 ++++------------- Source/cmDefinitions.h | 3 --- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index f54bc4d51..581f259fc 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -81,18 +81,8 @@ cmDefinitions cmDefinitions::MakeClosure( std::list::const_reverse_iterator rbegin, std::list::const_reverse_iterator rend) { - std::set undefined; cmDefinitions closure; - closure.MakeClosure(undefined, rbegin, rend); - return closure; -} - -//---------------------------------------------------------------------------- -void -cmDefinitions::MakeClosure(std::set& undefined, - std::list::const_reverse_iterator rbegin, - std::list::const_reverse_iterator rend) -{ + std::set undefined; for (std::list::const_reverse_iterator it = rbegin; it != rend; ++it) { @@ -101,12 +91,12 @@ cmDefinitions::MakeClosure(std::set& undefined, mi != it->Map.end(); ++mi) { // Use this key if it is not already set or unset. - if(this->Map.find(mi->first) == this->Map.end() && + if(closure.Map.find(mi->first) == closure.Map.end() && undefined.find(mi->first) == undefined.end()) { if(mi->second.Exists) { - this->Map.insert(*mi); + closure.Map.insert(*mi); } else { @@ -115,6 +105,7 @@ cmDefinitions::MakeClosure(std::set& undefined, } } } + return closure; } //---------------------------------------------------------------------------- diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index b24479352..e762b4128 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -76,9 +76,6 @@ private: 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); }; #endif From 98c5c90361f89f810cdd6fb233f3e822b638f143 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 16 May 2015 05:33:25 +0200 Subject: [PATCH 2/3] cmDefinitions: Centralize knowledge of iterator type. Currently we process a list of definitions, but that will change. --- Source/cmDefinitions.cxx | 33 ++++++++++++++------------------- Source/cmDefinitions.h | 12 +++++------- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 581f259fc..6fcc002b6 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -18,32 +18,29 @@ cmDefinitions::Def cmDefinitions::NoDef; //---------------------------------------------------------------------------- cmDefinitions::Def const& cmDefinitions::GetInternal( - const std::string& key, - std::list::reverse_iterator rbegin, - std::list::reverse_iterator rend) + const std::string& key, StackIter begin, StackIter end) { - assert(rbegin != rend); - MapType::const_iterator i = rbegin->Map.find(key); - if (i != rbegin->Map.end()) + assert(begin != end); + MapType::const_iterator i = begin->Map.find(key); + if (i != begin->Map.end()) { return i->second; } - std::list::reverse_iterator rit = rbegin; - ++rit; - if (rit == rend) + StackIter it = begin; + ++it; + if (it == end) { return cmDefinitions::NoDef; } - Def const& def = cmDefinitions::GetInternal(key, rit, rend); - return rbegin->Map.insert(MapType::value_type(key, def)).first->second; + Def const& def = cmDefinitions::GetInternal(key, it, end); + return begin->Map.insert(MapType::value_type(key, def)).first->second; } //---------------------------------------------------------------------------- const char* cmDefinitions::Get(const std::string& key, - std::list::reverse_iterator rbegin, - std::list::reverse_iterator rend) + StackIter begin, StackIter end) { - Def const& def = cmDefinitions::GetInternal(key, rbegin, rend); + Def const& def = cmDefinitions::GetInternal(key, begin, end); return def.Exists? def.c_str() : 0; } @@ -77,14 +74,12 @@ std::vector cmDefinitions::LocalKeys() const } //---------------------------------------------------------------------------- -cmDefinitions cmDefinitions::MakeClosure( - std::list::const_reverse_iterator rbegin, - std::list::const_reverse_iterator rend) +cmDefinitions cmDefinitions::MakeClosure(StackConstIter begin, + StackConstIter end) { cmDefinitions closure; std::set undefined; - for (std::list::const_reverse_iterator it = rbegin; - it != rend; ++it) + for (StackConstIter it = begin; it != end; ++it) { // Consider local definitions. for(MapType::const_iterator mi = it->Map.begin(); diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index e762b4128..b95ae6ba5 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -28,12 +28,13 @@ */ class cmDefinitions { + typedef std::list::reverse_iterator StackIter; + typedef std::list::const_reverse_iterator StackConstIter; public: /** Get the value associated with a key; null if none. Store the result locally if it came from a parent. */ static const char* Get(const std::string& key, - std::list::reverse_iterator rbegin, - std::list::reverse_iterator rend); + StackIter begin, StackIter end); /** Set (or unset if null) a value associated with a key. */ void Set(const std::string& key, const char* value); @@ -46,9 +47,7 @@ public: std::vector ClosureKeys(std::set& bound) const; - static cmDefinitions MakeClosure( - std::list::const_reverse_iterator rbegin, - std::list::const_reverse_iterator rend); + static cmDefinitions MakeClosure(StackConstIter begin, StackConstIter end); private: // String with existence boolean. @@ -74,8 +73,7 @@ private: MapType Map; static Def const& GetInternal(const std::string& key, - std::list::reverse_iterator rbegin, - std::list::reverse_iterator rend); + StackIter begin, StackIter end); }; #endif From f170985e7aaec7562ca481bab49228ece233c4e4 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 16 May 2015 05:33:30 +0200 Subject: [PATCH 3/3] cmDefinitions: Make the ClosureKeys method static. For consistency with all other closure-related methods. --- Source/cmDefinitions.cxx | 20 +++++++++++++------- Source/cmDefinitions.h | 4 ++-- Source/cmMakefile.cxx | 12 ++---------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 6fcc002b6..e2c687632 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -105,18 +105,24 @@ cmDefinitions cmDefinitions::MakeClosure(StackConstIter begin, //---------------------------------------------------------------------------- std::vector -cmDefinitions::ClosureKeys(std::set& bound) const +cmDefinitions::ClosureKeys(StackConstIter begin, StackConstIter end) { + std::set bound; std::vector defined; - defined.reserve(this->Map.size()); - for(MapType::const_iterator mi = this->Map.begin(); - mi != this->Map.end(); ++mi) + + for (StackConstIter it = begin; it != end; ++it) { - // Use this key if it is not already set or unset. - if(bound.insert(mi->first).second && mi->second.Exists) + defined.reserve(defined.size() + it->Map.size()); + for(MapType::const_iterator mi = it->Map.begin(); + mi != it->Map.end(); ++mi) { - defined.push_back(mi->first); + // Use this key if it is not already set or unset. + if(bound.insert(mi->first).second && mi->second.Exists) + { + defined.push_back(mi->first); + } } } + return defined; } diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index b95ae6ba5..80643a96d 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -44,8 +44,8 @@ public: /** Get the set of all local keys. */ std::vector LocalKeys() const; - std::vector - ClosureKeys(std::set& bound) const; + static std::vector ClosureKeys(StackConstIter begin, + StackConstIter end); static cmDefinitions MakeClosure(StackConstIter begin, StackConstIter end); diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 7b8d3afa5..9f9171c08 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -94,16 +94,8 @@ public: std::vector ClosureKeys() const { - std::vector closureKeys; - std::set bound; - for (std::list::const_reverse_iterator it = - this->VarStack.rbegin(); it != this->VarStack.rend(); ++it) - { - std::vector const& localKeys = it->ClosureKeys(bound); - closureKeys.insert(closureKeys.end(), - localKeys.begin(), localKeys.end()); - } - return closureKeys; + return cmDefinitions::ClosureKeys(this->VarStack.rbegin(), + this->VarStack.rend()); } void PopDefinitions()