/LiveScript

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

Inconsistent array casting #854

Closed
Announcement opened this Issue Feb 18, 2016 · 4 comments

Comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@Announcement @summivox @rhendric @vendethiel
@Announcement

Announcement commented Feb 18, 2016

Short arrays aren't enough to loop

[to 10] does what expected and so does [.. for [0 to 10]]

But in contrast...

[.. for [to 10]] throws

Parse error on line 1: Unexpected 'FROM'
@summivox

This comment has been minimized.

Sign in to view
Contributor

summivox commented Mar 21, 2016

Just FYI: [.. for to 10] now works thanks to #859

@vendethiel vendethiel closed this Mar 21, 2016

@rhendric

This comment has been minimized.

Sign in to view
Collaborator

rhendric commented Mar 21, 2016

[.. for to 10] works, but [.. for [to 10]] still doesn't; shouldn't this remain open?
@summivox

This comment has been minimized.

Sign in to view
Contributor

summivox commented Mar 22, 2016

Lexer black magic rewrite is to blame. Specifically, lexer.ls:868 skipped a square bracket (oops!) before any "official" rewrite takes place (oops again!).

IMHO this kind of thing belong to the parser. However a refactor would be non-trivial and requires attention from @vendethiel .

I also agree that this should remain open as it is certainly a bug.
@summivox

This comment has been minimized.

Sign in to view
Contributor

summivox commented Mar 22, 2016

I am going to patch this temporarily with a grammar fix.

summivox added a commit to summivox/LiveScript that referenced this issue Mar 22, 2016

@summivox
fix gkz#854 (warning: see full commit message) 
`[.. for [to 10]]` is wrongly rewritten by lexer into `[.. for [from 0 to 10]]` (not quite, see below). Patching the lexer could be non-trivial as said rewriting is currently ad-hoc (`forange` in `lexer.ls`, see gkz#854 for discussion). This temporary solution relaxes the grammar and allow the malformed lexer output.

Caveat: `[from x to y]` is *not* supported directly from source despite the change in grammar due to the same lexer hack. The `from` is currently considered a literal rather than a `FROM` token.

A possibly cleaner solution is to getting rid of `forange` and support `from` elision in a separate lexer rewrite step or in parser instead.
4eb140e

@summivox summivox referenced this issue Mar 22, 2016

Merged

Temporary fix for #854 #870

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment