Skip to content

Conversation

@agocke
Copy link
Member

@agocke agocke commented Jun 18, 2019

Fixes #34483, #36112

@agocke agocke force-pushed the fix-range-precedence branch from f9f35d3 to d9eacb3 Compare June 21, 2019 17:42
@agocke agocke changed the title Implement parsing changes from LDM Change Range precedence to be just below unary Jun 21, 2019
@agocke agocke marked this pull request as ready for review June 21, 2019 17:44
@agocke agocke requested a review from a team as a code owner June 21, 2019 17:44
@agocke agocke requested a review from gafter June 21, 2019 17:44
@agocke
Copy link
Member Author

agocke commented Jun 21, 2019

Note: this doesn't implement all the changes from #36514, but it does move the Range precedence, which I think is the most impactful change as it will take working code and make it an error.

@jcouv jcouv added the Feature - Range Range label Jun 21, 2019
Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though it looks like there are test issues.

@agocke
Copy link
Member Author

agocke commented Jun 21, 2019

Not sure what happened with the test machines -- they all seemed to hang and disconnect at once. Seems OK on retry

@agocke
Copy link
Member Author

agocke commented Jun 24, 2019

@gafter If we think that we'll still move the Range precedence down, I'd still like to take this. I don't think the precedence inversion is particularly worse than what we have now, and I believe all those cases are currently semantic errors, so changing things later won't be very impactful

Copy link
Member

@gafter gafter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@agocke agocke merged commit 1d4a8a9 into dotnet:master Jun 24, 2019
@agocke agocke deleted the fix-range-precedence branch June 24, 2019 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inverted precedence parsing range operator

5 participants