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

Implicit `for..in` comprehension conflicts with the following `in` keyword on the same depth. #923

Closed
pepkin88 opened this issue Sep 23, 2016 · 3 comments

Comments

@pepkin88
Copy link
Contributor

@pepkin88 pepkin88 commented Sep 23, 2016

The implicit version of for..in comprehension (the version without the in keyword) causes compilation error on line, where the in keyword is used, and is on the same depth. And by depth I don't mean indent, it is something different, something internal.

Minimal example:

[1 for a]
[b in c]

Extended example, with diverse ways of increasing the depth:

[[{b: {[.., 1] for a}}]]
->
  if c
    d
      .e (.f in [2 3])

I noticed, that only comprehensions are affected by this bug, and only those without the in keyword.
The construction for a => .. doesn't cause an error.

The in expression also has to be somehow wrapped. This:

[1 for a]
->
  b in c

is not erroneous.

rhendric added a commit to rhendric/LiveScript that referenced this issue Sep 23, 2016
This issue involved a lexer flag that was set in a for comprehension and
never cleared when the comprehension closed. The fix consists of
clearing all lexer flags at the current delimiter depth every time a
close delimiter is encountered. (It may be that this makes some
occurrences of `@fset _ false` elsewhere in the lexer redundant; to be
safe, I didn't attempt to remove any of them in this commit.)
@rhendric
Copy link
Collaborator

@rhendric rhendric commented Sep 23, 2016

It's always fun when you see a new issue and you know exactly what instruction must be missing to produce that effect. 😄 Nice catch.

gkz added a commit that referenced this issue Sep 27, 2016
@summivox
Copy link
Contributor

@summivox summivox commented Oct 25, 2016

@rhendric : before I stopped working on this, I was planning a migration of loophead handling to parser stage (instead of current lexer hacks)... Anyway good job figuring out the lexer fix!

@rhendric
Copy link
Collaborator

@rhendric rhendric commented Oct 25, 2016

Oh, @gkz or @pepkin88 : #925 merged, so one of you can close this—I screwed up the GitHub autoclose majyyks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants