after sleeping one night over the problem, this seems to be
the most natural solution, since the possibility of assignment
naturally arises from the fact that, for tree diff, we have
to distinguish between the *identity* of an element node and
its payload (which could be recursive). Thus, IFF the payoad
is an assignable value, why not allow to assign it. Doing so
elegnatly solves the problem with assignment of attributes
Signed-off-by: Ichthyostega <prg@ichthyostega.de>
I assumed that, since GenNode is composed of copyable and
assignable types, the standard implementation will do.
But I overlooked the run time type check on the opaque
payload type within lib::Variant. When a type mismatch
is detected, the default implementation has already
assigned and thus altered the IDs.
So we need to roll our own implementation, and to add
insult to injury, we can't use the copy-and-swap idiom either.
This is actually a STL library feature, and was added precisely
for the reason encountered here: if we want logarithmic search,
we'll have to construct a new GenNode object, just to have something
for the set to invoke the comparison operator.
C++14 introduced the convention that the Comparator of the set
may define a marker type `is_transparent` alongside with a generic
comparison operator. But, as is obvious from the source code of
our GNU Standard library implementation, our std::set has no such
overload to make use of that feature
http://en.cppreference.com/w/cpp/container/set/findhttp://stackoverflow.com/questions/20317413/what-are-transparent-comparators
The only good thing is that, just 10 minutes ago, I felt like
a complete moron because I'm writing a unit test for such a simple
storage class. ;-)
...when the Test-Nexus processes a command binding message.
In the real system of course we do not want to log every bind message.
The challenge here is the fact that command binding as such
is opaque, and the types of the data within the bind message
are opaque as well. Finally I settled on the compromise
to log them as strings, but only the DataCap part;
most value types applicable within GenNode
have a string representation to match.
the rationale is that I deliberately do not want to provide
a mechanism to iterate "over all contents in stringified form".
Because this could be seen as an invitation to process GenNode-
datastructures in an imperative way. Please recall we do not
want that. Users shall either *match* contents (using a visitor),
or they are required to know the type of the contents beforehand.
Both cases favour structural and type based programming over
dynamic run-time based inspection of contents
The actual task prompting me to add this iteration mechanism
is that I want to build a diagnostic, which allows to verify
that a binding message was sent over the bus with some
specific parameter values.
...no need to enclose empty sections when there are no
attributes or no children. Makes test code way more readable.
TestEventLog_test PASS as far as implemented
some tests rely on additional diagnostics code being linked in,
which happens, when lib/format-util.hpp is included prior to
the instantiation of lib::diff::Record rsp. lib::Variant.
The reason why i opended this can of worms was to avoid includion
of this formatting and diagnostics code into such basic headers
as lib/variant.hpp or lib/diff/gen-node.hpp
Now it turns out, that on some platforms the linker will use
a later instantiation of lib::Variant::Buff<GenNode>::operator string
in spite of a complete instantiation of this virtual function
being available already in liblumierasupport.so
But the real reason is that -- with this trickery -- we're violating
the single definition rule, so we get what we deserved.
TODO (Ticket #973): at a later point in development we have to re-assess,
the precise impact of including lib/format-util.hpp into
lib/diff/gen-node.hpp
Right now I expect GenNode to be used pervasively, so I am
reluctant to make that header too heavyweight.
because otherwise we'd need to send a whole subtree
over the wire and then descend into it just to find an element.
This too is a ripple effect of making '==' deep
well... this was quite a piece of work
Added some documentation, but a complete documentation,
preferably to the website, would be desirable, as would
be a more complete test covering the negative corner cases
yeah, working with open fire is dangerous...
For performace reasons I've undercut the premise
to make GenNode / Record immutable. Now I'm dealing with
raw storage layout together with this quite hairy distinction
between "attribute scope" and "child scope"
In hindsight, it might have been better to implement Record
as a single list, and to maintain a shortcut pointer to jump
to the start of the attributes.
while implementing this, I've discovered a conceptual error:
we allow to accept attributes, even when we've already entered
the child scope. This means that we can not predictable get back
at the "last" (i.e. the currently touched) element, because this
might be such an attribute. So a really correct implementation
would have to memorise the "current" element, which is really
tricky, given the various ways of touching elements in our
diff language.
In the end I've decided to ignore this problem (maybe a better
solution would have been to disallow those "late" attributes?)
My reasoning is that attributes are unlikely to be full records,
rather just values, and values are never mutated. (but note
that it is definitively possible to have an record as attribute!)
...while I must admit that I'm a bit doubtful about that
language feature, but it does come in handy when manually
writing diff messages. The reason is the automatic naming
of child objects, which makes it often hard to refer to
a child after the fact, since the name can not be
reconstructed systematically.
Obviously the downside of this "anonymous pick / delete"
is that we allow to pick (accept) or even delete just
any child, which happens to sit there, without being
able to detect a synchronisation mismatch between
sender and receiver.
i.e. flat match, not deep equality.
This allows to send just an Ref (with the ID) over the
wire to refer to an complete object to be picked, moved
or deleted on the receiver side.
in the first version, I defined equality to just compare the IDs
But that didn't seem right, or what one would expect by the concept
of equality (this is a long standing discussion with persistent
object-relationally mapped data).
So I changed the semantics of equaility to be "deep".
As this means possiblty to visit a whole tree depth-first,
it seems reasonable to provide the shallow "identity-comparison" likewise.
And the most reaonable choice is to use the "matches(object)" API
for that, since, in case of objects, the matches was defined
as full equality, which now seems redundant.
Thus: from now on: obj.matches(otherObj)
means they share the same IDs
The Ref-GenNode is just a specifically constructed GenNode,
and intended to be sliced down to an ordinary GenNode
immediately after construction. It seems, GCC didn't "get that"
and instead emitted an recursive invocation of the same ctor,
which obviously leads to stack overflow.
Problem solved by explicitly coding the copy initialisation,
after the full definition of Ref is available.
the type is the only meta attribute supported by now,
thus the decision was to handle this manually, instead of
introducing a full scope for meta attributes. Unfortunately
this leads to an assymetry: while it is possible to send an
attribute named "type", which will be intercepted and used
as a new type ID, the type will not show up when iterating
or searching through attributes.
When applying a diff, the only possibility is to *insert*
a new type attribute, and we need to check and handle this
likewise manually.
It is difficult to reconcile our general architecture for the
linearised diff representation with the processing of recursive,
tree-like data structures. The natural and most clean way to
deal with trees is to use recursion, i.e. the processor stack.
But in our case, this means we'd have to peek into the next
token of the language and then forward the diff iterator
into a recursive call on the nested scope. Essentially, this
breaks the separation between receiving a token sequence and
interpretation for a concrete target data structure.
For this reason, it is preferrable to make the stack an
internal state of the concrete interpreter. The downside of
this approach is the quite confusing data storage management;
we try to make the role of the storage elements a bit more
clear through descriptive accessor functions.
implement the list handling primitives analogous to the
implementation of list-diff-applicator -- just again with
the additional twist to keep the attribute and child scopes
separated.
...so now the stage is set. We can reimplement
the handling of the list diff cases here in the context
of tree diff application. The additional twist of course
being the distinction between attribute and child scope
each language token of our "linearised diff representation"
carries a payload data element, which typically is the piece
of data to be altered (added, mutated, etc).
Basically, these elements have value semantics and are
"sent over wire", and thus it seems natural when the
language interpreter functions accept that piece of payload
by-value. But since we're now sending GenNode elements as
parameter data in our diff, which typically are of the
size of 10 data elements (640 bit on a 64bit machine),
it seems more resonable to pass these argument elements
by const& through the interpreter function. This still
means we can (and will indeed) copy the mutated data
values when applying the diff, but we're able to
relay the data more efficiently to the point where
it's consumed.
this boils down to the two alternatives
- manipulate the target data structure
- build an altered copy
since our goal is to handle large tree structures efficiently,
the decision was cast in favour of data manipulation
so basically it's time to explicate the way
our diff language will actually be written.
Similar to the list diff case, it's a linear sequence
of verb tokens, but in this case, the payload value
in each token is a GenNode. This is the very reason
why GenNode was conceived as value object with an
opaque DataCap payload
