From bb7ac1e8e66b4f0d0960f2775f3aa8192c5b2032 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 14 Jul 2020 14:49:11 -0700 Subject: [PATCH] rework iteration, extend to Union --- documentation/value.rst | 32 ++++++++- src/data.cpp | 146 +++++++++++++++++++++++++++++++--------- src/dataimpl.h | 15 ++--- src/pvxs/data.h | 9 +-- test/testdata.cpp | 59 ++++++++++++++-- 5 files changed, 206 insertions(+), 55 deletions(-) diff --git a/documentation/value.rst b/documentation/value.rst index b8a39b5..f9d528f 100644 --- a/documentation/value.rst +++ b/documentation/value.rst @@ -13,12 +13,14 @@ Value Container #include namespace pvxs { ... } +`pvxs::Value` is the primary data container type used with PVXS. A `pvxs::Value` may be obtained via the remote peer (client or server), or created locally. See `ntapi` or `typedefapi`. -`pvxs::Value` is a pointer-like object which, maybe, references +`pvxs::Value` is a safe pointer-like object which, maybe, references a node in a tree of sub-structures and leaf fields. -This tree is called a Sturture as it behaves in many ways like a C 'struct'. +This tree will be referred to as a Structure as it behaves +in many ways like a C 'struct'. For example, the following code: @@ -51,7 +53,7 @@ Is analogous to the following pseudo code. With the chief functional difference being that the analogs of the casts are made safe. Also, the storage of the underlying Structure will be free'd when no more Values reference it. -A Value which does not reference any underlying Structure is not valid. +A Value which does not reference any underlying Structure is not valid, or "empty". .. code-block:: c++ @@ -70,6 +72,30 @@ All operations on an invalid Value should be safe and well defined. In this example, the operator[] lookup of a non-existant field returns an invalid Value. Attempting to extract an integer from this will then throw a `pvxs::NoField` exception. +Iteration +^^^^^^^^^ + +`pvxs::Value` instances pointing to a non-array structured data field (Struct or Union) +may be iterated. Iteration comes in three variations: `pvxs::Value::iall`, `pvxs::Value::ichildren`, +and `pvxs::Value::imarked`. + +For a Struct, iall() is a depth first traversal of all fields. +ichildren() traverses all child fields (excluding eg. grandchildren +and further). imarked() considers all fields, but only visits +those which have beem marked (`pvxs::Value::isMarked`). + +For a Union. iall() and ichildren() are identical, and will +visit all possible Union members, excluding the implicit NULL member. +Traversal does not effect member selection. +imarked() for a Union will visit at most one member (if one is selected)> + +Iteration of Union may return Value instances +allocated with temporary storage. Changes to these instances +will not effect the underlying structure. + +Iteration of other field types, including StructA and UnionA is not implemented at this time, +and will always appear as empty. + .. doxygenclass:: pvxs::Value :members: diff --git a/src/data.cpp b/src/data.cpp index 8790d58..e1212da 100644 --- a/src/data.cpp +++ b/src/data.cpp @@ -335,14 +335,23 @@ const std::string &Value::nameOf(const Value& descendant) const { if(!store || !descendant.store) throw NoField(); - auto pidx = store->index(); - auto didx = descendant.store->index(); - if(pidx >= didx || didx >= store->top->members.size()) - throw std::logic_error("not a descendant"); + + size_t doffset; + if(desc->code==TypeCode::Struct) { + doffset = descendant.desc - desc; + if(doffset==0 || doffset > desc->mlookup.size()) + throw std::logic_error("not a descendant"); + + } else if(desc->code==TypeCode::Union) { + doffset = descendant.desc - desc->members.data(); + + } else { + throw std::logic_error("nameOf() only implemented for Struct and Union"); + } // inefficient, but we don't keep a reverse mapping for(auto& it : desc->mlookup) { - if(it.second == didx-pidx) + if(it.second == doffset) return it.first; } @@ -802,52 +811,123 @@ size_t Value::nmembers() const void Value::_iter_fl(Value::IterInfo &info, bool first) const { - if(!store) - throw NoField(); + if(!desc || (desc->code!=TypeCode::Struct && desc->code!=TypeCode::Union)) { + // not iterable + info.pos = info.nextcheck = 0u; + return; + } - if(desc->code!=TypeCode::Struct) { - // TODO implement iteration of Union, or Struct[]/Union[] - info.pos = info.nextcheck = 0; + // Union iteration has no depth + if(desc->code.scalarOf()!=TypeCode::Struct) + info.depth = false; - } else if(info.depth) { - info.pos = info.nextcheck = first ? 1u : desc->size(); + // array instances have no marking + if(desc->code.isarray()) + info.marked = false; - if(info.marked) - _iter_advance(info); + size_t cnt; + if(info.depth) + cnt = desc->mlookup.size(); + else + cnt = desc->miter.size(); + + info.pos = first ? 0 : cnt; + + if(first && info.marked) { + info.nextcheck = info.pos; + _iter_advance(info); } else { - info.pos = info.nextcheck = first ? 0u : desc->miter.size(); + info.nextcheck = cnt+1; // for !marked, never need to check } } void Value::_iter_advance(IterInfo& info) const { - assert(info.depth); + assert(desc); + assert(info.marked); // should not be reached for simple iteration - // scan forward to find next non-marked - for(auto idx : range(info.pos, desc->size())) { - auto S = store.get() + idx; - if(S->valid) { - auto D = desc + idx; - info.pos = idx; - info.nextcheck = idx + D->size(); - return; + // for Struct, scan to next marked field + if(desc->code==TypeCode::Struct) { + assert(info.depth); // the following assume + + // scan forward to find next non-marked + for(auto idx : range(info.pos, desc->mlookup.size())) { + auto S = store.get() + idx + 1u; + if(S->valid) { + auto D = desc + idx + 1u; + info.pos = idx; + info.nextcheck = idx + D->size(); + return; + } + } + + // end of iteration + info.pos = desc->mlookup.size(); + info.nextcheck = info.pos+1; + + } else if(desc->code==TypeCode::Union) { + assert(!info.depth); // the following assume + + if(info.pos >= desc->miter.size()) + return; // at end of iteration + + auto& val = store->as(); + auto pos_desc = &desc->members[desc->miter[info.pos].second]; + + if(!val || pos_desc > val.desc) { + // no field selected, or pos is after selection + // end of iteration + info.pos = desc->miter.size(); + info.nextcheck = info.pos+1; + + } else if(pos_desc < val.desc) { + // jump forward to selection + for(auto i : range(info.pos, desc->miter.size())) { + if(val.desc==&desc->members[desc->miter[i].second]) { + info.pos = i; + info.nextcheck = i+1; + return; + } + } + assert(false); // corrupt iterator? } } - - info.pos = info.nextcheck = desc->size(); } Value Value::_iter_deref(const IterInfo& info) const { - auto idx = info.pos; - if(!info.depth) - idx = desc->miter[idx].second; - - decltype (store) store2(store, store.get()+idx); Value ret; - ret.store = std::move(store2); - ret.desc = desc + idx; + + if(desc->code==TypeCode::Struct) { + auto idx = info.pos; + if(info.depth) + idx++; // indexing starts with FieldDesc after top + else + idx = desc->miter[idx].second; + + assert(idx>0u); + assert(idxsize()); + decltype (store) store2(store, store.get()+idx); + ret.store = std::move(store2); + ret.desc = desc + idx; + + } else if(desc->code==TypeCode::Union) { + auto pos_desc = &desc->members[desc->miter[info.pos].second]; + + if(desc->code==TypeCode::Union && store->as().desc==pos_desc) { + // pointing to selected Union field + ret = store->as(); + + } else { + // array, or not selected union field. + // allocate temporary + + std::shared_ptr base(store, pos_desc); + ret = Value(base); + } + } + return ret; } diff --git a/src/dataimpl.h b/src/dataimpl.h index 6c21bf1..36dd219 100644 --- a/src/dataimpl.h +++ b/src/dataimpl.h @@ -48,11 +48,8 @@ struct Buffer; * with offset to descendant fields given as positive integers relative * to the current field. (not possible to jump _back_) * - * We deal with two different numeric values: - * 1. indicies in this FieldDesc array. found in FieldDesc::mlookup and FieldDesc::miter - * Relative to current position in FieldDesc array. (aka this+n) - * 2. offsets in associated FieldStorage array. found in FieldDesc::index - * Relative to current FieldDesc*. + * We deal with indicies in this FieldDesc array. found in FieldDesc::mlookup + * and FieldDesc::miter Relative to current position in FieldDesc array. (aka this+n) */ struct FieldDesc { // type ID string (Struct/Union) @@ -60,8 +57,8 @@ struct FieldDesc { // Lookup of all descendant fields of this Structure or Union. // "fld.sub.leaf" -> rel index - // For Struct, relative to this - // For Union, offset in members array + // For Struct, relative to this (always >=1) + // For Union, offset in members array (one entry will always be zero) std::map mlookup; // child iteration. child# -> ("sub", rel index in enclosing vector) @@ -73,8 +70,8 @@ struct FieldDesc { size_t parent_index=0; // For Union, UnionA, StructA - // For Union, the choices - // For UnionA/StructA, size()==1 containing a Union/Struct + // For Union, the choices concatenated together (members.size() !+ #choices) + // For UnionA/StructA containing a single Union/Struct std::vector members; TypeCode code{TypeCode::Null}; diff --git a/src/pvxs/data.h b/src/pvxs/data.h index cba5a38..4b63034 100644 --- a/src/pvxs/data.h +++ b/src/pvxs/data.h @@ -679,7 +679,10 @@ private: // all [pos, nextcheck) are marked size_t pos; size_t nextcheck; + // true only iterates marked fields (Struct), + // or only the selected field (Union) bool marked; + // false only iterates children (Struct only) bool depth; constexpr IterInfo() :pos(0u), nextcheck(0u), marked(false), depth(false) {} constexpr IterInfo(size_t pos, bool marked, bool depth) @@ -757,15 +760,13 @@ public: V operator*() const { return ref._iter_deref(*this); } Iter& operator++() { pos++; - if(marked && pos >= nextcheck) + if(pos >= nextcheck) ref._iter_advance(*this); return *this; } Iter operator++(int) { Iter ret(*this); - pos++; - if(marked && pos >= nextcheck) - ref._iter_advance(*this); + ++(*this); return ret; } bool operator==(const Iter& o) const { return pos == o.pos; } diff --git a/test/testdata.cpp b/test/testdata.cpp index 1e6a88b..082259b 100644 --- a/test/testdata.cpp +++ b/test/testdata.cpp @@ -78,7 +78,7 @@ void testName() }); } -void testIter() +void testIterStruct() { testDiag("%s", __func__); @@ -120,13 +120,59 @@ void testIter() testMarked(4u)<<"mark sub-struct"; val.unmark(); - val["value"].mark(); - val["alarm.status"].mark(); - val["timeStamp"].mark(); + val["value"].mark(); // 1 field + val["alarm.status"].mark(); // 1 field + val["timeStamp"].mark(); // 4 fields (struct node and 3x leaves) testMarked(6u)<<"mark sub-struct"; } +void testIterUnion() +{ + testDiag("%s", __func__); + + auto top = TypeDef(TypeCode::Union, { + members::UInt32("A"), + members::String("B"), + }).create(); + + { + auto it = top.iall().begin(); + auto end = top.iall().end(); + if(testOk1(it!=end)) + testEq(top.nameOf(*it), "A"); + ++it; + if(testOk1(it!=end)) + testEq(top.nameOf(*it), "B"); + ++it; + testOk1(it==end); + } + + testOk(top.imarked().begin()==top.imarked().end(), "imarked() empty"); + + top["->A"] = 42; + + { + auto it = top.imarked().begin(); + auto end = top.imarked().end(); + if(testOk1(it!=end)) + testEq(top.nameOf(*it), "A"); + ++it; + testOk1(it==end); + } + + top["->B"] = "test"; + + { + auto it = top.imarked().begin(); + auto end = top.imarked().end(); + if(testOk1(it!=end)) + testEq(top.nameOf(*it), "B"); + ++it; + testOk1(it==end); + } +} + void testPvRequest() { namespace M = members; @@ -264,12 +310,13 @@ void testAssignSimilar() MAIN(testdata) { - testPlan(80); + testPlan(92); testSetup(); testTraverse(); testAssign(); testName(); - testIter(); + testIterStruct(); + testIterUnion(); testPvRequest(); testConvertScalar(1.0, true); testConvertScalar(0.0, false);