Skip to content

Conversation

stijnkliemesch
Copy link
Contributor

Fixes Parser.throwStatement() creating ThrowStatement with incorect length

Fixes Parser.throwStatement() creating ThrowStatement with incorect length
@stijnkliemesch
Copy link
Contributor Author

The created ThrowStatement had its length property initialized with not a length value but the ASTNode's endposition value.

@gbrail
Copy link
Collaborator

gbrail commented Jan 16, 2019

Very straightforward and it doesn't break anything. However in the service of making this codebase better over time I'd still love to see a new test case. Thanks!

This added testcase specifically fails without the fix from commit 52f97f8.

Without the fix: In TokenStream.getLine(int, int) the first assertion fails since the throw statement's end position exceeds the cursor (the whole script length even). This results in a thrown AssertionError (if the assert keyword is active in the java runtime).
@stijnkliemesch
Copy link
Contributor Author

The testcase triggers a specific use of the ThrowStatement instance that, without the fix, ends up using the incorrect value.

This case is how I encountered the bug, I suppose the bug mentioned in the testcase commit message should have been a seperately reported issue before initiating this pull request.

@gbrail
Copy link
Collaborator

gbrail commented Jan 22, 2019

Thanks for the test case!

@gbrail gbrail merged commit f875a68 into mozilla:master Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants