Use AST Inlining from Black Diamonds #231
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
smarr
added
the
enhancement
ANTLR has been used years ago for SOM, was never used in TruffleSOM or SOMns Signed-off-by: Stefan Marr <[email protected]>
Signed-off-by: Stefan Marr <[email protected]>
|
Ok, so, here we go, this should make things work with SOMns. Unfortunately, this increases warnings with |
This was referenced Mar 14, 2018
Start using BD's versions of various basic mechanisms: - ProgramDefinitionError - DummyParent - IdProvider - WithSource - use NodeState for communicating details for self/super node creation - inlining/adapting to scope changes Signed-off-by: Stefan Marr <[email protected]>
Signed-off-by: Stefan Marr <[email protected]>
Signed-off-by: Stefan Marr <[email protected]>
Signed-off-by: Stefan Marr <[email protected]>
Signed-off-by: Stefan Marr <[email protected]>
Signed-off-by: Stefan Marr <[email protected]>
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.
With this PR, we are adopting the inlining/splitting support from Black Diamonds: SOM-st/black-diamonds#6
With 346842b, we unfortunately revert #177
While there is a performance issue, it is not possible to simply apply the solution in #177. It is semantically not sound, because it results in scopes pointing to the wrong parent. And at the moment, we need a direct parent chain.
The only solution seems to be to revamp scope handling entirely in SOMns. At the moment we mix run-time info and compile-time info (lexical scopes vs. root nodes), which is leading to this issue.
Think, I need to move the run-time info entirely in root nodes or SInvokable/SObject.
How to do that is not entirely clear yet to me. One step in doing this is going to be #233. But there is much more work needed, which I can't do at the moment. So, this means we have a performance regression in the mean time. But it's a somewhat obscure benchmark, so well...
We should also make sure that any issue cleaning this up, removes unnecessary tracking introduced by #177.
The corresponding PR for TruffleSOM is SOM-st/TruffleSOM#13