-
Notifications
You must be signed in to change notification settings - Fork 6
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
traversal abort #152
Labels
Comments
Smattr
added a commit
that referenced
this issue
Nov 17, 2019
We provide an API supporting range-based for loops and legacy iteration over an AST or a subtree rooted at a particular node. The purpose of this is to abstract patterns like indexing and validation into a common implementation. This needs a const counterpart, but then it should be ready to near-entirely replace the functionality of indexing, validation, and friends. This should then be a suitable base to build Node::all, Node::any, Node::none, etc on top of. FIXME: Have not had time to review this code or do anything more than ensure it compiles. Github: related to #152 "traversal abort"
Smattr
added a commit
that referenced
this issue
Nov 17, 2019
We provide an API supporting range-based for loops and legacy iteration over an AST or a subtree rooted at a particular node. The purpose of this is to abstract patterns like indexing and validation into a common implementation. This needs a const counterpart, but then it should be ready to near-entirely replace the functionality of indexing, validation, and friends. This should then be a suitable base to build Node::all, Node::any, Node::none, etc on top of. Github: related to #152 "traversal abort"
Smattr
added a commit
that referenced
this issue
Nov 18, 2019
We provide an API supporting range-based for loops and legacy iteration over an AST or a subtree rooted at a particular node. The purpose of this is to abstract patterns like indexing and validation into a common implementation. This needs a const counterpart, but then it should be ready to near-entirely replace the functionality of indexing, validation, and friends. This should then be a suitable base to build Node::all, Node::any, Node::none, etc on top of. Github: related to #152 "traversal abort"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When using the AST traversal functionality (inheriting from one of the
rumur::BaseTraversal
children), a common pattern is searching for a node matching some criteria. I.e. you start not knowing whether a matching node exists and are effectively done as soon as you find a matching node (something likestd::any_of
). However, the traversal class has no mechanism for aborting an in-progress traversal. That is, there's no way to signal, "I'm done, please stop recursing." You can throw an exception but it is kind of unpleasant to use an exception for this kind of control flow.The effect of this is that you keep (uselessly) traversing the AST when you already know the answer. The result is correct but performance is impacted.
This was a problem that was anticipated some time ago. The most expedient solution might be something like LLVM's design. I'd avoided doing this before now because I don't find their design particularly ergonomic either. It might be the least bad one though, and is certainly an improvement on the current implementation.
The text was updated successfully, but these errors were encountered: