implement aslBackwardsVisitor #72
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Nice! My only other issue with the visitor is supporting transforms from individual statements into lists of statements. I'd be keen to get rid of the custom walks I keen implementing. |
katrinafyi
force-pushed
the
backwards-visitor
branch
3 times, most recently
from
261b750 to
396a4c6
Compare
|
There is now a draft commit to support returning stmt list. However, it has not been tested at all beyond the existing tests (which all use transforms on single statements, so really not at all). Indeed, I think this will require a more careful implementation of the backwards visitor. One thought is that we need to make sure that we do not reverse statement lists before/after ChangeDoChildrenPost. It sure would be nice to have unit tests. |
katrinafyi
force-pushed
the
backwards-visitor
branch
from
1d5265c to
630277b
Compare
BREAKING! This change affects the signature of the Asl_visitor.visit_stmt method. For compatibility, a visit_stmt_single method is provided with equivalent behaviour to the old visit_stmt. There is also an added helper function to convert visitActions on single statements to visitActions on a list of statements. Both of these compatibility helpers WILL THROW if used with a visitor that returns non-singleton statement lists. This gives the user the flexibility to insert new statements or delete statements entirely. On the other hand, post-functions in ChangeDoChildrenPost will need to handle lists of functions as well. This follows the original CIL visitor: https://people.eecs.berkeley.edu/~necula/cil/api/Cil.cilVisitor.html
it is no longer a good idea for the backwards and forwards visitors to have a subtyping relation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a visitor suitable for basic backwards analyses. In doing so, we refactor the visit_* functions to place them inside a class. This makes it possible to override one or more of the methods, as we do in aslBackwardsVisitor for visit_stmts.
The backwards visitor has the same limitations as the existing forwards visitor. That is, merging results at control flow joins is difficult to do. However, for simple analyses, this can be worked around with careful use of enter/exit scope.
There is naming confusion in aslVisitor / aslForwardsVisitor / aslBackwardsVisitor which would be good to fix. aslVisitor defines the visit actions for specific AST nodes, whereas the aslForwardsVisitor/aslBackwardsVisitor applies these recursively in a particular order. Maybe these can be called aslForwardsTransform...?