maxtaco / coffee-script

IcedCoffeeScript
http://maxtaco.github.com/coffee-script
MIT License
727 stars 58 forks source link

Bug in for loop with range when an await statement is nested inside (1.6.3-g) #97

Closed joshuas closed 10 years ago

joshuas commented 10 years ago

Hello,

The following example code tries to loop 10 times with an index ranging from 0 to 9, but when compiled with 1.6.3-g it will not execute the loop at all. This is due to a bug which occurs when an await is within the loop and there's a " - 1" in the range (e.g. "for i in [0..count - 1]").

Note that the code below loops fine with ICS and no nested await, and works fine in pure CoffeeScript with the await/defer removed. It isn't a precedence issue with the "-" operator and the 1. Adding parenthesis doesn't fix it.

doAThing = (cb, i) ->
  console.log i
  cb()

count = 10
for i in [0..count - 1]
    await
        doAThing(defer(), i)
    console.log 'Done.'

Translates with 1.4.0c to:

// Generated by IcedCoffeeScript 1.4.0c
(function() {
  var count, doAThing, i, iced, __iced_deferrals, __iced_k, __iced_k_noop, _i, _next, _while,
    _this = this;

  iced = require('iced-coffee-script').iced;
  __iced_k = __iced_k_noop = function() {};

  doAThing = function(cb, i) {
    console.log(i);
    return cb();
  };

  count = 10;

  i = 0;
  _while = function(__iced_k) {
    var _break, _continue;
    _break = __iced_k;
    _continue = function() {
      return iced.trampoline(function() {
        ++i;                                <---------- Note this line and go to 1.6.3-g below.
        return _while(__iced_k);
      });
    };
    _next = _continue;
    if (!(i <= count - 1)) {  <---------- Note this line and go to 1.6.3-g below.
      return _break();
    } else {
      (function(__iced_k) {
        __iced_deferrals = new iced.Deferrals(__iced_k, {
          filename: "lib\testloop.coffee"
        });
        doAThing(__iced_deferrals.defer({
          lineno: 7
        }), i);
        __iced_deferrals._fulfill();
      })(function() {
        return _next(console.log('Done.'));
      });
    }
  };
  _while(__iced_k);

}).call(this);

...and translates with 1.6.3-g to:

// Generated by IcedCoffeeScript 1.6.3-g
(function() {
  var count, doAThing, i, iced, __iced_deferrals, __iced_k, __iced_k_noop, _i, _next, _while,
    _this = this;

  iced = require('iced-coffee-script').iced;
  __iced_k = __iced_k_noop = function() {};

  doAThing = function(cb, i) {
    console.log(i);
    return cb();
  };

  count = 10;

  i = 0;
  _while = function(__iced_k) {
    var _break, _continue;
    _break = __iced_k;
    _continue = function() {
      return iced.trampoline(function() {
        --i;                        <-------- Note this line is now --i not ++i.
        return _while(__iced_k);
      });
    };
    _next = _continue;
    if (!(i >= count - 1)) {   <-------- Note this line is now >= not <=.
      return _break();
    } else {

      (function(__iced_k) {
        __iced_deferrals = new iced.Deferrals(__iced_k, {
          filename: "lib\\testloop.coffee"
        });
        doAThing(__iced_deferrals.defer({
          lineno: 7
        }), i);
        __iced_deferrals._fulfill();
      })(function() {
        return _next(console.log('Done.'));
      });
    }
  };
  _while(__iced_k);

}).call(this);

I'm afraid I do not know in which version this bug was introduced. We were using 1.4.0c and upgraded to 1.6.3-g, so it's somewhere between 'em!

davebond commented 10 years ago

Seems to have been introduced in 14df8993a97fcfd8fb545d548643b041a9309bf1, if the ending value of the splat is an expression it defaults to a negative stride.

Dirty quick fix until this is fixed, make start and end value expressions

for i in [0+0..count-1]

Though still test this, its comparing strings with "0+0" <= "count-1", I think