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

Bug: when using (b=1; return) if true followed by any statement #5013

Closed
rosenfeld opened this issue Mar 14, 2018 · 10 comments · Fixed by #5014
Closed

Bug: when using (b=1; return) if true followed by any statement #5013

rosenfeld opened this issue Mar 14, 2018 · 10 comments · Fixed by #5014
Assignees
Labels

Comments

@rosenfeld
Copy link

Choose one: is this a bug report or feature request? Bug

Input Code

a = ->
  (b=1; return) if true
  #b=2

The code above compiles successfully in the last release, but if you uncomment the last line then it throws "error: cannot use a pure statement in an expression". This used to work but after upgrading to latest CoffeeScript my existing code is broken. Was this change intentional?

  • CoffeeScript version: 2.2.3
  • Node.js version: 8.9.1
@rosenfeld
Copy link
Author

This bug seems to have been introduced in release 2.0.2 since it was working on 2.0.1.

@vendethiel
Copy link
Collaborator

The compiler in 2.0.2+ tries to compile (b=1;return) using the comma operator.
Seems like 6faa7f2 is the culprit, removing a unwrapAll.
The removal was to accomodate with Flow, but broke this particular behavior.

@vendethiel vendethiel added the bug label Mar 14, 2018
@zdenko
Copy link
Collaborator

zdenko commented Mar 14, 2018

This looks more like #3441. The PR for it (#4849) didn't cover Return (which is the instance of Return and not StatementLiteral).

@zdenko zdenko self-assigned this Mar 14, 2018
@vendethiel
Copy link
Collaborator

@zdenko Ah, I tend to forget Coffee doesn't have the HURL type. Anyway, if you add the unwrapAll back where I linked, you'll see the Flow tests failing, but this one bug fixed. It's better fixed where you did, though :-).

@zdenko
Copy link
Collaborator

zdenko commented Mar 14, 2018

@vendethiel why is this not allowed -> (yield return)?

@vendethiel
Copy link
Collaborator

@zdenko What do you suggest it compiles to?

@zdenko
Copy link
Collaborator

zdenko commented Mar 14, 2018

(function* () {}); like here?

@zdenko
Copy link
Collaborator

zdenko commented Mar 14, 2018

@lydell I think you might be the right person to ask about this. Why can't -> (yield return) compile to (function* () {});?

@vendethiel
Copy link
Collaborator

Oh, that's right, we have this syntax. Well, yield return is a YieldReturn is a Return, so the fix for this issue probably fixes that as well.

zdenko added a commit to zdenko/coffeescript that referenced this issue Mar 15, 2018
zdenko added a commit to zdenko/coffeescript that referenced this issue Mar 15, 2018
@lydell
Copy link
Collaborator

lydell commented Mar 15, 2018

@zdenko Sorry, I have no idea why that doesn't compile. Your suggested compilation sounds sensible, though.

GeoffreyBooth pushed a commit that referenced this issue Mar 18, 2018
* fix #5013

* disallow statement in the expression
@GeoffreyBooth GeoffreyBooth mentioned this issue Mar 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants