From 2e1df16bdce56572b776d98dc0559383bd0cfa78 Mon Sep 17 00:00:00 2001
From: Ichthyostega <prg@ichthyostega.de>
Date: Sun, 5 Apr 2015 18:26:49 +0200
Subject: [PATCH] settle on a concrete implementation approach based on
 inheritance chain

After some reconsideration, I decide to stick to the approach with the closures,
but to use a metaprotramming technique to build an inheritance chain.
While I can not decide on the real world impact of storing all those closures,
in theory this approach should enable the compiler to remove all of the
storage overhead. Since, when storing the result into an auto variable
right within scope (as demonstrated in the test), the compiler
sees the concrete type and might be able to boil down the actual
generated virtual function implementations, thereby inlining the
given closures.

Whereas, on the other hand, if we'd go the obvious conventional route
and place the closures into a Map allocated on the stack, I wouldn't
expect the compiler to do data flow analysis to prove this allocation
is not necessary and inline it away.


NOTE: there is now guarantee this inlining trick will ever work.
And, moreover, we don't know anything regarding the runtime effect.
The whole picture is way more involved as it might seem at first sight.
Even if we go the completely conventional route and require every
participating object to supply an implementation of some kind of
"Serializable" interface, we'll end up with a (hand written!)
implementation class for each participating setup, which takes
up space in the code segment of the executable. While the closure
based approach chosen here, consumes data segment (or heap) space
per instance for the functors (or function pointers) representing
the closures, plus code segment space for the closures, but the
latter with a way higher potential for inlining, since the closure
code and the generated virtual functions are necessarily emitted
within the same compilation unit and within a local (inline, not
publickly exposed) scope.
---
 src/lib/diff/tree-mutator.hpp                 | 98 ++++++++++++-------
 .../diff/generic-tree-mutator-test.cpp        | 25 +++--
 2 files changed, 82 insertions(+), 41 deletions(-)

diff --git a/src/lib/diff/tree-mutator.hpp b/src/lib/diff/tree-mutator.hpp
index 2a13bdbc3..aae3cbd0b 100644
--- a/src/lib/diff/tree-mutator.hpp
+++ b/src/lib/diff/tree-mutator.hpp
@@ -73,15 +73,8 @@ namespace diff{
   using std::string;
   
   namespace {
-    class ChangeOperationBinder
-      {
-      public:
-        void
-        operator= (function<void(string)> closure)
-          {
-            UNIMPLEMENTED("install closure");
-          }
-      };
+    template<class PAR>
+    class Builder;
     
     ////////TODO only preliminary....
     typedef Literal ID;
@@ -106,59 +99,94 @@ namespace diff{
       
       /* ==== operation API ==== */
       
-      void
-      reiterate()
-        {
-          UNIMPLEMENTED("reset point of reference");
-        }
-      
-      void
+      virtual void
       insertChild (ID id)
         {
           UNIMPLEMENTED("establish new child node at current position");
         }
       
-      void
+      virtual void
       deleteChild (ID id)
         {
           UNIMPLEMENTED("destroy child node at current position");
         }
       
-      void
-      acceptChild (ID id)
-        {
-          UNIMPLEMENTED("acknowledge existence of child at current pos and move ahead");
-        }
-      
-      void
+      virtual void
       findChild (ID id)
         {
           UNIMPLEMENTED("look ahead, find and retrieve denoted child to be relocated at current position");
         }
       
-      TreeMutator&
+      virtual TreeMutator&
       mutateChild (ID id)
         {
           UNIMPLEMENTED("expose a recursive TreeMutator to transform the denoted child");
         }
       
-      void
+      virtual void
       setAttribute (ID id, Attribute newValue)
         {
-          UNIMPLEMENTED("apply a value change to the named attribute");
+          std::cout << "Empty Base Impl: apply a value change to the named attribute"<<std::endl;      ////////////////TODO empty implementation should be NOP
         }
       
-      
-      /* ==== binding API ==== */
-      
-      ChangeOperationBinder
-      change (Literal attributeID)
-        {
-          UNIMPLEMENTED ("setup binder");
-        }
+      /**
+       * start building a custom adapted tree mutator,
+       * where the operations are tied by closures or
+       * wrappers into the current implementation context.
+       */
+      static Builder<TreeMutator> build();
     };
   
+  namespace {
+    
+    template<class PAR>
+    struct ChangeOperation
+      : PAR
+      {
+        function<void(string)> change_;
+        
+        virtual void
+        setAttribute (ID id, Attribute newValue)
+          {
+            PAR::setAttribute(id, newValue);  ////////////////////TODO only as a check to verify the class chaining. Of course we do *not* want to call the inherited implementeation
+            
+            string dummy("blubb");
+            change_(dummy);
+          }
+        
+        ChangeOperation(function<void(string)> clo, PAR const& chain)
+          : PAR(chain)
+          , change_(clo)
+          { }
+      };
+    
+    template<class PAR>
+    struct Builder
+      : PAR
+      {
+        using Chain = ChangeOperation<PAR>;
+        
+        Builder(PAR par)
+          : PAR(par)
+          { }
+        
+        
+        /* ==== binding API ==== */
+        
+        Builder<Chain>
+        change (Literal attributeID, function<void(string)> closure)
+          {
+            return Builder<Chain> (Chain (closure, *this));
+          }
+      };
+
+  }
   
+  Builder<TreeMutator>
+  TreeMutator::build ()
+  {
+    return Builder<TreeMutator>(TreeMutator());
+  }
   
   
 }} // namespace lib::diff
diff --git a/tests/library/diff/generic-tree-mutator-test.cpp b/tests/library/diff/generic-tree-mutator-test.cpp
index 2a75f7832..1cef76642 100644
--- a/tests/library/diff/generic-tree-mutator-test.cpp
+++ b/tests/library/diff/generic-tree-mutator-test.cpp
@@ -38,6 +38,9 @@ using std::string;
 using std::cout;
 using std::endl;
 
+using lib::test::showType;
+using lib::test::demangleCxx;
+
 
 namespace lib {
 namespace diff{
@@ -87,15 +90,25 @@ namespace test{
       simpleAttributeBinding()
         {
           string localData;
-          TreeMutator mutator;
+          auto mutator =
+          TreeMutator::build()
+            .change("something", [&](string val)
+              {
+                cout << "Oink-Oink"<<endl;
+                localData = "Oink-Oink";
+              })
+            .change("something", [&](string val)
+              {
+                cout << "Change closure invoked with val="<<val<<endl;
+                localData = val;
+              });
           
-          mutator.change("something") = [&](string val)
-            {
-              cout << "Change closure invoked with val="<<val<<endl;
-              localData = val;
-            };
+          cout << "concrete TreeMutator type="<< demangleCxx (showType (mutator)) <<endl;
           
           CHECK (isnil (localData));
+          mutator.setAttribute("zoing", {});
+          CHECK (!isnil (localData));
+          cout << "localData changed to:"<<localData<<endl;
         }