diff --git a/src/lib/diff/test-mutation-target.hpp b/src/lib/diff/test-mutation-target.hpp index 808f06a8b..909a4b80b 100644 --- a/src/lib/diff/test-mutation-target.hpp +++ b/src/lib/diff/test-mutation-target.hpp @@ -376,26 +376,12 @@ namespace diff{ /* ==== re-Implementation of the operation API ==== */ - /** skip next recorded src element - * @remarks TestWireTap adapter together with TestMutationTarget - * maintain a "shadow copy" of the data and apply the detected diff - * against this internal copy. This allows to verify what's going on - */ - virtual void - skipSrc (GenNode const& n) override - { - if (pos_) - { - GenNode const& skippedElm = *pos_; - ++pos_; - target_.logSkip (skippedElm); - } - PAR::skipSrc(n); - } - /** record in the test target * that a new child element is * being inserted at current position + * @remarks TestWireTap adapter together with TestMutationTarget + * maintain a "shadow copy" of the data and apply the detected diff + * against this internal copy. This allows to verify what's going on */ virtual bool injectNew (GenNode const& n) override @@ -417,8 +403,20 @@ namespace diff{ matchSrc (GenNode const& n) override { return PAR::matchSrc(n) - or pos_? n.matches(*pos_) - : false; + or pos_ and n.matches(*pos_); + } + + /** skip next recorded src element without touching it */ + virtual void + skipSrc (GenNode const& n) override + { + if (pos_) + { + GenNode const& skippedElm = *pos_; + ++pos_; + target_.logSkip (skippedElm); + } + PAR::skipSrc(n); } /** accept existing element, when matching the given spec */ @@ -426,7 +424,7 @@ namespace diff{ acceptSrc (GenNode const& n) override { bool isSrcMatch = pos_ and n.matches(*pos_); - if (isSrcMatch) // NOTE: important to invoke our own match here + if (isSrcMatch) //NOTE: important to invoke our own match here { target_.inject (move(*pos_), "acceptSrc"); ++pos_; diff --git a/src/lib/diff/tree-diff-mutator-binding.cpp b/src/lib/diff/tree-diff-mutator-binding.cpp index 283d3ea22..c4869a992 100644 --- a/src/lib/diff/tree-diff-mutator-binding.cpp +++ b/src/lib/diff/tree-diff-mutator-binding.cpp @@ -265,12 +265,6 @@ namespace diff{ UNIMPLEMENTED("skip next src element and advance abstract source position"); } - bool - TreeDiffMutatorBinding::injectNew (GenNode const& n) - { - return treeMutator_->injectNew(n); - } - bool TreeDiffMutatorBinding::matchSrc (GenNode const& n) { @@ -320,7 +314,7 @@ namespace diff{ void TreeDiffMutatorBinding::ins (GenNode const& n) { - bool success = injectNew (n); + bool success = treeMutator_->injectNew(n); if (not success) __failMismatch (n, "insert"); } diff --git a/src/lib/diff/tree-diff-mutator-binding.hpp b/src/lib/diff/tree-diff-mutator-binding.hpp index b50cad24b..fd567149c 100644 --- a/src/lib/diff/tree-diff-mutator-binding.hpp +++ b/src/lib/diff/tree-diff-mutator-binding.hpp @@ -279,7 +279,6 @@ namespace diff{ /* == Forwarding: mutation primitives == */ void skipSrc(); - bool injectNew (GenNode const& n); bool matchSrc (GenNode const& n); bool acceptSrc (GenNode const& n); bool findSrc (GenNode const& n); diff --git a/src/lib/diff/tree-mutator-attribute-binding.hpp b/src/lib/diff/tree-mutator-attribute-binding.hpp index e40098511..6a9a226c7 100644 --- a/src/lib/diff/tree-mutator-attribute-binding.hpp +++ b/src/lib/diff/tree-mutator-attribute-binding.hpp @@ -174,6 +174,18 @@ * in a way not to ask this binding to "reorder" a field from an * existing class definition. */ + virtual void + skipSrc (GenNode const& refSpec) override + { + if (isApplicable(refSpec)) + throw error::Logic (_Fmt{"attempt to skip or drop attribute '%s', " + "which is bound to an object field and " + "thus can not cease to exist."} + % refSpec.idi.getSym()); + else + PAR::skipSrc (refSpec); + } + virtual bool findSrc (GenNode const& refSpec) override { diff --git a/src/lib/diff/tree-mutator-collection-binding.hpp b/src/lib/diff/tree-mutator-collection-binding.hpp index 60870a18b..c969f3a6a 100644 --- a/src/lib/diff/tree-mutator-collection-binding.hpp +++ b/src/lib/diff/tree-mutator-collection-binding.hpp @@ -200,21 +200,6 @@ /* ==== re-Implementation of the operation API ==== */ - /** skip next pending src element, - * causing this element to be discarded - */ - virtual void - skipSrc (GenNode const& n) override - { - if (binding_.isApplicable(n)) - { - if (pos_) - ++pos_; - } - else - PAR::skipSrc (n); - } - /** fabricate a new element, based on * the given specification (GenNode), * and insert it at current position @@ -244,12 +229,27 @@ matchSrc (GenNode const& spec) override { if (binding_.isApplicable(spec)) - return pos_? binding_.matches (spec, *pos_) - : false; + return pos_ and binding_.matches (spec, *pos_); else return PAR::matchSrc (spec); } + /** skip next pending src element, + * causing this element to be discarded + * @note can not perform a match on garbage data + */ + virtual void + skipSrc (GenNode const& n) override + { + if (binding_.isApplicable(n)) + { + if (pos_) + ++pos_; + } + else + PAR::skipSrc (n); + } + /** accept existing element, when matching the given spec */ virtual bool acceptSrc (GenNode const& n) override @@ -257,7 +257,7 @@ if (binding_.isApplicable(n)) { bool isSrcMatch = pos_ and binding_.matches (n, *pos_); - if (isSrcMatch) // NOTE: crucial to perform only our own match check here + if (isSrcMatch) //NOTE: crucial to perform only our own match check here { binding_.inject (move(*pos_)); ++pos_; diff --git a/src/lib/diff/tree-mutator.hpp b/src/lib/diff/tree-mutator.hpp index 54858ea85..2803c2417 100644 --- a/src/lib/diff/tree-mutator.hpp +++ b/src/lib/diff/tree-mutator.hpp @@ -210,14 +210,6 @@ namespace diff{ // do nothing by default } - /** skip next src element and advance abstract source position. - * The argument shall be used to determine applicability */ - virtual void - skipSrc (GenNode const&) - { - // do nothing by default - } - /** establish new element at current position * @return `true` when successfully inserted something */ virtual bool @@ -235,6 +227,22 @@ namespace diff{ return false; } + /** skip next src element and advance abstract source position. + * The argument shall be used to determine applicability + * @remarks this operation is used both to implement the `del` verb + * and the `skip` verb. Since the latter discards garbage + * left back by `find` we must not touch the contents, + * to prevent a SEGFAULT. Thus `skipSrc` can not match + * and thus can not return anything. Consequently the + * `del` implementation has to use `matchSrc` explicitly, + * and the latter must invoke the selector prior to + * performing the local match. */ + virtual void + skipSrc (GenNode const&) + { + // do nothing by default + } + /** accept existing element, when matching the given spec */ virtual bool acceptSrc (GenNode const&) diff --git a/tests/library/diff/tree-mutator-binding-test.cpp b/tests/library/diff/tree-mutator-binding-test.cpp index 4f54a6a36..dfdbc7b9e 100644 --- a/tests/library/diff/tree-mutator-binding-test.cpp +++ b/tests/library/diff/tree-mutator-binding-test.cpp @@ -766,7 +766,7 @@ namespace test{ gamma = val; }); - CHECK (sizeof(mutator1) <= sizeof(void*) // the TreeMutator VTable + CHECK (sizeof(mutator2) <= sizeof(void*) // the TreeMutator VTable + 3 * sizeof(void*) // one closure reference for each lambda + 3 * sizeof(GenNode::ID)); // one attribute-key for each binding @@ -789,7 +789,8 @@ namespace test{ CHECK (mutator2.matchSrc (ATTRIB1)); // behaviour of the binding remains unaffected CHECK (mutator2.acceptSrc (ATTRIB1)); // now pick and "accept" this src element (also a NOP) // acceptSrc - mutator2.skipSrc (ATTRIB1); // and 'skip' likewise is just not implemented for attributes // skipSrc + VERIFY_ERROR (LOGIC, mutator2.skipSrc (ATTRIB3)); + // and 'skip' likewise is just not implemented for attributes // skipSrc CHECK ( 1 == alpha); CHECK (-1 == beta); CHECK (3.45 == gamma); // all these non-operations actually didn't change anything... @@ -807,7 +808,8 @@ namespace test{ // but since all those operations are not relevant for our attribute binding, they will be passed on // to lower binding layers. And since, moreover, there /are no lower binding layers/ in our setup, // they will just do nothing and return false - mutator2.skipSrc (ATTRIB3); // skipSrc + CHECK (not mutator2.matchSrc (CHILD_B)); + mutator2.skipSrc (CHILD_B); // ...no setter binding, thus no effect // skipSrc CHECK (not mutator2.injectNew (SUB_NODE));// ...no setter binding, thus no effect // injectNew CHECK (not mutator2.matchSrc (CHILD_B)); CHECK (not mutator2.acceptSrc (CHILD_B)); // acceptSrc diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 3e11ada62..dbb99426e 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -973,6 +973,11 @@ + + + + + @@ -994,38 +999,85 @@ - - - - - - - + + + + + + +

+ since skipSrc performs both the `del` and the `skip` verb, it can not perform the match itself... +

+ + + - + + - - + + + + + + + + + +

+ ...because it is also used to discard garbage after a findSrc operation. +

+

+ Thus we need to avoid touching the actual data in the src sequence, because this might lead to SEGFAULT. +

+

+ +

+

+ For this reason, the implementation of the `del` verb has to invoke matchSrc explicitly beforehand, +

+

+ and this is the very reason `matchSrc` exists. Moreover, `matchSrc` must be written such +

+

+ as to ensure to invoke the Selector before performing a local match. And skipSrc has to +

+

+ proceed in precisely the same way. Thus, if the selector denies responsibility, we'll delegate +

+

+ to the next lower layer in both cases, and the result and behaviour depends on this next lower layer solely +

+ + + + + - + + + + +

- move into target + then move into target

- + +