Split line_no and node_id before new_insn_body #9898
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
|
I have no objection. (I'm not a author of |
kddnewton
force-pushed
the
split-line-node
branch
4 times, most recently
from
4f7ddb5
to
66640ac
Compare
Before this commit, there were many places where we had to generate dummy line nodes to hold both the line number and the node id that would then immediately get pulled out from the created node. Now we pass them explicitly so that we don't have to generate these nodes. This makes a clearer line between the parser and compiler, and also makes it easier to generate instructions when we don't have a specific node to tie them to. As such, it removes almost every single place where we needed to previously generate dummy nodes. This also makes it easier for the prism compiler, because now we can pass in line number and node id instead of trying to generate dummy nodes for every instruction that we compile.
kddnewton
force-pushed
the
split-line-node
branch
from
66640ac
to
844a960
Compare
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.
Before this commit, there were many places where we had to generate dummy line nodes to hold both the line number and the node id that would then immediately get pulled out from the created node. Now we pass them explicitly so that we don't have to generate these nodes.
This makes a clearer line between the parser and compiler, and also makes it easier to generate instructions when we don't have a specific node to tie them to. As such, it removes almost every single place where we needed to previously generate dummy nodes.
This also makes it easier for the prism compiler, because now we can pass in line number and node id instead of trying to generate dummy nodes for every instruction that we compile.
@ko1 would this be okay? This would really help out the prism compiler, and I think it's good for
compile.c(you can see it simplifies a lot of the callsites without having to generate dummy line nodes).