Conversation
Keep each node's children behind Rc<Node> instead of Rc<RefCell<Node>> no longer allowing mutation through a shared child reference. At the same time store conditional nodes behind Rc handles instead of directly so that cloning a handle shares existing syntax without copying the entire conditional subtree. Make process_content read-only and extract process_content_steps so the same evaluation can operate on borrowed instruction sequences (later via pre-dicts but already allowed here). It only requires an iterable sequence of trackable lifetime borrowed content steps and only copies the ones that are actually needed. Adapt child insertion, conditional parsing, dumping, and traversal to the new representation. Python boundary conversions and child getters still return copied/detached Node values for inspection. Signed-off-by: Plamen Dimitrov <plamen.dimitrov@intra2net.com>
PreDict walks parsed syntax and accumulates the state needed to produce parameter dictionaries. This includes both failed cases as well as partially processed content for the purpose of traversal. The former is entirely migrated away from the now immutable nodes (part of the overall immutable AST) and into PreDict which implements the actual tree node traversal and allows multiple diverging traversals as separate PreDict instances (pre-dicts). The latter separates a plain layer of processed node content needed for multiple traversal-instance-bound joining so that the pre-dict branch would consist of only borrowed read-only nodes and not clone all their data. The most important step to make these two possible is that pre-dicts can now be updated from an immutable node. This allows the recursive plain dict getter to only increase the reference count of a child node and provide a borrowed version when updating the pre-dict from that node. At the same time the join dict getter would increase the reference count and only clone a join-free content together with each only filter. More details about the failed cases: - Failed cases use a type alias to reduce accumulated type complexity. - The previous API of tiny methods regarding failed case manipulation has been reduced to a single "check_failed_case" method (previously "failed_case_might_pass") with the really small methods represented as direct implementations within the pre-dict node update method. - Compared to processed node content failed cases should survive any pre-dict resets as the fields being popped describe the current traversal path. - The "check_failed_case" method now accepts a similar content iterator and an explicit node content as well as failed case instance. More details about the pre-dict update from a shared node: - The borrowed node branch is only exposed externally as a clone for inspection purposes. - The implementation of the new node update method has been slightly improved in terms of reusable ctx variables. - The failed case now pushes a special "failed" frame that could then later be used by the traversal process so the parent can advance to the next child instead of retrying this one. Both PreDict migrated functions were moved to rust-local scope and thus need their own implementation block. The python side failed cases test only focuses on empty dictionaries (python interface) while a new rust side test contracts the actual pre-dict branch, route, and collected failed cases. Signed-off-by: Plamen Dimitrov <plamen.dimitrov@intra2net.com>
Add a frozen Tree object holding a shared root and expose it as an AST attribute Parser.ast. Parsing additional configuration copies the root value and returns a new tree snapshot while sharing existing descendants. Earlier snapshots and dictionary traversals already in progress retain their original syntax. Note this means that the previous pattern of parse_file/parse_string of replacing the parser node now correctly translated to a tree returning another tree and makes sense since we can never truly guarantee a previous tree is not already currently being traversed or held by another parser. The tree struct remains immutable and thus safe and we can only return a new (still guaranteed read-only) tree as a separate snapshot. Each dictionary generator starts a fresh PreDict directly from the currently held AST snapshot through a new PreDict.update_from_tree facilitator. Tree.is_empty checks the root directly without copying its children or content. The tree itself would only copy the root from a previous snapshot to then use as a mutable node with the new filename for both its own parse_string and parse_file replacing the module level functions for the python side. Parser.node becomes a getter for a detached inspection value and we also test that modifying it does not affect the snapshot. The exposed cloned node preserves the previous interface but also clarifies the referential implementation of the tree structure (via a root node and not via explicit ownership of all nodes). The newly supported interleaved traversal and parsing are tested via new contracts, inclusive of overall and fail case snapshot isolation. Signed-off-by: Plamen Dimitrov <plamen.dimitrov@intra2net.com>
Iterate over the current internal and inherited content steps (or instructions) by reference instead of cloning both sequences into a temporary final-content vector and reusing that in hot paths. Most importantly, use a borrowed final content iterator when processing inherited content and when applying instructions to the final output dictionary. The Python final_content getter still returns owned inspection values when needed. When it comes to the final output dictionary, we can also build name and shortname from borrowed label strings rather than cloning flattened label lists and then cloning each string. Token application still clones the token it consumes but this is still much less cloning than the previous approach. Signed-off-by: Plamen Dimitrov <plamen.dimitrov@intra2net.com>
Entering the join dictionary getter of a pre-dict would previously clone its entire PreDict instance, remove the current node, and then take a shallow copy. This is already wasteful for both removing a node that was just copied and for copying any unnecessary data. Initialize the join selection directly from the already processed parent frames instead, and reuse the same helper for a shallow copy. The from_frames shallow copy mostly share the same implementation with the exception of more direct calls and variable assignments. We carry forward context, internal and inherited content, shortname, dependencies and options, but start with fresh traversal and failure history. Include coverage for a test contract that the initial frame before the first node pre-dict update also survives joins at the root as well as for nested cases. Signed-off-by: Plamen Dimitrov <plamen.dimitrov@intra2net.com>
|
@katusk This is the final implementation of the immutable AST concept and overall struct redesign! I would like to hear your rustacean opinion on the way this was done. |
katusk
left a comment
There was a problem hiding this comment.
LGTM at first glance, just few nitpicks.
| ContentType::Filters(Filters::JoinFilter {filter, line }) => { | ||
| // accumulate join steps | ||
| new_joins.push(ContentStep { | ||
| filename: t.filename.clone(), |
There was a problem hiding this comment.
You already .cloned() above, so probably you can just move the t.filename, right?
|
|
||
| // might pass if any filter (external or internal) is missing from all content | ||
| let in_all_content = |step: &ContentStep| { | ||
| content.clone().any(|candidate| candidate == step) || node_content.contains(step) |
There was a problem hiding this comment.
This should be a read-only operation, content does not need to be clone()d.
| match t.content_type { | ||
| ContentType::Filters(Filters::JoinFilter {filter, line }) => { | ||
| // accumulate join steps | ||
| new_joins.push(ContentStep { |
There was a problem hiding this comment.
I'd add a ContentStep::new(...) constructor, that can be used at a bunch of different places instead of writing out all the field names -- which have different types afaics, so a new(...) should not make things more error-prone if you accidentally mix up the argument order.
No description provided.