ichthyo/LUMIERA.clone
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

investigate and confirm the logic underlying the matchSrc, skipSrc and acceptSrc primitives

Browse source 
In Theory, acceptSrc and skipSrc are to operate symmetrically,
with the sole difference that skipSrc does not move anything
into the new content.

BUT, since skipSrc is also used to implement the `skip` verb,
which serves to discard garbage left back by a preceeding `find`,
we cannot touch the data found in the src position without risk
of SEGFAULT. For this reason, there is a dedicated matchSrc operation,
which shall be used to generate the verification step to properly
implement the `del` verb.

I've spent quite some time to verify the logic of predicate evaluation.
It seems to be OK: whenever the SELECTOR applies, then we'll perform
the local match, and then also we'll perform the skipSrc. Otherwise,
we'll delegate both operations likewise to the next lower layer,
without touching anything here.
This commit is contained in:
Fischlurch 2016-08-09 23:42:42 +02:00
parent 43f3560b15
commit 0782dd4922
8 changed files with 135 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
src/lib/diff/test-mutation-target.hpp
View file

@ -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_;

8
src/lib/diff/tree-diff-mutator-binding.cpp
View file

@ -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");
}

1
src/lib/diff/tree-diff-mutator-binding.hpp
View file

@ -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);

12
src/lib/diff/tree-mutator-attribute-binding.hpp
View file

@ -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
{

36
src/lib/diff/tree-mutator-collection-binding.hpp
View file

@ -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_;

24
src/lib/diff/tree-mutator.hpp
View file

@ -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&)

8
tests/library/diff/tree-mutator-binding-test.cpp
View file

@ -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

78
wiki/thinkPad.ichthyo.mm
View file

@ -973,6 +973,11 @@
</html></richcontent>
<cloud COLOR="#fce9c0"/>
<font NAME="SansSerif" SIZE="16"/>
<node CREATED="1455982947867" ID="ID_1339677569" MODIFIED="1470772472993" TEXT="injectNew">
<font BOLD="true" NAME="SansSerif" SIZE="12"/>
<node CREATED="1455982969073" ID="ID_1885889112" MODIFIED="1455982974332" TEXT="inject new content"/>
<node CREATED="1457047512175" ID="ID_1126383522" MODIFIED="1457047519426" TEXT="at implicit &quot;current&quot; position"/>
</node>
<node CREATED="1455927425726" ID="ID_1776437339" MODIFIED="1464117059267" TEXT="hasSrc">
<font BOLD="true" NAME="SansSerif" SIZE="12"/>
<node CREATED="1455928216420" ID="ID_662720483" MODIFIED="1464117064570" TEXT="further src elements available"/>
@ -994,38 +999,85 @@
<icon BUILTIN="help"/>
</node>
</node>
<node CREATED="1455927425726" ID="ID_1759686725" MODIFIED="1457120215833" TEXT="skipSrc">
<font BOLD="true" NAME="SansSerif" SIZE="12"/>
<node CREATED="1455928216420" ID="ID_1581600385" MODIFIED="1455928325793" TEXT="advance source position"/>
<node CREATED="1470527086660" ID="ID_13765501" MODIFIED="1470527097135" TEXT="guarded by selector">
<icon BUILTIN="messagebox_warning"/>
</node>
</node>
<node CREATED="1455927396505" ID="ID_392033275" MODIFIED="1457120240382" TEXT="matchSrc">
<font BOLD="true" NAME="SansSerif" SIZE="12"/>
<node CREATED="1455928268589" ID="ID_97473072" MODIFIED="1455928318898" TEXT="ID comparison">
<node CREATED="1455928524530" ID="ID_545057240" MODIFIED="1457120284424" TEXT="implicit next pos"/>
<node CREATED="1455928530738" ID="ID_1035043901" MODIFIED="1455928533677" TEXT="ID"/>
</node>
<node CREATED="1470778603801" ID="ID_1808045935" MODIFIED="1470778650743" TEXT="needed to implement the `del` verb">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
since skipSrc performs both the `del` and the `skip` verb, it can not perform the match itself...
</p>
</body>
</html>
</richcontent>
</node>
<node CREATED="1455982947867" ID="ID_1339677569" MODIFIED="1457047494973" TEXT="injectNew">
</node>
<node CREATED="1455927425726" ID="ID_1759686725" MODIFIED="1470772470034" TEXT="skipSrc">
<font BOLD="true" NAME="SansSerif" SIZE="12"/>
<node CREATED="1455982969073" ID="ID_1885889112" MODIFIED="1455982974332" TEXT="inject new content"/>
<node CREATED="1457047512175" ID="ID_1126383522" MODIFIED="1457047519426" TEXT="at implicit &quot;current&quot; position"/>
<node CREATED="1470527086660" ID="ID_13765501" MODIFIED="1470527097135" TEXT="guarded by selector">
<icon BUILTIN="messagebox_warning"/>
</node>
<node CREATED="1470772440180" ID="ID_1241607377" MODIFIED="1470778593859" TEXT="can not match by itself">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
...because it is also used to discard garbage after a findSrc operation.
</p>
<p>
Thus we need to avoid touching the actual data in the src sequence, because this might lead to SEGFAULT.
</p>
<p>
</p>
<p>
For this reason, the implementation of the `del` verb has to invoke matchSrc explicitly beforehand,
</p>
<p>
and this is the very reason `matchSrc` exists. Moreover, `matchSrc` must be written such
</p>
<p>
as to ensure to invoke the Selector before performing a local match. And skipSrc has to
</p>
<p>
proceed in precisely the same way. Thus, if the selector denies responsibility, we'll delegate
</p>
<p>
to the next lower layer in both cases, and the result and behaviour depends on this next lower layer solely
</p>
</body>
</html>
</richcontent>
</node>
<node CREATED="1455928216420" ID="ID_1581600385" MODIFIED="1470778395226" TEXT="thus just advance source position"/>
</node>
<node CREATED="1455927413191" ID="ID_1624797970" MODIFIED="1457120269834" TEXT="acceptSrc">
<font BOLD="true" NAME="SansSerif" SIZE="12"/>
<node CREATED="1455928275316" ID="ID_702364156" MODIFIED="1455928304302">
<node CREATED="1470527086660" ID="ID_410793564" MODIFIED="1470772489007" TEXT="guarded by selector">
<icon BUILTIN="messagebox_warning"/>
</node>
<node CREATED="1470772440180" ID="ID_836603881" MODIFIED="1470772503839" TEXT="verify match with next src position"/>
<node CREATED="1455928275316" ID="ID_702364156" MODIFIED="1470772519749">
<richcontent TYPE="NODE"><html>
<head>
</head>
<body>
<p>
<i>move</i>&#160;into target
then <i>move</i>&#160;into target
</p>
</body>
</html></richcontent>
</html>
</richcontent>
<node CREATED="1455928537273" ID="ID_1036724915" MODIFIED="1457120296935" TEXT="implicit next pos"/>
</node>
</node>