Skip to content

Conversation

@odersky
Copy link
Contributor

@odersky odersky commented Nov 16, 2016

Some trees, which do not consume input have unassigned positions (so that
they can fit in whatever range they are integrated). It's therefore risky
to take the start or end of a parsed tree's position. This commit guards
against the case where the position of the tree does not exist.

Fixes #1707. Review by @liufengyun

@odersky
Copy link
Contributor Author

odersky commented Nov 17, 2016

Previous PR fell short; replaced with a scheme that's more general and thorough.

@odersky odersky changed the title Fix #1705: Make sure errorTermTrees have positions Fix #1705: Survive non-existing positions in parser Nov 17, 2016
@odersky
Copy link
Contributor Author

odersky commented Nov 17, 2016

Rebased to master to eliminate that pesky test failure.

@rethab
Copy link
Contributor

rethab commented Nov 20, 2016

@odersky are you sure this fixes #1705 ? Seems rather unrelated

@smarter
Copy link
Member

smarter commented Nov 20, 2016

Indeed, it's actually #1707, neg/i1705.scala should be renamed to neg/i1707.scala

Some trees, which do not consume input have unassigned positions (so that
they can fit in whatever range they are integrated). It's therefore risky
to take the start or end of a parsed tree's position. This commit guards
against the case where the position of the tree does not exist.
@odersky
Copy link
Contributor Author

odersky commented Nov 24, 2016

Test file renamed and rebased to master

@liufengyun
Copy link
Contributor

LGTM @odersky , changes are straight-forward.

@odersky odersky changed the title Fix #1705: Survive non-existing positions in parser Fix #1707: Survive non-existing positions in parser Nov 24, 2016
@odersky odersky merged commit 566e8f7 into scala:master Nov 24, 2016
@allanrenucci allanrenucci deleted the fix-#1705 branch December 14, 2017 16:59
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.

5 participants