From b43c162e99c7302ae81c746bc5aab1d9a64787b3 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 25 Apr 2015 16:03:04 +0200 Subject: cmMakefile: Use the Internal class to enclose the VarStack. Put knowledge of the implementation details in one place. --- Source/cmMakefile.cxx | 141 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 95 insertions(+), 46 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 0935383..d71b815 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -50,6 +50,84 @@ public: std::stack > VarInitStack; std::stack > VarUsageStack; bool IsSourceFileTryCompile; + + void PushDefinitions() + { + cmDefinitions* parent = 0; + if (!this->VarStack.empty()) + { + parent = &this->VarStack.top(); + } + this->VarStack.push(cmDefinitions(parent)); + } + + void InitializeDefinitions(cmMakefile* parent) + { + this->VarStack.top() = parent->Internal->VarStack.top().Closure(); + } + + const char* GetDefinition(std::string const& name) + { + return this->VarStack.top().Get(name); + } + + void SetDefinition(std::string const& name, std::string const& value) + { + this->VarStack.top().Set(name, value.c_str()); + } + + void RemoveDefinition(std::string const& name) + { + this->VarStack.top().Set(name, 0); + } + + std::set LocalKeys() const + { + return this->VarStack.top().LocalKeys(); + } + + std::set ClosureKeys() const + { + return this->VarStack.top().ClosureKeys(); + } + + void PopDefinitions() + { + this->VarStack.pop(); + } + + bool RaiseScope(std::string const& var, const char* varDef, cmMakefile* mf) + { + cmDefinitions& cur = this->VarStack.top(); + if(cmDefinitions* up = cur.GetParent()) + { + // First localize the definition in the current scope. + cur.Get(var); + + // Now update the definition in the parent scope. + up->Set(var, varDef); + } + else if(cmLocalGenerator* plg = mf->GetLocalGenerator()->GetParent()) + { + // Update the definition in the parent directory top scope. This + // directory's scope was initialized by the closure of the parent + // scope, so we do not need to localize the definition first. + cmMakefile* parent = plg->GetMakefile(); + if (varDef) + { + parent->AddDefinition(var, varDef); + } + else + { + parent->RemoveDefinition(var); + } + } + else + { + return false; + } + return true; + } }; // default is not to be building executables @@ -59,11 +137,9 @@ cmMakefile::cmMakefile(cmLocalGenerator* localGenerator) StateSnapshot(localGenerator->GetGlobalGenerator() ->GetCMakeInstance()->GetState()) { - const cmDefinitions& defs = cmDefinitions(); - const std::set globalKeys = defs.LocalKeys(); - this->Internal->VarStack.push(defs); - this->Internal->VarInitStack.push(globalKeys); - this->Internal->VarUsageStack.push(globalKeys); + this->Internal->PushDefinitions(); + this->Internal->VarInitStack.push(std::set()); + this->Internal->VarUsageStack.push(std::set()); this->Internal->IsSourceFileTryCompile = false; if (this->LocalGenerator->GetParent()) @@ -1523,7 +1599,7 @@ void cmMakefile::InitializeFromParent() cmMakefile *parent = this->LocalGenerator->GetParent()->GetMakefile(); // Initialize definitions with the closure of the parent scope. - this->Internal->VarStack.top() = parent->Internal->VarStack.top().Closure(); + this->Internal->InitializeDefinitions(parent); this->AddDefinition("CMAKE_CURRENT_SOURCE_DIR", this->GetCurrentSourceDirectory()); @@ -1717,7 +1793,7 @@ void cmMakefile::AddDefinition(const std::string& name, const char* value) return; } - this->Internal->VarStack.top().Set(name, value); + this->Internal->SetDefinition(name, value); if (!this->Internal->VarUsageStack.empty() && this->VariableInitialized(name)) { @@ -1787,13 +1863,13 @@ void cmMakefile::AddCacheDefinition(const std::string& name, const char* value, this->GetState()->AddCacheEntry(name, haveVal ? val.c_str() : 0, doc, type); // if there was a definition then remove it - this->Internal->VarStack.top().Set(name, 0); + this->Internal->RemoveDefinition(name); } void cmMakefile::AddDefinition(const std::string& name, bool value) { - this->Internal->VarStack.top().Set(name, value? "ON" : "OFF"); + this->Internal->SetDefinition(name, value ? "ON" : "OFF"); if (!this->Internal->VarUsageStack.empty() && this->VariableInitialized(name)) { @@ -1817,8 +1893,7 @@ void cmMakefile::CheckForUnusedVariables() const { return; } - const cmDefinitions& defs = this->Internal->VarStack.top(); - const std::set& locals = defs.LocalKeys(); + const std::set& locals = this->Internal->LocalKeys(); std::set::const_iterator it = locals.begin(); for (; it != locals.end(); ++it) { @@ -1892,7 +1967,7 @@ void cmMakefile::CheckForUnused(const char* reason, void cmMakefile::RemoveDefinition(const std::string& name) { - this->Internal->VarStack.top().Set(name, 0); + this->Internal->RemoveDefinition(name); if (!this->Internal->VarUsageStack.empty() && this->VariableInitialized(name)) { @@ -2365,7 +2440,7 @@ const char* cmMakefile::GetRequiredDefinition(const std::string& name) const bool cmMakefile::IsDefinitionSet(const std::string& name) const { - const char* def = this->Internal->VarStack.top().Get(name); + const char* def = this->Internal->GetDefinition(name); this->Internal->VarUsageStack.top().insert(name); if(!def) { @@ -2391,7 +2466,7 @@ const char* cmMakefile::GetDefinition(const std::string& name) const { this->Internal->VarUsageStack.top().insert(name); } - const char* def = this->Internal->VarStack.top().Get(name); + const char* def = this->Internal->GetDefinition(name); if(!def) { def = this->GetState()->GetInitializedCacheValue(name); @@ -2431,8 +2506,7 @@ std::vector cmMakefile std::vector res; if ( !cacheonly ) { - std::set definitions = - this->Internal->VarStack.top().ClosureKeys(); + std::set definitions = this->Internal->ClosureKeys(); res.insert(res.end(), definitions.begin(), definitions.end()); } std::vector cacheKeys = @@ -4324,10 +4398,9 @@ std::string cmMakefile::GetListFileStack() const void cmMakefile::PushScope() { - cmDefinitions* parent = &this->Internal->VarStack.top(); + this->Internal->PushDefinitions(); const std::set& init = this->Internal->VarInitStack.top(); const std::set& usage = this->Internal->VarUsageStack.top(); - this->Internal->VarStack.push(cmDefinitions(parent)); this->Internal->VarInitStack.push(init); this->Internal->VarUsageStack.push(usage); @@ -4348,10 +4421,9 @@ void cmMakefile::PopScope() this->PopLoopBlockBarrier(); - cmDefinitions* current = &this->Internal->VarStack.top(); std::set init = this->Internal->VarInitStack.top(); std::set usage = this->Internal->VarUsageStack.top(); - const std::set& locals = current->LocalKeys(); + const std::set& locals = this->Internal->LocalKeys(); // Remove initialization and usage information for variables in the local // scope. std::set::const_iterator it = locals.begin(); @@ -4367,7 +4439,8 @@ void cmMakefile::PopScope() usage.erase(*it); } } - this->Internal->VarStack.pop(); + + this->Internal->PopDefinitions(); this->Internal->VarInitStack.pop(); this->Internal->VarUsageStack.pop(); // Push initialization and usage up to the parent scope. @@ -4382,31 +4455,7 @@ void cmMakefile::RaiseScope(const std::string& var, const char *varDef) return; } - cmDefinitions& cur = this->Internal->VarStack.top(); - if(cmDefinitions* up = cur.GetParent()) - { - // First localize the definition in the current scope. - cur.Get(var); - - // Now update the definition in the parent scope. - up->Set(var, varDef); - } - else if(cmLocalGenerator* plg = this->LocalGenerator->GetParent()) - { - // Update the definition in the parent directory top scope. This - // directory's scope was initialized by the closure of the parent - // scope, so we do not need to localize the definition first. - cmMakefile* parent = plg->GetMakefile(); - if (varDef) - { - parent->AddDefinition(var, varDef); - } - else - { - parent->RemoveDefinition(var); - } - } - else + if (!this->Internal->RaiseScope(var, varDef, this)) { std::ostringstream m; m << "Cannot set \"" << var << "\": current scope has no parent."; -- cgit v0.12 From 60200ca5088058c70282500994727f2017276df8 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 25 Apr 2015 16:33:26 +0200 Subject: cmDefinitions: Add an Erase method. --- Source/cmDefinitions.cxx | 5 +++++ Source/cmDefinitions.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index abb46b3..58500c9 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -61,6 +61,11 @@ void cmDefinitions::Set(const std::string& key, const char* value) } } +void cmDefinitions::Erase(const std::string& key) +{ + this->Map.erase(key); +} + //---------------------------------------------------------------------------- std::set cmDefinitions::LocalKeys() const { diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 83cd0d9..13f885d 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -40,6 +40,8 @@ public: /** Set (or unset if null) a value associated with a key. */ void Set(const std::string& key, const char* value); + void Erase(const std::string& key); + /** Get the set of all local keys. */ std::set LocalKeys() const; -- cgit v0.12 From 5067ae41b03442a7dba9210595e782678835a3ff Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 25 Apr 2015 16:36:48 +0200 Subject: cmDefinitions: Externalize the Set logic. --- Source/cmDefinitions.cxx | 11 +---------- Source/cmMakefile.cxx | 10 +++++++++- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 58500c9..d2b37bb 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -49,16 +49,7 @@ const char* cmDefinitions::Get(const std::string& key) void cmDefinitions::Set(const std::string& key, const char* value) { Def def(value); - if(this->Up || def.Exists) - { - // In lower scopes we store keys, defined or not. - this->Map[key] = def; - } - else - { - // In the top-most scope we need not store undefined keys. - this->Map.erase(key); - } + this->Map[key] = def; } void cmDefinitions::Erase(const std::string& key) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index d71b815..8754427 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -78,7 +78,15 @@ public: void RemoveDefinition(std::string const& name) { - this->VarStack.top().Set(name, 0); + if (this->VarStack.size() > 1) + { + // In lower scopes we store keys, defined or not. + this->VarStack.top().Set(name, 0); + } + else + { + this->VarStack.top().Erase(name); + } } std::set LocalKeys() const -- cgit v0.12 From 818bf727c1eb4a500decb5856715a964c948242e Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 16:34:13 +0200 Subject: cmDefinitions: Change LocalKeys to return a vector. This is more efficient and we lose nothing. --- Source/cmDefinitions.cxx | 7 ++++--- Source/cmDefinitions.h | 2 +- Source/cmMakefile.cxx | 10 +++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index d2b37bb..448ba9d 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -58,16 +58,17 @@ void cmDefinitions::Erase(const std::string& key) } //---------------------------------------------------------------------------- -std::set cmDefinitions::LocalKeys() const +std::vector cmDefinitions::LocalKeys() const { - std::set keys; + std::vector keys; + keys.reserve(this->Map.size()); // Consider local definitions. for(MapType::const_iterator mi = this->Map.begin(); mi != this->Map.end(); ++mi) { if (mi->second.Exists) { - keys.insert(mi->first); + keys.push_back(mi->first); } } return keys; diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 13f885d..d21ec23 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -43,7 +43,7 @@ public: void Erase(const std::string& key); /** Get the set of all local keys. */ - std::set LocalKeys() const; + std::vector LocalKeys() const; /** Compute the closure of all defined keys with values. This flattens the scope. The result has no parent. */ diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 8754427..9209e49 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -89,7 +89,7 @@ public: } } - std::set LocalKeys() const + std::vector LocalKeys() const { return this->VarStack.top().LocalKeys(); } @@ -1901,8 +1901,8 @@ void cmMakefile::CheckForUnusedVariables() const { return; } - const std::set& locals = this->Internal->LocalKeys(); - std::set::const_iterator it = locals.begin(); + const std::vector& locals = this->Internal->LocalKeys(); + std::vector::const_iterator it = locals.begin(); for (; it != locals.end(); ++it) { this->CheckForUnused("out of scope", *it); @@ -4431,10 +4431,10 @@ void cmMakefile::PopScope() std::set init = this->Internal->VarInitStack.top(); std::set usage = this->Internal->VarUsageStack.top(); - const std::set& locals = this->Internal->LocalKeys(); + const std::vector& locals = this->Internal->LocalKeys(); // Remove initialization and usage information for variables in the local // scope. - std::set::const_iterator it = locals.begin(); + std::vector::const_iterator it = locals.begin(); for (; it != locals.end(); ++it) { init.erase(*it); -- cgit v0.12 From 78e1454ea09e9c568578e26971d6cd45b7fa39c7 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 11:38:08 +0200 Subject: cmDefinitions: Replace ClosureKeys recursion with looping. --- Source/cmDefinitions.cxx | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 448ba9d..a6f46e2 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -131,22 +131,22 @@ std::set cmDefinitions::ClosureKeys() const void cmDefinitions::ClosureKeys(std::set& defined, std::set& undefined) const { - // Consider local definitions. - for(MapType::const_iterator mi = this->Map.begin(); - mi != this->Map.end(); ++mi) + cmDefinitions const* up = this; + + while (up) { - // Use this key if it is not already set or unset. - if(defined.find(mi->first) == defined.end() && - undefined.find(mi->first) == undefined.end()) + // Consider local definitions. + for(MapType::const_iterator mi = up->Map.begin(); + mi != up->Map.end(); ++mi) { - std::set& m = mi->second.Exists? defined : undefined; - m.insert(mi->first); + // Use this key if it is not already set or unset. + if(defined.find(mi->first) == defined.end() && + undefined.find(mi->first) == undefined.end()) + { + std::set& m = mi->second.Exists? defined : undefined; + m.insert(mi->first); + } } - } - - // Traverse parents. - if(cmDefinitions const* up = this->Up) - { - up->ClosureKeys(defined, undefined); + up = up->Up; } } -- cgit v0.12 From ca9fa77d5d34a993073cd5256d65f88cd2e1a28f Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 15:38:09 +0200 Subject: cmDefinitions: Inline GetClosureKeys implementation. --- Source/cmDefinitions.cxx | 8 +------- Source/cmDefinitions.h | 4 ---- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index a6f46e2..4b0eed4 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -123,14 +123,7 @@ std::set cmDefinitions::ClosureKeys() const { std::set defined; std::set undefined; - this->ClosureKeys(defined, undefined); - return defined; -} -//---------------------------------------------------------------------------- -void cmDefinitions::ClosureKeys(std::set& defined, - std::set& undefined) const -{ cmDefinitions const* up = this; while (up) @@ -149,4 +142,5 @@ void cmDefinitions::ClosureKeys(std::set& defined, } up = up->Up; } + return defined; } diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index d21ec23..950970b 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -86,10 +86,6 @@ private: cmDefinitions(ClosureTag const&, cmDefinitions const* root); void ClosureImpl(std::set& undefined, cmDefinitions const* defs); - - // Implementation of ClosureKeys() method. - void ClosureKeys(std::set& defined, - std::set& undefined) const; }; #endif -- cgit v0.12 From 012a75a00f060d6ca36cc9ffb97439a7ad526395 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 15:36:49 +0200 Subject: cmDefinitions: Make ClosureKeys API vector-based. Construct the final list directly in a named return value. Use a single set to track bindings already found. Co-Author: Brad King --- Source/cmDefinitions.cxx | 12 +++++------- Source/cmDefinitions.h | 2 +- Source/cmMakefile.cxx | 5 ++--- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 4b0eed4..f2100eb 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -119,10 +119,10 @@ void cmDefinitions::ClosureImpl(std::set& undefined, } //---------------------------------------------------------------------------- -std::set cmDefinitions::ClosureKeys() const +std::vector cmDefinitions::ClosureKeys() const { - std::set defined; - std::set undefined; + std::vector defined; + std::set bound; cmDefinitions const* up = this; @@ -133,11 +133,9 @@ std::set cmDefinitions::ClosureKeys() const mi != up->Map.end(); ++mi) { // Use this key if it is not already set or unset. - if(defined.find(mi->first) == defined.end() && - undefined.find(mi->first) == undefined.end()) + if(bound.insert(mi->first).second && mi->second.Exists) { - std::set& m = mi->second.Exists? defined : undefined; - m.insert(mi->first); + defined.push_back(mi->first); } } up = up->Up; diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 950970b..4664090 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -50,7 +50,7 @@ public: cmDefinitions Closure() const; /** Compute the set of all defined keys. */ - std::set ClosureKeys() const; + std::vector ClosureKeys() const; private: // String with existence boolean. diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 9209e49..e2a9f61 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -94,7 +94,7 @@ public: return this->VarStack.top().LocalKeys(); } - std::set ClosureKeys() const + std::vector ClosureKeys() const { return this->VarStack.top().ClosureKeys(); } @@ -2514,8 +2514,7 @@ std::vector cmMakefile std::vector res; if ( !cacheonly ) { - std::set definitions = this->Internal->ClosureKeys(); - res.insert(res.end(), definitions.begin(), definitions.end()); + res = this->Internal->ClosureKeys(); } std::vector cacheKeys = this->GetState()->GetCacheEntryKeys(); -- cgit v0.12 From 24885d4efa17d4232e266ef053899613c32fdeb7 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 15:44:26 +0200 Subject: cmDefinitions: Replace private constructor with MakeClosure. --- Source/cmDefinitions.cxx | 17 ++++++----------- Source/cmDefinitions.h | 11 +++-------- Source/cmMakefile.cxx | 2 +- 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index f2100eb..c4d285a 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -75,21 +75,16 @@ std::vector cmDefinitions::LocalKeys() const } //---------------------------------------------------------------------------- -cmDefinitions cmDefinitions::Closure() const -{ - return cmDefinitions(ClosureTag(), this); -} - -//---------------------------------------------------------------------------- -cmDefinitions::cmDefinitions(ClosureTag const&, cmDefinitions const* root): - Up(0) +cmDefinitions cmDefinitions::MakeClosure() const { std::set undefined; - this->ClosureImpl(undefined, root); + cmDefinitions closure; + closure.MakeClosure(undefined, this); + return closure; } //---------------------------------------------------------------------------- -void cmDefinitions::ClosureImpl(std::set& undefined, +void cmDefinitions::MakeClosure(std::set& undefined, cmDefinitions const* defs) { // Consider local definitions. @@ -114,7 +109,7 @@ void cmDefinitions::ClosureImpl(std::set& undefined, // Traverse parents. if(cmDefinitions const* up = defs->Up) { - this->ClosureImpl(undefined, up); + this->MakeClosure(undefined, up); } } diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 4664090..6917402 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -45,13 +45,11 @@ public: /** Get the set of all local keys. */ std::vector LocalKeys() const; - /** Compute the closure of all defined keys with values. - This flattens the scope. The result has no parent. */ - cmDefinitions Closure() const; - /** Compute the set of all defined keys. */ std::vector ClosureKeys() const; + cmDefinitions MakeClosure() const; + private: // String with existence boolean. struct Def: public std::string @@ -81,10 +79,7 @@ private: // Internal query and update methods. Def const& GetInternal(const std::string& key); - // Implementation of Closure() method. - struct ClosureTag {}; - cmDefinitions(ClosureTag const&, cmDefinitions const* root); - void ClosureImpl(std::set& undefined, + void MakeClosure(std::set& undefined, cmDefinitions const* defs); }; diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index e2a9f61..dbb355c 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -63,7 +63,7 @@ public: void InitializeDefinitions(cmMakefile* parent) { - this->VarStack.top() = parent->Internal->VarStack.top().Closure(); + this->VarStack.top() = parent->Internal->VarStack.top().MakeClosure(); } const char* GetDefinition(std::string const& name) -- cgit v0.12 From f983d8913b3293f0f328811d243222c6e19c795e Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 15:49:43 +0200 Subject: cmDefinitions: Replace recursion with loop. --- Source/cmDefinitions.cxx | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index c4d285a..2ee2618 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -87,29 +87,27 @@ cmDefinitions cmDefinitions::MakeClosure() const void cmDefinitions::MakeClosure(std::set& undefined, cmDefinitions const* defs) { - // Consider local definitions. - for(MapType::const_iterator mi = defs->Map.begin(); - mi != defs->Map.end(); ++mi) + while(defs) { - // Use this key if it is not already set or unset. - if(this->Map.find(mi->first) == this->Map.end() && - undefined.find(mi->first) == undefined.end()) + // Consider local definitions. + for(MapType::const_iterator mi = defs->Map.begin(); + mi != defs->Map.end(); ++mi) { - if(mi->second.Exists) - { - this->Map.insert(*mi); - } - else + // Use this key if it is not already set or unset. + if(this->Map.find(mi->first) == this->Map.end() && + undefined.find(mi->first) == undefined.end()) { - undefined.insert(mi->first); + if(mi->second.Exists) + { + this->Map.insert(*mi); + } + else + { + undefined.insert(mi->first); + } } } - } - - // Traverse parents. - if(cmDefinitions const* up = defs->Up) - { - this->MakeClosure(undefined, up); + defs = defs->Up; } } -- cgit v0.12 From aaaa65b6a5cdf227282a9d04bf5287413757ff03 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 25 Apr 2015 17:21:51 +0200 Subject: cmMakefile: Remove stack adaptor for the VarStack. The purpose of the stack is to allow access only to the top of it. Access to items which are not at the top is needed, so cmDefinitions objects get a Parent pointer. The existence of the Parent pointer is a workaround for the inappropriate use of stack in the first place. Remove it now. --- Source/cmMakefile.cxx | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index dbb355c..8cfb83a 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -46,7 +46,7 @@ class cmMakefile::Internals { public: - std::stack > VarStack; + std::list VarStack; std::stack > VarInitStack; std::stack > VarUsageStack; bool IsSourceFileTryCompile; @@ -56,24 +56,24 @@ public: cmDefinitions* parent = 0; if (!this->VarStack.empty()) { - parent = &this->VarStack.top(); + parent = &this->VarStack.back(); } - this->VarStack.push(cmDefinitions(parent)); + this->VarStack.push_back(cmDefinitions(parent)); } void InitializeDefinitions(cmMakefile* parent) { - this->VarStack.top() = parent->Internal->VarStack.top().MakeClosure(); + this->VarStack.back() = parent->Internal->VarStack.back().MakeClosure(); } const char* GetDefinition(std::string const& name) { - return this->VarStack.top().Get(name); + return this->VarStack.back().Get(name); } void SetDefinition(std::string const& name, std::string const& value) { - this->VarStack.top().Set(name, value.c_str()); + this->VarStack.back().Set(name, value.c_str()); } void RemoveDefinition(std::string const& name) @@ -81,32 +81,32 @@ public: if (this->VarStack.size() > 1) { // In lower scopes we store keys, defined or not. - this->VarStack.top().Set(name, 0); + this->VarStack.back().Set(name, 0); } else { - this->VarStack.top().Erase(name); + this->VarStack.back().Erase(name); } } std::vector LocalKeys() const { - return this->VarStack.top().LocalKeys(); + return this->VarStack.back().LocalKeys(); } std::vector ClosureKeys() const { - return this->VarStack.top().ClosureKeys(); + return this->VarStack.back().ClosureKeys(); } void PopDefinitions() { - this->VarStack.pop(); + this->VarStack.pop_back(); } bool RaiseScope(std::string const& var, const char* varDef, cmMakefile* mf) { - cmDefinitions& cur = this->VarStack.top(); + cmDefinitions& cur = this->VarStack.back(); if(cmDefinitions* up = cur.GetParent()) { // First localize the definition in the current scope. -- cgit v0.12 From d858f36339d61e45913165bc957d645bf1060f54 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 15:54:02 +0200 Subject: cmDefinitions: Use list of cmDefinitions* to create closure. --- Source/cmDefinitions.cxx | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 2ee2618..5fe57c8 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -11,6 +11,8 @@ ============================================================================*/ #include "cmDefinitions.h" +#include + //---------------------------------------------------------------------------- cmDefinitions::Def cmDefinitions::NoDef; @@ -87,11 +89,18 @@ cmDefinitions cmDefinitions::MakeClosure() const void cmDefinitions::MakeClosure(std::set& undefined, cmDefinitions const* defs) { + std::list ups; while(defs) { + ups.push_back(defs); + defs = defs->Up; + } + for (std::list::const_iterator it = ups.begin(); + it != ups.end(); ++it) + { // Consider local definitions. - for(MapType::const_iterator mi = defs->Map.begin(); - mi != defs->Map.end(); ++mi) + for(MapType::const_iterator mi = (*it)->Map.begin(); + 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() && @@ -107,7 +116,6 @@ void cmDefinitions::MakeClosure(std::set& undefined, } } } - defs = defs->Up; } } -- cgit v0.12 From 60becdc65c5f8cfad4b2c6a33e3649d2acbddf39 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 16:00:18 +0200 Subject: cmDefinitions: Implement MakeClosure in terms of a list of ancestors. --- Source/cmDefinitions.cxx | 25 +++++++++++++------------ Source/cmDefinitions.h | 5 ++++- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 5fe57c8..873a7b4 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -11,8 +11,6 @@ ============================================================================*/ #include "cmDefinitions.h" -#include - //---------------------------------------------------------------------------- cmDefinitions::Def cmDefinitions::NoDef; @@ -81,22 +79,25 @@ cmDefinitions cmDefinitions::MakeClosure() const { std::set undefined; cmDefinitions closure; - closure.MakeClosure(undefined, this); - return closure; -} - -//---------------------------------------------------------------------------- -void cmDefinitions::MakeClosure(std::set& undefined, - cmDefinitions const* defs) -{ + cmDefinitions const* defs = this; std::list ups; while(defs) { ups.push_back(defs); defs = defs->Up; } - for (std::list::const_iterator it = ups.begin(); - it != ups.end(); ++it) + closure.MakeClosure(undefined, ups.begin(), ups.end()); + return closure; +} + +//---------------------------------------------------------------------------- +void +cmDefinitions::MakeClosure(std::set& undefined, + std::list::iterator begin, + std::list::iterator end) +{ + for (std::list::const_iterator 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 6917402..40e0531 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -17,6 +17,8 @@ #include "cmsys/hash_map.hxx" #endif +#include + /** \class cmDefinitions * \brief Store a scope of variable definitions for CMake language. * @@ -80,7 +82,8 @@ private: Def const& GetInternal(const std::string& key); void MakeClosure(std::set& undefined, - cmDefinitions const* defs); + std::list::iterator begin, + std::list::iterator end); }; #endif -- cgit v0.12 From aa4d1ee80f1ced5b09335cc84bdd373c0875fd80 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 16:13:56 +0200 Subject: cmDefinitions: Convert MakeClosure into a static method. Accept a range of cmDefinitions*. --- Source/cmDefinitions.cxx | 13 ++++--------- Source/cmDefinitions.h | 4 +++- Source/cmMakefile.cxx | 10 +++++++++- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 873a7b4..d1fbe74 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -75,18 +75,13 @@ std::vector cmDefinitions::LocalKeys() const } //---------------------------------------------------------------------------- -cmDefinitions cmDefinitions::MakeClosure() const +cmDefinitions cmDefinitions::MakeClosure( + std::list::iterator begin, + std::list::iterator end) { std::set undefined; cmDefinitions closure; - cmDefinitions const* defs = this; - std::list ups; - while(defs) - { - ups.push_back(defs); - defs = defs->Up; - } - closure.MakeClosure(undefined, ups.begin(), ups.end()); + closure.MakeClosure(undefined, begin, end); return closure; } diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 40e0531..67b6170 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -50,7 +50,9 @@ public: /** Compute the set of all defined keys. */ std::vector ClosureKeys() const; - cmDefinitions MakeClosure() const; + static cmDefinitions MakeClosure( + std::list::iterator begin, + std::list::iterator end); private: // String with existence boolean. diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 8cfb83a..8797090 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -63,7 +63,15 @@ public: void InitializeDefinitions(cmMakefile* parent) { - this->VarStack.back() = parent->Internal->VarStack.back().MakeClosure(); + std::list defPtrs; + for (std::list::iterator it = + parent->Internal->VarStack.begin(); + it != parent->Internal->VarStack.end(); ++it) + { + defPtrs.push_back(&*it); + } + this->VarStack.back() = cmDefinitions::MakeClosure(defPtrs.begin(), + defPtrs.end()); } const char* GetDefinition(std::string const& name) -- cgit v0.12 From f79cd99d6dcdfcdcd341c5ea90a5f2d9c4d6d3bc Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Wed, 29 Apr 2015 23:48:43 +0200 Subject: cmDefinitions: Implement MakeClosure in terms of reverse iterators. Iterate directly over the parent content provided by cmMakefile. --- Source/cmDefinitions.cxx | 18 +++++++++--------- Source/cmDefinitions.h | 8 ++++---- Source/cmMakefile.cxx | 12 +++--------- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index d1fbe74..718d9ec 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -76,27 +76,27 @@ std::vector cmDefinitions::LocalKeys() const //---------------------------------------------------------------------------- cmDefinitions cmDefinitions::MakeClosure( - std::list::iterator begin, - std::list::iterator end) + std::list::const_reverse_iterator rbegin, + std::list::const_reverse_iterator rend) { std::set undefined; cmDefinitions closure; - closure.MakeClosure(undefined, begin, end); + closure.MakeClosure(undefined, rbegin, rend); return closure; } //---------------------------------------------------------------------------- void cmDefinitions::MakeClosure(std::set& undefined, - std::list::iterator begin, - std::list::iterator end) + std::list::const_reverse_iterator rbegin, + std::list::const_reverse_iterator rend) { - for (std::list::const_iterator it = begin; - it != end; ++it) + for (std::list::const_reverse_iterator it = rbegin; + it != rend; ++it) { // Consider local definitions. - for(MapType::const_iterator mi = (*it)->Map.begin(); - mi != (*it)->Map.end(); ++mi) + for(MapType::const_iterator mi = it->Map.begin(); + 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() && diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 67b6170..87f5c34 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -51,8 +51,8 @@ public: std::vector ClosureKeys() const; static cmDefinitions MakeClosure( - std::list::iterator begin, - std::list::iterator end); + std::list::const_reverse_iterator rbegin, + std::list::const_reverse_iterator rend); private: // String with existence boolean. @@ -84,8 +84,8 @@ private: Def const& GetInternal(const std::string& key); void MakeClosure(std::set& undefined, - std::list::iterator begin, - std::list::iterator end); + std::list::const_reverse_iterator rbegin, + std::list::const_reverse_iterator rend); }; #endif diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 8797090..6451874 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -63,15 +63,9 @@ public: void InitializeDefinitions(cmMakefile* parent) { - std::list defPtrs; - for (std::list::iterator it = - parent->Internal->VarStack.begin(); - it != parent->Internal->VarStack.end(); ++it) - { - defPtrs.push_back(&*it); - } - this->VarStack.back() = cmDefinitions::MakeClosure(defPtrs.begin(), - defPtrs.end()); + this->VarStack.back() = + cmDefinitions::MakeClosure(parent->Internal->VarStack.rbegin(), + parent->Internal->VarStack.rend()); } const char* GetDefinition(std::string const& name) -- cgit v0.12 From 5ccff6408c93e67d7f8445ce1bf6465b068d6f6b Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 15:38:36 +0200 Subject: cmDefinitions: Externalize looping for ClosureKeys. --- Source/cmDefinitions.cxx | 23 ++++++++--------------- Source/cmDefinitions.h | 4 ++-- Source/cmMakefile.cxx | 11 ++++++++++- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 718d9ec..115e30a 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -116,26 +116,19 @@ cmDefinitions::MakeClosure(std::set& undefined, } //---------------------------------------------------------------------------- -std::vector cmDefinitions::ClosureKeys() const +std::vector +cmDefinitions::ClosureKeys(std::set& bound) const { std::vector defined; - std::set bound; - - cmDefinitions const* up = this; - - while (up) + defined.reserve(this->Map.size()); + for(MapType::const_iterator mi = this->Map.begin(); + mi != this->Map.end(); ++mi) { - // Consider local definitions. - for(MapType::const_iterator mi = up->Map.begin(); - mi != up->Map.end(); ++mi) + // Use this key if it is not already set or unset. + if(bound.insert(mi->first).second && mi->second.Exists) { - // 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); - } + defined.push_back(mi->first); } - up = up->Up; } return defined; } diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 87f5c34..4c7f11f 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -47,8 +47,8 @@ public: /** Get the set of all local keys. */ std::vector LocalKeys() const; - /** Compute the set of all defined keys. */ - std::vector ClosureKeys() const; + std::vector + ClosureKeys(std::set& bound) const; static cmDefinitions MakeClosure( std::list::const_reverse_iterator rbegin, diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 6451874..1d3e496 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -98,7 +98,16 @@ public: std::vector ClosureKeys() const { - return this->VarStack.back().ClosureKeys(); + 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; } void PopDefinitions() -- cgit v0.12 From b48ea26a9d3b4384874554fe64edfce8784a9255 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 26 Apr 2015 12:39:53 +0200 Subject: cmDefinitions: Invert conditional code. Return the simple case first. --- Source/cmDefinitions.cxx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 115e30a..61328be 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -29,13 +29,14 @@ cmDefinitions::GetInternal(const std::string& key) { return i->second; } - if(cmDefinitions* up = this->Up) + cmDefinitions* up = this->Up; + if(!up) { - // Query the parent scope and store the result locally. - Def def = up->GetInternal(key); - return this->Map.insert(MapType::value_type(key, def)).first->second; + return this->NoDef; } - return this->NoDef; + // Query the parent scope and store the result locally. + Def def = up->GetInternal(key); + return this->Map.insert(MapType::value_type(key, def)).first->second; } //---------------------------------------------------------------------------- -- cgit v0.12