Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling mutating trees with iterators #2

Open
mortenpi opened this issue May 17, 2022 · 1 comment
Open

Handling mutating trees with iterators #2

mortenpi opened this issue May 17, 2022 · 1 comment

Comments

@mortenpi
Copy link
Member

Currently when e.g. iterating over children(node) you can mutate the tree while the iteration is happening. This will likely lead to unexpected behavior (note: changing or updating the AbstractElement is fine).

We should minimally document that you should not do that. However, I wonder if there is something else we could do to make sure that you don't get bad behavior. A few options:

  1. Collect Nodes into an array when iterator is constructed and then naively iterate over that array instead. If some of them get unlinked etc., then that won't affect the iteration per se. However, this will mean allocating a potentially big array (especially in the whole-tree case).. we could have an keyword argument for iterator functions to allow for unsafe, but efficient iteration (e.g. children(node, unsafe=true)?

  2. Make the tree immutable during iterators. This would mean attaching some global metadata to each node (e.g. something as simple as a Ref{Bool}).

@mortenpi
Copy link
Member Author

mortenpi commented Aug 11, 2022

Make the tree immutable during iterators. This would mean attaching some global metadata to each node (e.g. something as simple as a Ref{Bool}).

Alternatively, if we do it carefully, we could wrap all Node instances that are returned in the tree iterators in some other type, e.g.

struct ImmutableNode{T}
  node::Node{T}
end

that does not allow mutating the fields, and all the mutating functions would fail on this node. getproperty would be forwarded to .node, but would still have to wrap anything that returns a Node in another instance of ImmutableNode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant