Fix a NPE #2864
Merged
Fix a NPE #2864
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
|
Forgot to mention that the JS bug only appears starting with 4.8, thanks to a dramatic improvement by @lmy269 in PredictionContext. combineCommonParents, which on significant inputs (like 500 lines of code) improves performance by 40% <- yes |
|
Wow. Good catch. I'm really nervous about touching this stuff in Java target after so many years though.. |
|
I'll give a try to see how it affects tests and performance, just don't accept the PR |
|
Mostly concerned with breaking something! nervous about changing that stuff haha |
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.
@parrt
bumped into this bug with the JS runtime, caused I suspect by a bug which may affect all targets.
The root cause would come from PredictionContext.merge, lines 169 and 172, where an ArrayPredictionContext is created from a SingletonPredictionContext, which is some cases is an EmptyPredictionContext, with a null parent.
This translates into ArrayPredictionContext.parents being initialized to { null }, which is a nasty code smell...
I'll happily fix it if you think it makes sense, just let me know.