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

Improve do (x = y) -> and support do (x) ->` #960

Closed
TrevorBurnham opened this issue Dec 21, 2010 · 31 comments
Closed

Improve do (x = y) -> and support do (x) ->` #960

TrevorBurnham opened this issue Dec 21, 2010 · 31 comments

Comments

@TrevorBurnham
Copy link
Collaborator

Jeremy has just added satyr's implementation of do to master in response to issue 959. The do syntax was originally discussed at issue 788.

As of the current master, do (x = y) -> ... compiles to

(function(x) {
  if (x == null) {
    x = y;
  }
  ...
})();

treating y as a default value of x;which is really just an inefficient way of writing

(function(x){
  ...
}(y));

Changing do to rewrite in this way would also allow you to write do (x = x), which currently is nonsensical, to capture a particular value of x (useful in loops). I'd furthermore propose that do (x) be made a shorthand for do (x = x), just as {x} is a great shorthand for {x: x}.

@jashkenas
Copy link
Owner

Yo Trevor, instead of opening tickets as this stuff evolves, mind dropping by #coffeescript?

@jashkenas
Copy link
Owner

I'm afraid that do's time on master was short-lived. Closing this ticket -- we can continue the main conversation around loop scoping on #959.

@TrevorBurnham
Copy link
Collaborator Author

Now that do has been made a part of the language, I think its implementation should be tweaked a bit:

Currently, do (x = x) -> is useless, because both x's refer to the one with function scope (perhaps this form should be a syntax error if you don't want inconsistency); and do ($ = jQuery) -> ... compiles to

(function($) {
  if ($ == null) {
    $ = jQuery;
  }
  ...
})($);

instead of the clearly more efficient (and, I would argue, more readable)

(function($) {
  ...
})(jQuery);

Also, one additional tweak: do (x) => ... currently uses __bind, when it could more efficiently (and, again, readably) compile to

(function(x) {
  ...
}).call(this, x);

@rofrankel
Copy link

I actually just came here to create exactly this ticket. As far as I can tell, Coco does what Trevor is suggesting, and it would definitely be helpful to me. One use case is when the value you want to bind to the anonymous function isn't bound in the current context, e.g.

((x) -> -> x + 1)(y + 1)

What I want to say here is really:

do (x=y+1) -> -> x + 1

And I want it to compile to:

(function(x) {
  return function() {
    return x + 1;
  };
})(y + 1);

But it currently compiles to:

(function(x) {
  if (x == null) {
    x = y + 1;
  }
  return function() {
    return x + 1;
  };
})(x);

Note that this can even cause an unintuitive ReferenceError!

@rofrankel
Copy link

Whoops, forgot to add that with CoffeeScript's current implementation I am forced to bind y + 1 to a variable or forgo use of do.

@TrevorBurnham
Copy link
Collaborator Author

Still hoping to see these do improvements in the near future. Here's another optimization (noted at #1134): do functions should not return a value unless the return value is used. For instance, in the code

foo = (arr) ->
  for x in arr
    do (x) ->
      setTimeout func(x), 0
      return
  return

that inner return shouldn't be necessary in order to produce optimal code. (Of course, this matters quite a bit more when it makes the difference between a loop and a list comprehension.)

@TrevorBurnham
Copy link
Collaborator Author

Update: The State of do

I believe that this is the most important open issue on CoffeeScript right now. Since it was added to 1.0, do has become a very popular feature. However, while I'm glad to have it, its implementation leaves a lot to be desired. Its behavior should be radically changed for 1.1.0. These changes would make do both simpler and better.

What should do do?

do exists primarily to capture scope in loops. For instance,

for x in [1,2,3]
  setTimeout (-> console.log x), 50

gives you 3 three times, because there's only one x. To capture the individual x values, you would write

for x in [1,2,3]
  do (x) ->
    setTimeout (-> console.log x), 50

Allowing arbitrary values to be passed

In the current implementation, do (x = 1) -> ... is equivalent to ((x = 1) -> ...)(x), which means an error if x isn't defined. That limits the flexibility of do to define and execute anonymous functions, and it's a bit of an odd reversal: While shadowing is generally discouraged in CoffeeScript, it means that do can only pass arguments involving shadowing. I'd prefer for do (x = 1) -> .... to simply mean ((x) -> ...)(1). Only when = is unused should the shadowed variable be passed in. This also makes it easier to explain do: do (x) -> would be shorthand for do (x = x).

This would allow you to write

for x, i in arr
  do (x, i, len = arr.length) ->
    setTimeout (-> console.log x, i, len), 50

instead of

for x, i in arr
  do (x, i) ->
    len = arr.length
    setTimeout (-> console.log x, i, len), 50

and things like

do (x = (x % 2 is 0) then x else x - 1) ->

One could argue that this makes the = syntax inconsistent, since its meaning in non-do functions is so different. That's true, unfortunately, and that's one reason why I think the default argument syntax should be changed to ?=, in conjunction with deeper behavior changes; see #1091.

Improving efficiency and linearity

do functions compile slightly oddly in for loops:

for x in [1,2,3]
  do (x) ->
    setTimeout (-> console.log x), 50

becomes

var x, _fn, _i, _len, _ref;
_ref = [1, 2, 3];
_fn = function(x) {
  return setTimeout((function() {
    return console.log(x);
  }), 50);
};
for (_i = 0, _len = _ref.length; _i < _len; _i++) {
  x = _ref[_i];
  _fn(x);
}

I'd prefer to see do compile more simply to an anonymous function (as it does outside of for loops. This is more consistent with the ethos of CoffeeScript, in which the order of the compiled output matches the order of the input whenever possible. (This is, for instance, a big part of the reason why CoffeeScript doesn't support defer; see #350.) Ideally, that return before setTimeout wouldn't be there either, since the return value is never used; see #1134.

Relatedly, do in conjunction with => should compile to use .call(this), as with the wrapper, rather than using the __bind helper:

do (x) => x

becomes

var __bind = function(fn, me){ return function(){ return fn.apply(me, arguments); }; };
__bind(function(x) {
  return x;
}, this)(x);

but should be

(function(x) {
  return x;
}).call(this, x);

Summary

do is one of the weirder, but more useful, parts of CoffeeScript. We can make it more accessible and, on the whole, useful by thinking of it more simply as a way of rewriting anonymous functions, with just three rules:

  1. do (x = y) -> ... is shorthand for ((x) -> ...)(y),
  2. do (x) -> ... is shorthand for do (x = x) -> ..., and
  3. do => works the same as do ->, except that the function runs in the current context.

@TrevorBurnham
Copy link
Collaborator Author

Here's another problem with the current do syntax that I just ran into in real-world code:

do (x) ->

works, but

do func = (x) ->

does not. This makes starting a recursion with do much more difficult than it ought to be.

@jamesonquinn
Copy link

+1

@satyr
Copy link
Collaborator

satyr commented May 27, 2011

I'd prefer to see do compile more simply to an anonymous function

How does that help efficiency?

@TrevorBurnham
Copy link
Collaborator Author

@satyr I was referring to the byte code size of

var _fn;
_fn = function(i) { ... }
for for (_i = 0, _len = _ref.length; _i < _len; _i++) {
  _fn(i);
}

vs.

for for (_i = 0, _len = _ref.length; _i < _len; _i++) {
  (function(i) { ... })(i);
}

The former may be more efficient in the runtime performance sense; was that the original reasoning behind it? As #1403 reports, it's also causing scoping bugs...

