From 0f070dd3e12408e0e33945c3c8f585268c4b0db9 Mon Sep 17 00:00:00 2001
From: Stephen Kelly <steveire@gmail.com>
Date: Sun, 31 May 2015 11:26:05 +0200
Subject: [PATCH] cmMakefile: Decouple the container of cmDefinitions from
 scoping logic.

Maintain a Parent tree node for writing to in RaiseScope.
---
 Source/cmMakefile.cxx | 80 ++++++++++++++++++++++++++++++-------------
 1 file changed, 56 insertions(+), 24 deletions(-)

diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index afb016640..aef016e60 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -47,73 +47,105 @@
 class cmMakefile::Internals
 {
 public:
+
+  enum ScopeType
+  {
+    BaseScope,
+    MacroScope,
+    FunctionScope,
+    VariableScope
+  };
+
+  struct VarScope
+  {
+    ScopeType Type;
+    cmLinkedTree<cmDefinitions>::iterator Vars;
+    cmLinkedTree<cmDefinitions>::iterator ParentScope;
+  };
+
   cmLinkedTree<cmDefinitions> VarTree;
-  cmLinkedTree<cmDefinitions>::iterator VarTreeIter;
+  cmLinkedTree<VarScope> VarScopes;
+  cmLinkedTree<VarScope>::iterator VarScopeIter;
   bool IsSourceFileTryCompile;
 
-  void PushDefinitions()
+  void PushDefinitions(ScopeType scope)
   {
-    assert(this->VarTreeIter.IsValid());
-    this->VarTreeIter = this->VarTree.Extend(this->VarTreeIter);
+    assert(this->VarScopeIter.IsValid());
+    assert(this->VarScopeIter->Vars.IsValid());
+    cmLinkedTree<cmDefinitions>::iterator origin =
+        this->VarScopeIter->Vars;
+    cmLinkedTree<cmDefinitions>::iterator parentScope =
+//         this->VarTree.Extend(origin);
+        origin;
+    this->VarScopeIter->Vars = parentScope;
+    this->VarScopeIter = this->VarScopes.Extend(this->VarScopeIter);
+    this->VarScopeIter->ParentScope = parentScope;
+    this->VarScopeIter->Vars = this->VarTree.Extend(origin);
+    this->VarScopeIter->Type = scope;
   }
 
   void InitializeVarScope()
   {
-    this->VarTreeIter = this->VarTree.Root();
-    this->PushDefinitions();
+    assert(!this->VarScopeIter.IsValid());
+    this->VarScopeIter = this->VarScopes.Extend(this->VarScopes.Root());
+    this->VarScopeIter->Vars = this->VarTree.Extend(this->VarTree.Root());
+    this->VarScopeIter->ParentScope = this->VarTree.Root();
+    this->VarScopeIter->Type = BaseScope;
   }
 
   void InitializeDefinitions(cmMakefile* parent)
   {
-    *this->VarTreeIter =
-        cmDefinitions::MakeClosure(parent->Internal->VarTreeIter,
+    assert(this->VarScopeIter.IsValid());
+    assert(this->VarScopeIter->Vars.IsValid());
+    *this->VarScopeIter->Vars =
+        cmDefinitions::MakeClosure(parent->Internal->VarScopeIter->Vars,
                                    parent->Internal->VarTree.Root());
   }
 
   const char* GetDefinition(std::string const& name)
   {
-    assert(this->VarTreeIter != this->VarTree.Root());
+    assert(this->VarScopeIter.IsValid());
+    assert(this->VarScopeIter->Vars.IsValid());
     return cmDefinitions::Get(name,
-                              this->VarTreeIter, this->VarTree.Root());
+                              this->VarScopeIter->Vars, this->VarTree.Root());
   }
 
   bool IsInitialized(std::string const& name)
   {
     return cmDefinitions::HasKey(name,
-                                 this->VarTreeIter, this->VarTree.Root());
+                              this->VarScopeIter->Vars, this->VarTree.Root());
   }
 
   void SetDefinition(std::string const& name, std::string const& value)
   {
-    this->VarTreeIter->Set(name, value.c_str());
+    this->VarScopeIter->Vars->Set(name, value.c_str());
   }
 
   void RemoveDefinition(std::string const& name)
   {
-    this->VarTreeIter->Set(name, 0);
+    this->VarScopeIter->Vars->Set(name, 0);
   }
 
   std::vector<std::string> UnusedKeys() const
   {
-    return this->VarTreeIter->UnusedKeys();
+    return this->VarScopeIter->Vars->UnusedKeys();
   }
 
   std::vector<std::string> ClosureKeys() const
   {
-    return cmDefinitions::ClosureKeys(this->VarTreeIter, this->VarTree.Root());
+    return cmDefinitions::ClosureKeys(this->VarScopeIter->Vars,
+                                      this->VarTree.Root());
   }
 
   void PopDefinitions()
   {
-    ++this->VarTreeIter;
+    ++this->VarScopeIter;
   }
 
   bool RaiseScope(std::string const& var, const char* varDef, cmMakefile* mf)
   {
-    cmLinkedTree<cmDefinitions>::iterator it = this->VarTreeIter;
-    assert(it != this->VarTree.Root());
-    ++it;
-    if(it == this->VarTree.Root())
+    assert(this->VarScopeIter->Vars != this->VarTree.Root());
+    if(this->VarScopeIter->ParentScope == this->VarTree.Root())
       {
       cmLocalGenerator* plg = mf->LocalGenerator->GetParent();
       if(!plg)
@@ -135,10 +167,10 @@ public:
       return true;
       }
     // First localize the definition in the current scope.
-    cmDefinitions::Raise(var, this->VarTreeIter, this->VarTree.Root());
+    cmDefinitions::Raise(var, this->VarScopeIter->Vars, this->VarTree.Root());
 
     // Now update the definition in the parent scope.
-    it->Set(var, varDef);
+    this->VarScopeIter->ParentScope->Set(var, varDef);
     return true;
   }
 };
@@ -1642,7 +1674,7 @@ void cmMakefile::PushFunctionScope(std::string const& fileName,
         fileName);
   assert(this->StateSnapshot.IsValid());
 
-  this->Internal->PushDefinitions();
+  this->Internal->PushDefinitions(Internals::FunctionScope);
 
   this->PushLoopBlockBarrier();
 
@@ -4430,7 +4462,7 @@ std::string cmMakefile::FormatListFileStack() const
 
 void cmMakefile::PushScope()
 {
-  this->Internal->PushDefinitions();
+  this->Internal->PushDefinitions(Internals::VariableScope);
 
   this->PushLoopBlockBarrier();