From ad6d348d8fd2869809f12e0fbde8d024fc8b2df9 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sat, 26 Mar 2016 02:01:31 +0100 Subject: [PATCH] make TreeMutator noncopyable to prevent dangling references since we're moving elements around to apply the diff, dangerous situation might arise in case anyone takes a copy of the mutator. Thus we effectively limit the possible usage pattern and only allow to build an anonymous TreeMutator subclass through the Builder-DSL. The concrete "onion layers" of the TreeMutator are now limited - to be created by the chaining operations of the Builder DSl - to be moved into target location, retaining ownership. --- src/lib/diff/test-mutation-target.hpp | 6 +- .../diff/tree-mutator-attribute-binding.hpp | 4 +- .../diff/tree-mutator-collection-binding.hpp | 7 +-- src/lib/diff/tree-mutator.hpp | 55 ++++++++++++++----- 4 files changed, 47 insertions(+), 25 deletions(-) diff --git a/src/lib/diff/test-mutation-target.hpp b/src/lib/diff/test-mutation-target.hpp index 68d1c8692..eeb091f72 100644 --- a/src/lib/diff/test-mutation-target.hpp +++ b/src/lib/diff/test-mutation-target.hpp @@ -363,8 +363,8 @@ namespace diff{ Iter pos_; public: - TestWireTap(Target& dummy, PAR const& chain) - : PAR(chain) + TestWireTap(Target& dummy, PAR&& chain) + : PAR{forward (chain)} , target_(dummy) , pos_(target_.initMutation (identify(this))) { } @@ -517,7 +517,7 @@ namespace diff{ Builder> Builder::attachDummy (TestMutationTarget& dummy) { - return WireTap (dummy, *this); + return WireTap (dummy, move(*this)); } } diff --git a/src/lib/diff/tree-mutator-attribute-binding.hpp b/src/lib/diff/tree-mutator-attribute-binding.hpp index d4c6b592f..a658cf870 100644 --- a/src/lib/diff/tree-mutator-attribute-binding.hpp +++ b/src/lib/diff/tree-mutator-attribute-binding.hpp @@ -76,8 +76,8 @@ PAR::setAttribute(id, newValue); } - ChangeOperation(ID id, CLO clo, PAR const& chain) - : PAR(chain) + ChangeOperation(ID id, CLO clo, PAR&& chain) + : PAR(std::forward(chain)) , attribID_(id) , change_(clo) { } diff --git a/src/lib/diff/tree-mutator-collection-binding.hpp b/src/lib/diff/tree-mutator-collection-binding.hpp index 1afe7d873..8d61182bb 100644 --- a/src/lib/diff/tree-mutator-collection-binding.hpp +++ b/src/lib/diff/tree-mutator-collection-binding.hpp @@ -178,15 +178,12 @@ public: - ChildCollectionMutator(BIN wiringClosures, PAR const& chain) - : PAR(chain) + ChildCollectionMutator(BIN wiringClosures, PAR&& chain) + : PAR(std::forward(chain)) , binding_(wiringClosures) , pos_(binding_.initMutation()) { } - ChildCollectionMutator (ChildCollectionMutator&&) =default; - ChildCollectionMutator (ChildCollectionMutator const&) =delete; - ChildCollectionMutator& operator= (ChildCollectionMutator const&) =delete; diff --git a/src/lib/diff/tree-mutator.hpp b/src/lib/diff/tree-mutator.hpp index 5f0bd4e86..0d87405d4 100644 --- a/src/lib/diff/tree-mutator.hpp +++ b/src/lib/diff/tree-mutator.hpp @@ -45,8 +45,8 @@ ** TreeMutator is both an interface and a set of building blocks. ** On concrete usage, the (private, non disclosed) target data structure is assumed ** to _build a subclass of TreeMutator._ To this end, the TreeMutator is complemented - ** by a builder API. Each call on this builder -- typically providing some closure -- - ** will add yet another decorating layer on top of the basic TreeMutator (recall all + ** by a **builder DSL**. Each call on this builder -- typically providing some closure -- + ** will add yet another _decorating layer_ on top of the basic TreeMutator (recall all ** the "mutation primitives" are implemented NOP within the base class). So the actual ** TreeMutator will be structured like an onion, where each layer cares for the sole ** concrete aspect it was tied for by the supplied closure. For example, there might @@ -58,6 +58,25 @@ ** API is only declared (forward) by default. The TestMutationTarget is a helper class, ** which can be attached through this binding and allows a unit test fixture to record ** and verify all the mutation operations encountered. + ** + ** ## Lifecycle + ** The TreeMutator is conceived as a disposable, one-way-off object. On initialisation, + ** it will _"grab" the contents of its target_ and push them back into place one by one + ** while consuming a mutation diff. For this reason, TreeMutator is made **non-copyable**, + ** just supporting move construction, as will happen when using the DSL functions on + ** the builder. This is also the only supported usage pattern: you create an anonymous + ** TreeMutator sub type by using the Builder functions right within the scope about to + ** consume one strike of diff messages. These messages should cover anything to confirm + ** or reshape _all of the target's contents_. After that, you must not refer to the + ** exhausted TreeMutator anymore, just let it fall out of scope. Incidentally, this + ** also means that _any failure or exception encountered_ while applying a diff will + ** **corrupt the target data structure**. The basic assumption is that + ** - the target data structure will actually be built through diff messages solely + ** - and that all received diff messages are sane, as being drawn from a + ** semantically and structurally equivalent source structure + ** If unable to uphold this consistency assumption, it is the client's responsibility + ** to care for _transactional behaviour,_ i.e. create a clone copy of the data structure + ** beforehand, and "commit" or "roll back" the result atomically. ** ** @note to improve readability, the actual implementation of the "binding layers" ** is defined in separate headers and included towards the bottom of this header. @@ -78,14 +97,9 @@ #include "lib/diff/gen-node.hpp" #include "lib/opaque-holder.hpp" #include "lib/iter-adapter-stl.hpp" -//#include "lib/util.hpp" -//#include "lib/format-string.hpp" -#include -#include ////TODO +#include #include -//#include -//#include namespace lib { @@ -142,9 +156,7 @@ namespace diff{ namespace error = lumiera::error; -//using util::_Fmt; - using lib::Literal; - using std::function; + using lib::Literal; /////TODO placeholder using std::string; @@ -157,7 +169,7 @@ namespace diff{ template struct Builder; - using ID = Literal; + using ID = Literal; /////TODO placeholder using Attribute = DataCap; } @@ -176,6 +188,15 @@ namespace diff{ { public: + /** only allow default and move construction */ + TreeMutator () =default; + TreeMutator (TreeMutator&&) =default; + TreeMutator (TreeMutator const&) =delete; + + TreeMutator& operator= (TreeMutator const&) =delete; + TreeMutator& operator= (TreeMutator&&) =delete; + + /* ==== operation API ==== */ @@ -274,6 +295,8 @@ namespace diff{ using lib::meta::Strip; using lib::meta::Types; + using std::forward; + using std::move; /** * Type rebinding helper to pick up the actual argument type. @@ -328,13 +351,15 @@ namespace diff{ * @remarks all generated follow-up builders are chained and * derive from the implementation of the preceding * "binding layer" and the TreeMutator interface. + * @note on each chained builder call, the compound is + * moved "inside out" into the next builder. */ template struct Builder : PAR { Builder(PAR&& par) - : PAR(std::forward(par)) + : PAR{forward (par)} { } template @@ -352,7 +377,7 @@ namespace diff{ Builder> change (Literal attributeID, CLO closure) { - return Change (attributeID, closure, *this); + return Change (attributeID, closure, move(*this)); } /** set up a binding to a structure of "child objects", @@ -369,7 +394,7 @@ namespace diff{ Builder> attach (BIN&& collectionBindingSetup) { - return Collection (std::forward(collectionBindingSetup), *this); + return Collection {forward(collectionBindingSetup), move(*this)}; } /** set up a diagnostic layer, binding to TestMutationTarget.