From e50e9cb8e781d4c9df7d856fbe8353b2b3d83849 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sat, 23 Nov 2024 17:21:41 +0100 Subject: [PATCH] Library: attempt workaround to problem with references There is an insidious problem when the Transformer takes references to internal state within upstream iterators or state core. This problem only manifests when a invariant based filtering or grouping operation is added after the Transformer, because such an operation (notably Filter) will typically attempt to establish the invariant from the constructor (to avoid dangling state). Unfortunately doing so involves pulling data ''before the overall pipeline is moved into final location'' A workaround is to make the Transformer ''disengage'' on copy, so to provoke a refresh and new pull in the new location after the copy / move / swap. This only works if the transformer function as such is idempotent. --- doc/technical/code/codingGuidelines.txt | 10 +-- doc/technical/howto/crackNuts.txt | 2 +- src/lib/iter-explorer.hpp | 44 ++++++++++-- wiki/thinkPad.ichthyo.mm | 95 ++++++++++++++----------- 4 files changed, 96 insertions(+), 55 deletions(-) diff --git a/doc/technical/code/codingGuidelines.txt b/doc/technical/code/codingGuidelines.txt index a27d8ddca..baf5ae791 100644 --- a/doc/technical/code/codingGuidelines.txt +++ b/doc/technical/code/codingGuidelines.txt @@ -78,6 +78,11 @@ The Lumiera project uses GNU indentation style with slight adaptations. `&`,`|`, `^`, sometimes also for `!` -- using those is taken as an indication of entering the ``danger zone''... + +Spelling +~~~~~~~~ +Lumiera uses _British spelling._ Please set your spell checker accordingly. + Naming conventions ~~~~~~~~~~~~~~~~~~ Naming conventions are used to characterise the kind of element at hand and give a visual @@ -110,11 +115,6 @@ In case a definition actually denotes an object, there should be The object pointer/handle should be passed as 1^st^ argument with the name +self+ -Spelling -~~~~~~~~ -Lumiera uses _British spelling._ Please set your spell checker accordingly. - - General Code Arrangement and Layout ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - Headers and translation units are named `*.hpp` and `*.cpp` rsp. `*.h` and `*.c` + diff --git a/doc/technical/howto/crackNuts.txt b/doc/technical/howto/crackNuts.txt index f444a8d02..875e92075 100644 --- a/doc/technical/howto/crackNuts.txt +++ b/doc/technical/howto/crackNuts.txt @@ -119,7 +119,7 @@ Lumiera iterators:: construct and in while-loops. IterExplorer:: The function `lib::explore(IT)` builds on top of these features and is meant to basically - iterate anything that is iterable -- so use it to abstract away the details. + iterate anything that is iterable -- this can be used to level and abstract away the details. + - can be filtered and transformed - can be reduced or collected into a vector diff --git a/src/lib/iter-explorer.hpp b/src/lib/iter-explorer.hpp index 67052c245..cf3712a16 100644 --- a/src/lib/iter-explorer.hpp +++ b/src/lib/iter-explorer.hpp @@ -79,10 +79,11 @@ ** ** @warning all builder operations work by _moving_ the existing pipeline built thus far into the parent ** of the newly built subclass object. The previously existing pipeline is defunct after that - ** move; if you captured it into a variable, be sure to capture the _result_ of the new - ** builder operation as well and don't use the old variable anymore. Moreover, it should - ** be ensured that any "state core" used within IterExplorer has an efficient move ctor; - ** including RVO, the compiler is typically able to optimise such move calls away altogether. + ** move; if you captured it into a variable, be sure to capture the _result_ of the new builder + ** operation as well and don't use the old variable anymore. An insidious trap can be to store + ** references to source iterator state _from within_ the pipeline. Moreover, it should be ensured + ** that any "state core" used within IterExplorer has an efficient move ctor; including RVO, + ** the compiler is typically able to optimise such move calls away altogether. ** ** @see IterExplorer_test ** @see iter-adapter.hpp @@ -763,6 +764,10 @@ namespace lib { * `operator->` on any iterator downstream. This is also the reason why the * ItemWrapper is necessary, precisely _because we want to support_ functions * producing a value; it provides a safe location for this value to persist. + * @warning handling a transformer function which exposes references can be dangerous. + * For this reason, Transformer attempts to »dis-engage« on each copy / assignment, + * in order to provoke a re-invocation of the transformer function, which hopefully + * picks up references to the copied / moved / swapped location. Be careful though! */ template class Transformer @@ -782,15 +787,38 @@ namespace lib { using pointer = typename meta::ValueTypeBinding::pointer; - Transformer() =default; - // inherited default copy operations - template Transformer (SRC&& dataSrc, FUN&& transformFunctor) : SRC{move (dataSrc)} // NOTE: slicing move to strip IterExplorer (Builder) , trafo_{_FunTraits::adaptFunctor (forward (transformFunctor))} { } + Transformer() =default; + Transformer (Transformer const& o) + : SRC{o} + , trafo_{o.trafo_} + , treated_{/* deliberately empty: force re-engage */} + { } + Transformer (Transformer && o) + : SRC{move (o)} + , trafo_{move (o.trafo_)} + , treated_{/* deliberately empty: force re-engage */} + { } + Transformer& + operator= (Transformer changed) + { + swap (*this,changed); + return *this; + } + friend void + swap (Transformer& t1, Transformer& t2) + { + using std::swap; + t1.treated_.reset(); + t2.treated_.reset(); + swap (t1.trafo_, t2.trafo_); + } + /** refresh state when other layers manipulate the source sequence * @remark expansion replaces the current element by a sequence of @@ -981,6 +1009,8 @@ namespace lib { * This limitation was deemed acceptable (adapting a function with several arguments would * require quite some nasty technicalities). The first argument of this `aggFun` refers * to the accumulator by value, and thereby also implicitly defines the aggregate result type. + * @warning the Aggregator \a AGG *must not capture references* to upstream internal state, because + * the overall pipeline will be moved into final location _after the initial ctor call._ */ template class GroupAggregator diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index ecd08c3d2..680b285af 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -19134,9 +19134,7 @@ - - - +

...unter der Annahme, daß das Kürzen ggfs.auch verlängern kann, und damit schon relativ nahe am verfügbaren Platz ist. Dann verhindert die Hysterese, daß nochmal geprüft wird @@ -19590,9 +19588,7 @@ - - - +

es gibt anderswo einen dedizierten "Content Controller" @@ -20045,9 +20041,7 @@ - - - +

liegt an der inkrementellen Natur der Layout-Zuteilung @@ -20593,9 +20587,7 @@ - - - +

braucht feste Speicher-Addresse @@ -21256,9 +21248,7 @@ - - - +

weil nach hier etablierter Policy diesen erlaubt wäre, "von oben" in den trackPresenter.displayFrame.trackBody reinzugreifen für die startLine_ @@ -23206,9 +23196,7 @@ - - - +

...hab ich mich je anders entschieden? @@ -26893,9 +26881,7 @@ - - - +

aber verschachtelte sub-Tracks werden in dieser gehandhabt @@ -37050,9 +37036,7 @@ - - - +

nur bei laufender Event-Loop @@ -40250,9 +40234,7 @@ - - - +

Menu @@ -41827,9 +41809,7 @@ - - - +

der Nenner ist limitiert (kleiner Time::SCALE) und nicht toxisch @@ -42665,9 +42645,7 @@ - - - +

und die Testsuite ist auf Anhieb GRÜN @@ -43340,9 +43318,7 @@ - - - +

...und der ganze Ansatz mit Newton-Näherung stellte sich als unsinnig heraus; jetzt rechnen wir die Optimierung mit einem Schuß in float, und reizen dann sogar noch den Headroom aus, wodurch die Werte viel genauer werden @@ -43693,9 +43669,7 @@ - - - +

da reichen meine Algebra-Kenntnisse nicht aus.... @@ -43933,9 +43907,7 @@ - - - +

Und zwar, wenn der Nenner viel kleiner ist als der Zähler, und der Zähler extrem groß. Dann würde nämlich die Ganzzahl-Division keine signifikante Verringerung der Dimension bewirken, und die anschließende re-Quantisierung das Ergebnis (bedingt durch die Normierung auf einen gemeinsamen Nenner) sogar noch vergrößern @@ -53155,6 +53127,45 @@ + + + + + + + +

+ ich brauche einen engaged-State +

+ + + + + + + + + + + + + + + + + +

+ ...er läßt nämlich den ItemWrapper zunächst leer, und verwendet das für eine Lazy-invocation der Transformation +

+ + + + + + + + +