From c49dd04b4428a3acaec9aae0fc01d73105d3b40f Mon Sep 17 00:00:00 2001
From: Ichthyostega <prg@ichthyostega.de>
Date: Sat, 26 Mar 2016 00:48:38 +0100
Subject: [PATCH] address an insidious dangling reference

I still feel somewhat queasy with this whole situation!
We need to return the product of the DSL/Builder by value,
but we also want to swap away the current contents before
starting the mutation, and we do not want a stateful lifecycle
for the mutator implementation. Which means, we need to swap
right at construction, and then we copy -- TADAAA!

Thus I'm going for the solution to disallow copying of the
mutator, yet to allow moving, and to change the builder
to move its product into place. Probably should even push
this policy up into the base class (TreeMutator) to set
everyone straight.

Looks like this didn't show up with the test dummy implementation
just because in this case the src buffer also lived within th
TestMutationTarget, which is assumed to sit where it is, so
effectively we moved around only pointers.
---
 src/lib/diff/tree-mutator-collection-binding.hpp     | 12 ++++++++++++
 src/lib/diff/tree-mutator.hpp                        |  4 ++--
 .../library/diff/tree-manipulation-binding-test.cpp  |  4 ++++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/lib/diff/tree-mutator-collection-binding.hpp b/src/lib/diff/tree-mutator-collection-binding.hpp
index 50da11331..a0476eade 100644
--- a/src/lib/diff/tree-mutator-collection-binding.hpp
+++ b/src/lib/diff/tree-mutator-collection-binding.hpp
@@ -174,6 +174,7 @@
         using Iter = typename BIN::iterator;
         
         BIN binding_;
+      public: ///////////////TODO: diagnostics
         Iter pos_;
         
         
@@ -184,8 +185,19 @@
           , pos_(binding_.initMutation())
           { }
         
+        ChildCollectionMutator (ChildCollectionMutator&&)       =default;
+        ChildCollectionMutator (ChildCollectionMutator const&)  =delete;
+        ChildCollectionMutator& operator= (ChildCollectionMutator const&) =delete;
         
         
+        /////////////////TODO : diagnostics
+        typename BIN::const_iterator
+        exposeSrcBuffer()  const
+          {
+            return eachElm (binding_.contentBuffer);
+          }
+        /////////////////TODO : diagnostics
+        
         /* ==== re-Implementation of the operation API ==== */
         
         /** skip next pending src element,
diff --git a/src/lib/diff/tree-mutator.hpp b/src/lib/diff/tree-mutator.hpp
index 57fa6ebed..5f0bd4e86 100644
--- a/src/lib/diff/tree-mutator.hpp
+++ b/src/lib/diff/tree-mutator.hpp
@@ -333,8 +333,8 @@ namespace diff{
     struct Builder
       : PAR
       {
-        Builder(PAR par)
-          : PAR(par)
+        Builder(PAR&& par)
+          : PAR(std::forward<PAR>(par))
           { }
         
         template<class CLO>
diff --git a/tests/library/diff/tree-manipulation-binding-test.cpp b/tests/library/diff/tree-manipulation-binding-test.cpp
index 213d3e6b4..09d045eb2 100644
--- a/tests/library/diff/tree-manipulation-binding-test.cpp
+++ b/tests/library/diff/tree-manipulation-binding-test.cpp
@@ -354,6 +354,7 @@ namespace test{
           CHECK (isnil (contents));
           
           cout << "injected......" << join(target) <<endl;
+          cout << "exhausted....." << join(mutator.exposeSrcBuffer()) <<endl;;
           
           
           // --- second round: reorder the collection ---
@@ -376,6 +377,9 @@ namespace test{
                           }));
           
           CHECK (isnil (target));                   // the "visible" new content is still void
+          cout << "target......" << join(target) <<endl;;
+          cout << "srcBuff....." << join(mutator2.exposeSrcBuffer()) <<endl;;
+          cout << "pos_........" << *mutator2.pos_ <<endl;;
           
           CHECK (mutator2.matchSrc (ATTRIB1));      // current head element of src "matches" the given spec
           CHECK (isnil (target));                   // the match didn't change anything