Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #4568: refine sameLine implicit object tagging #4598

Merged

Conversation

helixbass
Copy link
Collaborator

@GeoffreyBooth this fixes #4568 but still preserves @xixixao's fix for #4533

Refines #4534 by stopping the traversal up the stack (looking for implicit objects to tag as no longer being sameLine) once we see something non-implicit (eg an EXPRESSION_START)

This fixes @jo-soft's reported regression in #4568 (simplified):

fn bar: {
  foo: 123
} if not condition

Basically what was happening post-#4534 was that when it hit the OUTDENT after 123, it was tagging the bar: ... implicit object as no longer being sameLine. The correct (and pre-#4534) behavior is that it should treat the OUTDENT as being contained to within the literal {...} (represented by the literal { being at the top of the stack). So we should stop traversing the stack tagging implicit objects as sameLine: no once we hit that literal {. But for cases like #4533, we can still preserve this approach of traversing the stack tagging sameLine: no as long as we're only traversing through implicit objects/calls

@GeoffreyBooth GeoffreyBooth requested a review from lydell July 4, 2017 03:49
@GeoffreyBooth
Copy link
Collaborator

This is a very clean fix. Looks good to me. @lydell?

@helixbass thank you for finding a fix for this that doesn’t diverge the 1.x and 2 lines.

Copy link
Collaborator

@lydell lydell left a comment

Choose a reason for hiding this comment

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

I'll give my usual review for this kind of things: I'm no good at the rewriter, but @helixbass has clearly put a lot of thought into this, and if the tests pass and it fixes something, then go for it! Nothing stands out in the code.

@GeoffreyBooth GeoffreyBooth merged commit e4bf163 into jashkenas:master Jul 4, 2017
@helixbass helixbass deleted the iss4568_implicit_object_regression branch December 28, 2019 18:15
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.

Incompatible code generated with 1.12.6 using function call followed by if statement.
3 participants