gkz / LiveScript

LiveScript is a language which compiles to JavaScript. It has a straightforward mapping to JavaScript and allows you to write expressive code devoid of repetitive boilerplate. While LiveScript adds many features to assist in functional style programming, it also has many improvements for object oriented and imperative programming.
http://livescript.net
MIT License
2.32k stars 155 forks source link

optimize `x in [0]`, `x in []` #987

Closed rhendric closed 6 years ago

rhendric commented 6 years ago

This applies the same optimization in use for x in [0 1] to one- and zero-element array literals.

Fix gkz/LiveScript#986.


This is a small optimization, so I'm setting the comment period for this at one week (plus change, to account for Thanksgiving weekend), after which I will merge on Nov 27 if nobody has any concerns.

vendethiel commented 6 years ago

Why not error for an empty array?

rhendric commented 6 years ago

@vendethiel, because there's nothing ambiguous or difficult to understand or handle about the empty array case, IMO. It's a silly thing to write but so is the singleton array case, and as you can see from the files changed, there were two examples of that in our very own lexer. I figure users don't set out to write unnecessarily complex expressions; they write expressions that maybe need the full power of in and then add and remove elements as their code evolves. If a user wants to keep in [x] or in [ ], for symmetry with other parts of the code, for ease of code generation, or simply out of sheer laziness, I don't see why LS should force them to change since the meaning is still perfectly clear.

Contrast with, for example, array slices, where x[] is an error instead of desugaring to []. Here, there is a good reason to error, because there's a discontinuity in the syntax: x[0] is simple element access, not a one-element slice. So it's natural to wonder what the ‘correct’ next element in the sequence x[0 1 2], x[0 1], x[0], ??? should be. Since that case is both silly and ambiguous, it makes sense to not define it and make it a syntax error. But x in [] is quite unambiguous—that code has a meaning today and shouldn't change.

vendethiel commented 6 years ago

I disagree. It's not about being "difficult to understand"-LS doesn't really care about that. It's about never meaning what you want. The only case you'd have such code is after a refactoring.

rhendric commented 6 years ago

Hm. Well, this does give me pause—I wasn't expecting controversy here. I am worried about the idea of taking an expression that isn't confusing and used to work and making it an error without good justification, and I'm not sure that ‘you probably meant to remove this code instead’ is good enough. I'm not aware of much precedent for that level of paternalism here—even the complaint about 0 == \zero is a warning, not an error, and IMO that's a much clearer sign of an issue with your code.

vendethiel commented 6 years ago

A warning would be totally fine IMHO.

rhendric commented 6 years ago

Okay, let's start this up again. Since the compiler warnings proposal didn't generate any objections, I merged it, and now this PR has warnings. I'll give anyone one more week to have second thoughts and then merge on March 14.