(Edit: Ran a quick jsPerf, which does find the _fn approach giving better performance: http://jsperf.com/fn-vs-inline-function OTOH, is this really an optimization CoffeeScript should be doing, given that the compiler doesn't normally reorder code?)

@satyr
Copy link
Collaborator

satyr commented May 30, 2011

The former may be more efficient in the runtime performance sense; was that the original reasoning behind it?

Yes. See how it came to be:

@TrevorBurnham
Copy link
Collaborator Author

That seems like the sort of optimization that makes sense for Coco, but is an odd fit with CoffeeScript, which doesn't reorder code (except to a very limited extent for postfix expressions, of course). It's cool, but it also puts more distance between the CoffeeScript input and the JS output than one expects...

@satyr
Copy link
Collaborator

satyr commented May 30, 2011

makes sense for Coco, but is an odd fit with CoffeeScript

Maybe. Note that I didn't touch Coffee for this--adopting the patch was Jeremy's choice.

@michaelficarra
Copy link
Collaborator

I think the function should be extracted. It only makes sense to define a single function rather than define a function in each iteration of the loop.

@TrevorBurnham
Copy link
Collaborator Author

OK. I'm neutral on the issue of the single function optimization, then. The important thing is to fix the bugs and make do more versatile.

@goomtrex
Copy link

Howdy.

I agree with the proposed additions to do. Why not call a spade a spade and rename it to let?

@jamesonquinn
Copy link

I agree that let would be the more intuitive name. It's also a clearly horrible variable name (a quick survey of major languages finds "flight" in Czech as the only noun), so adding it as a keyword should not cause much damage. Therefore deprecating do and moving (eventually) to let seems feasible.

@TrevorBurnham
Copy link
Collaborator Author

The reason we call it do rather than let is that the semantics of CoffeeScript's do differ significantly from those of Harmony's let, in that do creates a function while let doesn't. That means that variable scopes, this, and keywords like break, continue, and return are all affected by do; let doesn't affect any of those things.

If CoffeeScript were to support the let keyword, it would be best for it to do a direct passthrough, compiling to Harmony's let. But as long as let is only supported by Firefox, that isn't very useful.

@jamesonquinn
Copy link

I don't care about Harmony's let. I just think that this is more like English let than like English do.

Also, I don't think people will get confused with the Harmony let semantics, because this construct has an explicit -> or => to remind you that it uses a function.

In short, let's not let other lets not let our do do it's own let. Coffeescript is Coffeescript, not Harmony.

@eirikurn
Copy link

What about with. would that be confusing at all? ;)

@jamesonquinn
Copy link

I'm sure eirikurn is being facetious, but on a serious note, what do these three keywords do in other programming languages? do is generally a way to introduce a block ("while...do..." or "for...do..." or "do ... while"). let is variable binding (in various languages from Basic to F#) - the primary purpose of this construct in Coffeescript. with is variable-binding-with-unpacking. Apart from being justly hated in a javascript context, that's just not what's happening here.

@jed
Copy link

jed commented Aug 21, 2011

i also just got bitten by a named do function, as @TrevorBurnham discusses here. this to me is the most visible flaw in the current do implementation.

@daytonn
Copy link

daytonn commented Sep 8, 2011

I think something like this would work well:

  do (x as y) ->

would compile to:

  (function(y) {

  })(x);

and

  do (x = y) ->

would compile to:

  (function(x) {
    if (x == null) {
      x = y;
    }
  })();

@jimmycuadra
Copy link

What's the status of this? As of CoffeeScript 1.1.3,

do ($ = jQuery) ->

still compiles to

(function($) {
  if ($ == null) $ = jQuery;
})($);

as opposed to the clearer and more idiomatic

(function($) {
})(jQuery);

@michaelficarra
Copy link
Collaborator

@jimmycuadra: I think you've answered your own question.

@jimmycuadra
Copy link

How so? I can't tell if a decision was reached and we're waiting for an implementation, or if more discussion was desired.

@michaelficarra
Copy link
Collaborator

Ah, you were asking about the state of the discussion, not the state of the compiler. I think it is still being discussed, but generally favourable. A good patch would probably be accepted, and at least revive the discussion.

@jashkenas
Copy link
Owner

@TrevorBurnham: The above patch should satisfy your wishes ... do's now compile to the more efficient version, and allow undefined values. The bind problem no longer exists, because we have optimized binds for local functions now. I also fixed an edge case where you would give the function a name, but not be able to pass arguments, because the assignment was an expression. @jed: I believe that's what you were asking for.

Here's the test case:

do (nonExistent = 'one') ->
  eq nonExistent, 'one'

two = 2
do (one = 1, two, three = 3) ->
  eq one, 1
  eq two, 2
  eq three, 3

do func = (two, func) ->
  eq two, 2
  eq func, func

@TrevorBurnham
Copy link
Collaborator Author

This is very welcome. Thanks, Jeremy.

@rofrankel
Copy link

Yes, thanks. :) This is a huge improvement IMO.

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

No branches or pull requests