cmMakefile: Remove VarInitStack.

In cmMakefile::PushScope, a copy of the closure of keys initialized
in the parent scope is made.  In PopScope, essentially the same copy
is inserted back into the parent.  That means a lot of duplication
of strings and a lot of string comparisons.  None of it is needed,
because the cmDefinitions keys already provide a canonical
representation of what is initialized.

The removal of the separate container also makes the variable handling
code more easy to reason about in general.

Before this patch, configuring llvm uses 200 KiB for the VarInitStack.
Overall peak memory consumption goes from 35.5 MiB to 35.1 MiB.

3 files changed, 28 insertions, 21 deletions

diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx
index 97a16ea234..5b03bd43a6 100644
--- a/Source/cmDefinitions.cxx
+++ b/Source/cmDefinitions.cxx
@@ -54,6 +54,20 @@ void cmDefinitions::Raise(const std::string& key,
   cmDefinitions::GetInternal(key, begin, end, true);
 }
 
+bool cmDefinitions::HasKey(const std::string& key,
+                           StackConstIter begin, StackConstIter end)
+{
+  for (StackConstIter it = begin; it != end; ++it)
+    {
+    MapType::const_iterator i = it->Map.find(key);
+    if (i != it->Map.end())
+      {
+      return true;
+      }
+    }
+  return false;
+}
+
 //----------------------------------------------------------------------------
 void cmDefinitions::Set(const std::string& key, const char* value)
 {
diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h
index 894ff7a028..bd3d392fcf 100644
--- a/Source/cmDefinitions.h
+++ b/Source/cmDefinitions.h
@@ -40,6 +40,9 @@ public:
 
   static void Raise(const std::string& key, StackIter begin, StackIter end);
 
+  static bool HasKey(const std::string& key,
+                     StackConstIter begin, StackConstIter end);
+
   /** 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 645a4332fb..bad4e6a858 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -47,7 +47,6 @@ class cmMakefile::Internals
 {
 public:
   std::list<cmDefinitions> VarStack;
-  std::stack<std::set<std::string> > VarInitStack;
   std::stack<std::set<std::string> > VarUsageStack;
   bool IsSourceFileTryCompile;
 
@@ -69,6 +68,12 @@ public:
                                     this->VarStack.rend());
   }
 
+  bool IsInitialized(std::string const& name)
+  {
+    return cmDefinitions::HasKey(name, this->VarStack.rbegin(),
+                                 this->VarStack.rend());
+  }
+
   void SetDefinition(std::string const& name, std::string const& value)
   {
     this->VarStack.back().Set(name, value.c_str());
@@ -137,7 +142,6 @@ cmMakefile::cmMakefile(cmLocalGenerator* localGenerator)
     StateSnapshot(localGenerator->GetStateSnapshot())
 {
   this->Internal->PushDefinitions();
-  this->Internal->VarInitStack.push(std::set<std::string>());
   this->Internal->VarUsageStack.push(std::set<std::string>());
   this->Internal->IsSourceFileTryCompile = false;
 
@@ -1719,13 +1723,12 @@ void cmMakefile::AddDefinition(const std::string& name, const char* value)
     return;
     }
 
-  this->Internal->SetDefinition(name, value);
   if (this->VariableInitialized(name))
     {
     this->LogUnused("changing definition", name);
     this->Internal->VarUsageStack.top().erase(name);
     }
-  this->Internal->VarInitStack.top().insert(name);
+  this->Internal->SetDefinition(name, value);
 
 #ifdef CMAKE_BUILD_WITH_CMAKE
   cmVariableWatch* vv = this->GetVariableWatch();
@@ -1794,13 +1797,12 @@ void cmMakefile::AddCacheDefinition(const std::string& name, const char* value,
 
 void cmMakefile::AddDefinition(const std::string& name, bool value)
 {
-  this->Internal->SetDefinition(name, value ? "ON" : "OFF");
   if (this->VariableInitialized(name))
     {
     this->LogUnused("changing definition", name);
     this->Internal->VarUsageStack.top().erase(name);
     }
-  this->Internal->VarInitStack.top().insert(name);
+  this->Internal->SetDefinition(name, value ? "ON" : "OFF");
 #ifdef CMAKE_BUILD_WITH_CMAKE
   cmVariableWatch* vv = this->GetVariableWatch();
   if ( vv )
@@ -1832,12 +1834,7 @@ void cmMakefile::MarkVariableAsUsed(const std::string& var)
 
 bool cmMakefile::VariableInitialized(const std::string& var) const
 {
-  if(this->Internal->VarInitStack.top().find(var) !=
-      this->Internal->VarInitStack.top().end())
-    {
-    return true;
-    }
-  return false;
+  return this->Internal->IsInitialized(var);
 }
 
 bool cmMakefile::VariableUsed(const std::string& var) const
@@ -1891,13 +1888,12 @@ void cmMakefile::LogUnused(const char* reason,
 
 void cmMakefile::RemoveDefinition(const std::string& name)
 {
-  this->Internal->RemoveDefinition(name);
   if (this->VariableInitialized(name))
     {
     this->LogUnused("unsetting", name);
     this->Internal->VarUsageStack.top().erase(name);
     }
-  this->Internal->VarInitStack.top().insert(name);
+  this->Internal->RemoveDefinition(name);
 #ifdef CMAKE_BUILD_WITH_CMAKE
   cmVariableWatch* vv = this->GetVariableWatch();
   if ( vv )
@@ -4316,9 +4312,7 @@ std::string cmMakefile::FormatListFileStack() const
 void cmMakefile::PushScope()
 {
   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->VarInitStack.push(init);
   this->Internal->VarUsageStack.push(usage);
 
   this->PushLoopBlockBarrier();
@@ -4336,7 +4330,6 @@ void cmMakefile::PopScope()
 
   this->PopLoopBlockBarrier();
 
-  std::set<std::string> init = this->Internal->VarInitStack.top();
   std::set<std::string> usage = this->Internal->VarUsageStack.top();
   const std::vector<std::string>& locals = this->Internal->LocalKeys();
   // Remove initialization and usage information for variables in the local
@@ -4344,7 +4337,6 @@ void cmMakefile::PopScope()
   std::vector<std::string>::const_iterator it = locals.begin();
   for (; it != locals.end(); ++it)
     {
-    init.erase(*it);
     if (!this->VariableUsed(*it))
       {
       this->LogUnused("out of scope", *it);
@@ -4356,10 +4348,8 @@ void cmMakefile::PopScope()
     }
 
   this->Internal->PopDefinitions();
-  this->Internal->VarInitStack.pop();
   this->Internal->VarUsageStack.pop();
-  // Push initialization and usage up to the parent scope.
-  this->Internal->VarInitStack.top().insert(init.begin(), init.end());
+  // Push usage up to the parent scope.
   this->Internal->VarUsageStack.top().insert(usage.begin(), usage.end());
 }