From ba3e637e9dd92e34b810a3be1aa01e09cbe40284 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jesper=20=C3=96qvist?= <joqvist@google.com>
Date: Sun, 29 May 2016 17:50:15 -0700
Subject: [PATCH] Partially decouple NTA proxy objects

Proxy objects are used to locate NTA children from parameterized NTAs
during inherited attribute evaluation. Previously the proxy object was a
list, and the list could be used to iterate over evaluated parameterized
NTA values.

Proxy objects are now plain ASTNode instances, and when not using
rewrites or using circular NTA rewrites they do not contain references
to the child nodes using them as proxy parents.

Null references are no longer stored in the proxy objects in any
configuration.

This change also renames the proxy objects from attrname_list to
attrname_proxy.

see #112 (bitbucket)
---
 ChangeLog                                |  8 ++++++
 src/template/ast/Attributes.tt           | 31 +++++++++++++++---------
 src/template/ast/Circular.tt             | 24 +++++++++---------
 src/template/ast/InheritedAttributes.tt  |  2 +-
 src/template/flush/Flush.tt              |  2 +-
 src/template/incremental/Debug.tt        | 12 ++++-----
 src/template/incremental/NTAs.tt         |  4 +--
 src/template/incremental/Notification.tt | 24 +++++++++---------
 src/template/incremental/State.tt        |  8 +++---
 9 files changed, 66 insertions(+), 49 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index ca8d92c7..3b7bd0ec 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2016-05-29  Jesper Öqvist <joqvist@google.com>
+
+    * Inherited equations on parameterized NTAs can no longer use the child
+    index which depended on evaluation order.
+    * Made parameterized NTA proxy object more loosely coupled to
+    parameterized NTA values in non-legacy rewrite mode (only setting parent
+    links to proxy object).
+
 2016-03-23  Jesper Öqvist <jesper.oqvist@cs.lth.se>
 
     * Added a new variation of the contributes statement to allow NTA children
diff --git a/src/template/ast/Attributes.tt b/src/template/ast/Attributes.tt
index 84db75b9..41df0ccb 100644
--- a/src/template/ast/Attributes.tt
+++ b/src/template/ast/Attributes.tt
@@ -56,7 +56,7 @@ $endif
 $else
 $if(#declaredNTA)
   /** @apilevel internal */
-  protected $List #(signature)_list;
+  protected $ASTNode #(signature)_proxy;
 $endif
 $if(LazyMaps)
   /** @apilevel internal */
@@ -233,12 +233,14 @@ $endif
 
 SynDecl.higherOrderAttributeCode:norewrite [[
 $if(#isParameterized)
-if (#(signature)_list == null) {
-  #(signature)_list = new $List();
+if (#(signature)_proxy == null) {
+  #(signature)_proxy = new $ASTNode();
   $include(AttrDecl.incHookCreateNtaList)
-  #(signature)_list.setParent(this);
+  #(signature)_proxy.setParent(this);
+}
+if (#(signature)_value != null) {
+  #(signature)_value.setParent(#(signature)_proxy);
 }
-#(signature)_list.add(#(signature)_value);
 $else
 #(signature)_value.setParent(this);
 $endif
@@ -246,19 +248,26 @@ $endif
 
 SynDecl.higherOrderAttributeCode:rewritesEnabled [[
 $if(#isParameterized)
-if (#(signature)_list == null) {
-  #(signature)_list = new $List();
+if (#(signature)_proxy == null) {
+  #(signature)_proxy = new $ASTNode();
   $include(AttrDecl.incHookCreateNtaList)
   $if(LegacyRewrite)
-  #(signature)_list.is$$Final = true;
+  #(signature)_proxy.is$$Final = true;
   $endif
-  #(signature)_list.setParent(this);
+  #(signature)_proxy.setParent(this);
 }
-#(signature)_list.add(#(signature)_value);
 if (#(signature)_value != null) {
-  #(signature)_value = (#boxedType) #(signature)_list.getChild(#(signature)_list.numChildren - 1);
   $if(LegacyRewrite)
+  #(signature)_proxy.addChild(#(signature)_value);
+  // Proxy child access is used to trigger rewrite of NTA value.
+  #(signature)_value = (#boxedType) #(signature)_proxy.getChild(#(signature)_proxy.numChildren - 1);
   #(signature)_value.is$$Final = true;
+  $else
+  #(signature)_value.setParent(#(signature)_proxy);
+  if (#(signature)_value.mayHaveRewrite()) {
+    #(signature)_value = (#boxedType) #(signature)_value.rewrittenNode();
+    #(signature)_value.setParent(#(signature)_proxy);
+  }
   $endif
 }
 $else
diff --git a/src/template/ast/Circular.tt b/src/template/ast/Circular.tt
index 1fb3873a..5614f7f0 100644
--- a/src/template/ast/Circular.tt
+++ b/src/template/ast/Circular.tt
@@ -165,11 +165,11 @@ AttrDecl.circularEquation:parameterized [[
       _value.value = $BottomValue;
 $if(#getNTA)
        if (_value.value != null) {
-         if (#(signature)_list == null) {
-           #(signature)_list = new $List();
-           #(signature)_list.setParent(#ntaParent);
+         if (#(signature)_proxy == null) {
+           #(signature)_proxy = new $ASTNode();
+           #(signature)_proxy.setParent(#ntaParent);
          }
-         (($ASTNode) _value.value).setParent(#(signature)_list);
+         (($ASTNode) _value.value).setParent(#(signature)_proxy);
       }
 $endif
     }
@@ -196,11 +196,11 @@ $endif
           _value.value = new_#(signature)_value;
 $if(#getNTA)
           if (_value.value != null) {
-            if (#(signature)_list == null) {
-              #(signature)_list = new $List();
-              #(signature)_list.setParent(#ntaParent);
+            if (#(signature)_proxy == null) {
+              #(signature)_proxy = new $ASTNode();
+              #(signature)_proxy.setParent(#ntaParent);
             }
-            (($ASTNode) _value.value).setParent(#(signature)_list);
+            (($ASTNode) _value.value).setParent(#(signature)_proxy);
           }
 $endif
         }
@@ -249,11 +249,11 @@ $endif
         _value.value = new_#(signature)_value;
 $if(#getNTA)
         if (_value.value != null) {
-          if (#(signature)_list == null) {
-            #(signature)_list = new $List();
-            #(signature)_list.setParent(#ntaParent);
+          if (#(signature)_proxy == null) {
+            #(signature)_proxy = new $ASTNode();
+            #(signature)_proxy.setParent(#ntaParent);
           }
-          (($ASTNode) _value.value).setParent(#(signature)_list);
+          (($ASTNode) _value.value).setParent(#(signature)_proxy);
         }
 $endif
       }
diff --git a/src/template/ast/InheritedAttributes.tt b/src/template/ast/InheritedAttributes.tt
index 2bf4e847..0283b7cf 100644
--- a/src/template/ast/InheritedAttributes.tt
+++ b/src/template/ast/InheritedAttributes.tt
@@ -122,7 +122,7 @@ $EvalStmt
 # non-terminal child equation clause
 InhEq.emitNTAClause [[
 $if(IsParameterized)
-if (_callerNode == $(AttrSignature)_list) {
+if (_callerNode == $(AttrSignature)_proxy) {
   // #declaredat
   int $ChildIndexVar = _callerNode.getIndexOfChild(_childNode);
   $EvalStmt
diff --git a/src/template/flush/Flush.tt b/src/template/flush/Flush.tt
index 37219f83..7edb61ce 100644
--- a/src/template/flush/Flush.tt
+++ b/src/template/flush/Flush.tt
@@ -137,7 +137,7 @@ $if(#isLazy)
 #(signature)_values = $CreateDefaultMap;
     $endif
     $if(#getNTA)
-#(signature)_list = null;
+#(signature)_proxy = null;
     $endif
   $else
     $if(#simpleCacheCheck)
diff --git a/src/template/incremental/Debug.tt b/src/template/incremental/Debug.tt
index c2d247a1..ef8b0ad5 100644
--- a/src/template/incremental/Debug.tt
+++ b/src/template/incremental/Debug.tt
@@ -224,8 +224,8 @@ if ($(AttrSign)_handler != null) {
 $endif
 $if (IsNTA)
 $if (IsParameterized)
-if ($(AttrSign)_list != null) {
-  $(AttrSign)_list.dumpDependencies();
+if ($(AttrSign)_proxy != null) {
+  $(AttrSign)_proxy.dumpDependencies();
 }
 $else
 if ($(AttrSign)_computed && ($(AttrSign)_value instanceof $ASTNode)) {
@@ -240,8 +240,8 @@ if ($(AttrSign)_handler != null) {
 }
 $if (IsNTA)
 $if (IsParameterized)
-if ($(AttrSign)_list != null) {
-  $(AttrSign)_list.dumpDependencies();
+if ($(AttrSign)_proxy != null) {
+  $(AttrSign)_proxy.dumpDependencies();
 }
 $else
 if ($(AttrSign)_computed && ($(AttrSign)_value instanceof $ASTNode)) {
@@ -270,8 +270,8 @@ public void #name.dumpDepsInTree() {
 # Generate string with code for dumping dependencies in NTAs
 ASTDecl.checkAndDumpNTADeps = [[
 $if (IsParameterized)
-if ($(AttrSign)_list != null) {
-  $(AttrSign)_list.dumpDepsInTree();
+if ($(AttrSign)_proxy != null) {
+  $(AttrSign)_proxy.dumpDepsInTree();
 }
 $else
 if ($(AttrSign)_computed && ($(AttrSign)_value instanceof $ASTNode)) {
diff --git a/src/template/incremental/NTAs.tt b/src/template/incremental/NTAs.tt
index 19efeb55..9fac34d9 100644
--- a/src/template/incremental/NTAs.tt
+++ b/src/template/incremental/NTAs.tt
@@ -63,13 +63,13 @@ $if(IncrementalEnabled)
 
 $if(IncrementalLevelNode)
 $if(#getNTA)
-#(signature)_list.inc_internalNTAList(#(signature)_values);
+#(signature)_proxy.inc_internalNTAList(#(signature)_values);
 $endif
 $endif
 
 $if(IncrementalLevelRegion)
 $if(#getNTA)
-#(signature)_list.inc_internalNTAList(#(signature)_values);
+#(signature)_proxy.inc_internalNTAList(#(signature)_values);
 $endif
 $endif
 
diff --git a/src/template/incremental/Notification.tt b/src/template/incremental/Notification.tt
index 2cb60e27..d8528d09 100644
--- a/src/template/incremental/Notification.tt
+++ b/src/template/incremental/Notification.tt
@@ -252,12 +252,12 @@ $if(IsParameterized)
 if (attrID.equals("$AttrSign") && $(AttrSign)_values != null && $(AttrSign)_values.containsKey(_parameters)) {
   $if(IsNTA)
   $AttrType value = ($AttrType)$(AttrSign)_values.remove(_parameters);
-  for (int i = 0; i < $(AttrSign)_list.children.length; i++) {
-    $ASTNode child = $(AttrSign)_list.children[i];
+  for (int i = 0; i < $(AttrSign)_proxy.children.length; i++) {
+    $ASTNode child = $(AttrSign)_proxy.children[i];
     if (child != null && value == child) {
       // using dummy node to flush dependencies from NTA
       value.inc_flush_subtree(($DDGNodeName)$(AttrSign)_handler.get(_parameters));
-      $(AttrSign)_list.removeChild(i);
+      $(AttrSign)_proxy.removeChild(i);
       break;
     }
   }
@@ -296,10 +296,10 @@ $if(IncrementalLevelAttr)
 $if(IsParameterized)
 if (attrID.equals("$AttrSign") && $(AttrSign)_values != null && !$(AttrSign)_values.isEmpty()) {
   $if(IsNTA)
-  if ($(AttrSign)_list != null) {
-    $(AttrSign)_list.setParent(null);
+  if ($(AttrSign)_proxy != null) {
+    $(AttrSign)_proxy.setParent(null);
     // using dummy node to flush dependencies from NTA
-    $(AttrSign)_list.inc_flush_subtree($(AttrSign)_handler);
+    $(AttrSign)_proxy.inc_flush_subtree($(AttrSign)_handler);
   }
   $endif
   $AttrResetVisit
@@ -763,12 +763,12 @@ $if(IncrementalChangeFlush)
 
 $if(IsNTA)
 $if(IsParameterized)
-if ($(AttrSign)_list != null) {
-  for (int index = 0; index < $(AttrSign)_list.numChildren; index++) {
-    $ASTNode value = $(AttrSign)_list.children[index];
+if ($(AttrSign)_proxy != null) {
+  for (int index = 0; index < $(AttrSign)_proxy.numChildren; index++) {
+    $ASTNode value = $(AttrSign)_proxy.children[index];
     if (!value.isRegionRoot()) {
       state().enterConstruction();
-      $(AttrSign)_list.removeChild(index);
+      $(AttrSign)_proxy.removeChild(index);
       // removeChild will decrease the index of all remaining children and numChildren
       // hence, to visit the remainder of the list index need to be decreased by one for each removeChild
       index--;
@@ -998,8 +998,8 @@ $if(IncrementalEnabled)
 $if(IncrementalChangeFlush)
 $if(IsNTA)
 $if(IsParameterized)
-if ($(AttrSign)_list != null) {
-  $(AttrSign)_list.inc_flush_subtree(h);
+if ($(AttrSign)_proxy != null) {
+  $(AttrSign)_proxy.inc_flush_subtree(h);
 }
 $else
 $if(IsNtaWithTree)
diff --git a/src/template/incremental/State.tt b/src/template/incremental/State.tt
index e0d32a5e..aa359045 100644
--- a/src/template/incremental/State.tt
+++ b/src/template/incremental/State.tt
@@ -250,8 +250,8 @@ if ($(AttrSign)_handler != null) {
 $endif
 $if(ChangeStateValue)
 $if(IsParameterized)
-if ($(AttrSign)_list != null) {
-  $(AttrSign)_list.inc_changeState(newState);
+if ($(AttrSign)_proxy != null) {
+  $(AttrSign)_proxy.inc_changeState(newState);
 }
 $else
 if ($(AttrSign)_computed && ($(AttrSign)_value instanceof $ASTNode)) {
@@ -363,8 +363,8 @@ if ($(AttrSign)_handler != null) {
 $endif
 $if(ThrowAwayValue)
 $if(IsParameterized)
-if ($(AttrSign)_list != null) {
-  $(AttrSign)_list.inc_throwAway();
+if ($(AttrSign)_proxy != null) {
+  $(AttrSign)_proxy.inc_throwAway();
 }
 $else
 if ($(AttrSign)_computed && ($(AttrSign)_value instanceof $ASTNode)) {
-- 
GitLab