-
Notifications
You must be signed in to change notification settings - Fork 155
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
Better handling of splat func arguments (prevent v8 deoptimization) #868
Conversation
Do we have a test for cases such as |
list = @"rend#{ left.constructor.display-name }" o, items, rite | ||
if rite.to-string! is \arguments and not ret | ||
arg = true | ||
if left not instanceof Arr then @carp 'arguments can only destructure to array' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please newline that if
Ah you got me. A closure is generated in the case of EDIT: of course the following would still generate sub-optimal (but correct) code but IMHO those who actually write this totally deserve it. Indeed they somehow make sense if named, so I'm not going to hack further... [a, ...[b], c] = &
[a, ...[b, ...c], d] = & |
That's fine, don't rush it! Just wanted to make sure it worked. |
Thought it was hard but actually there is a simple but inelegant fix. What prevents a good fix boils down to something simple: [a]:b = [.. for til 4] At top level this should not compile to a closure, but it does right now, i.e. #856 (funny I raised this one too). |
`[.. 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.
8389fcf
to
cbc6a7e
Compare
@gkz is that okay? |
see #857, jashkenas/coffeescript#3274 for discussion