Merge topic 'refactor-cmDefinitions'

b48ea26a cmDefinitions: Invert conditional code.
5ccff640 cmDefinitions: Externalize looping for ClosureKeys.
f79cd99d cmDefinitions: Implement MakeClosure in terms of reverse iterators.
aa4d1ee8 cmDefinitions: Convert MakeClosure into a static method.
60becdc6 cmDefinitions: Implement MakeClosure in terms of a list of ancestors.
d858f363 cmDefinitions: Use list of cmDefinitions* to create closure.
aaaa65b6 cmMakefile: Remove stack adaptor for the VarStack.
f983d891 cmDefinitions: Replace recursion with loop.
24885d4e cmDefinitions: Replace private constructor with MakeClosure.
012a75a0 cmDefinitions: Make ClosureKeys API vector-based.
ca9fa77d cmDefinitions: Inline GetClosureKeys implementation.
78e1454e cmDefinitions: Replace ClosureKeys recursion with looping.
818bf727 cmDefinitions: Change LocalKeys to return a vector.
5067ae41 cmDefinitions: Externalize the Set logic.
60200ca5 cmDefinitions: Add an Erase method.
b43c162e cmMakefile: Use the Internal class to enclose the VarStack.

1 files changed, 117 insertions, 50 deletions

diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 9f1d107..bd42f4e 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -46,10 +46,107 @@
 class cmMakefile::Internals
 {
 public:
-  std::stack<cmDefinitions, std::list<cmDefinitions> > VarStack;
+  std::list<cmDefinitions> VarStack;
   std::stack<std::set<std::string> > VarInitStack;
   std::stack<std::set<std::string> > VarUsageStack;
   bool IsSourceFileTryCompile;
+
+  void PushDefinitions()
+  {
+    cmDefinitions* parent = 0;
+    if (!this->VarStack.empty())
+      {
+      parent = &this->VarStack.back();
+      }
+    this->VarStack.push_back(cmDefinitions(parent));
+  }
+
+  void InitializeDefinitions(cmMakefile* parent)
+  {
+    this->VarStack.back() =
+        cmDefinitions::MakeClosure(parent->Internal->VarStack.rbegin(),
+                                   parent->Internal->VarStack.rend());
+  }
+
+  const char* GetDefinition(std::string const& name)
+  {
+    return this->VarStack.back().Get(name);
+  }
+
+  void SetDefinition(std::string const& name, std::string const& value)
+  {
+    this->VarStack.back().Set(name, value.c_str());
+  }
+
+  void RemoveDefinition(std::string const& name)
+  {
+    if (this->VarStack.size() > 1)
+      {
+      // In lower scopes we store keys, defined or not.
+      this->VarStack.back().Set(name, 0);
+      }
+    else
+      {
+      this->VarStack.back().Erase(name);
+      }
+  }
+
+  std::vector<std::string> LocalKeys() const
+  {
+    return this->VarStack.back().LocalKeys();
+  }
+
+  std::vector<std::string> ClosureKeys() const
+  {
+    std::vector<std::string> closureKeys;
+    std::set<std::string> bound;
+    for (std::list<cmDefinitions>::const_reverse_iterator it =
+         this->VarStack.rbegin(); it != this->VarStack.rend(); ++it)
+      {
+      std::vector<std::string> const& localKeys = it->ClosureKeys(bound);
+      closureKeys.insert(closureKeys.end(),
+                         localKeys.begin(), localKeys.end());
+      }
+    return closureKeys;
+  }
+
+  void PopDefinitions()
+  {
+    this->VarStack.pop_back();
+  }
+
+  bool RaiseScope(std::string const& var, const char* varDef, cmMakefile* mf)
+  {
+    cmDefinitions& cur = this->VarStack.back();
+    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 +156,9 @@ cmMakefile::cmMakefile(cmLocalGenerator* localGenerator)
     StateSnapshot(localGenerator->GetGlobalGenerator()
                                 ->GetCMakeInstance()->GetState())
 {
-  const cmDefinitions& defs = cmDefinitions();
-  const std::set<std::string> 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<std::string>());
+  this->Internal->VarUsageStack.push(std::set<std::string>());
   this->Internal->IsSourceFileTryCompile = false;
 
   if (this->LocalGenerator->GetParent())
@@ -1496,7 +1591,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());
@@ -1690,7 +1785,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))
     {
@@ -1760,13 +1855,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))
     {
@@ -1790,9 +1885,8 @@ void cmMakefile::CheckForUnusedVariables() const
     {
     return;
     }
-  const cmDefinitions& defs = this->Internal->VarStack.top();
-  const std::set<std::string>& locals = defs.LocalKeys();
-  std::set<std::string>::const_iterator it = locals.begin();
+  const std::vector<std::string>& locals = this->Internal->LocalKeys();
+  std::vector<std::string>::const_iterator it = locals.begin();
   for (; it != locals.end(); ++it)
     {
     this->CheckForUnused("out of scope", *it);
@@ -1865,7 +1959,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))
     {
@@ -2338,7 +2432,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)
     {
@@ -2364,7 +2458,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);
@@ -2404,9 +2498,7 @@ std::vector<std::string> cmMakefile
   std::vector<std::string> res;
   if ( !cacheonly )
     {
-    std::set<std::string> definitions =
-        this->Internal->VarStack.top().ClosureKeys();
-    res.insert(res.end(), definitions.begin(), definitions.end());
+    res = this->Internal->ClosureKeys();
     }
   std::vector<std::string> cacheKeys =
       this->GetState()->GetCacheEntryKeys();
@@ -4297,10 +4389,9 @@ std::string cmMakefile::GetListFileStack() const
 
 void cmMakefile::PushScope()
 {
-  cmDefinitions* parent = &this->Internal->VarStack.top();
+  this->Internal->PushDefinitions();
   const std::set<std::string>& init = this->Internal->VarInitStack.top();
   const std::set<std::string>& usage = this->Internal->VarUsageStack.top();
-  this->Internal->VarStack.push(cmDefinitions(parent));
   this->Internal->VarInitStack.push(init);
   this->Internal->VarUsageStack.push(usage);
 
@@ -4321,13 +4412,12 @@ void cmMakefile::PopScope()
 
   this->PopLoopBlockBarrier();
 
-  cmDefinitions* current = &this->Internal->VarStack.top();
   std::set<std::string> init = this->Internal->VarInitStack.top();
   std::set<std::string> usage = this->Internal->VarUsageStack.top();
-  const std::set<std::string>& locals = current->LocalKeys();
+  const std::vector<std::string>& locals = this->Internal->LocalKeys();
   // Remove initialization and usage information for variables in the local
   // scope.
-  std::set<std::string>::const_iterator it = locals.begin();
+  std::vector<std::string>::const_iterator it = locals.begin();
   for (; it != locals.end(); ++it)
     {
     init.erase(*it);
@@ -4340,7 +4430,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.
@@ -4355,31 +4446,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.";