Iced3 noautocb

[zapu]

I'm making a new pull request. It contains all the fixes from previous PR, but this one has autocb support removed. autocb will now generate an error while compiling, I feel like this might aid the iced3 adoption. I, for one, was using autocb very extensively in iced projects. Good thing that single grep might be enough to spot all places one would have to refactor, but having an error will help catching anything that might have slipped.

Using this branch, all tests pass, and also iced-runtime compiles now.

There were some problems in defer handling, but they were fixed mostly by looking at iced2 branch,

This code would not run, or even compile:

a =
  defer: ->
    @

a.defer()

Because it was parsed as:

[IDENTIFIER a] [= =] [INDENT 2] [{ {] [DEFER defer] [: :] [-> ->] [INDENT 2] [@ @] [OUTDENT 4] [} }] [OUTDENT 2] [TERMINATOR \n] [IDENTIFIER a] [. .] [DEFER defer] [CALL_START (] [CALL_END )] [TERMINATOR \n]

Instead of:

[IDENTIFIER a] [= =] [INDENT 2] [{ {] [IDENTIFIER defer] [: :] [-> ->] [INDENT 2] [@ @] [OUTDENT 4] [} }] [OUTDENT 2] [TERMINATOR \n] [IDENTIFIER a] [. .] [DEFER defer] [CALL_START (] [CALL_END )] [TERMINATOR \n]

(notice the DEFER defer instead of IDENTIFIER defer)

The defer call is still compiled as:

a.defer({
    lineno: 4,
    context: __iced_deferrals
  });

I'm not sure if that's correct... I've never seen things named defer in any iced code (and I personally dislike that this is possible), but iced-runtime does this.

[maxtaco]

This is amazing, thank you @zapu! I will hopefully get a moment to take a look this weekend!

[maxtaco]

Made some comments but this looks great, thanks @zapu !

[zapu]

@maxtaco does this pull request needs any more improvement? Or could we do the iced-runtime split and move forward? Thanks :)

[maxtaco]

sorry i've been slammed. i was going to push the iced-runtime-3 thing first, but had some issues last week and then became diverted

it's still high on my list, sorry for the delay

On Mon, Nov 16, 2015 at 11:56 AM, Michał Zochniak notifications@github.com wrote:

@maxtaco https://github.com/maxtaco does this pull request needs any more improvement? Or could we do the iced-runtime split and move forward? Thanks :)

— Reply to this email directly or view it on GitHub https://github.com/maxtaco/coffee-script/pull/168#issuecomment-157096435 .

[zapu]

OK. Thank you for getting back to me. I'll try to figure out other roadblocks, but actual code shall wait ntil this part gets "frozen".

[zapu]

Hm, things seem to just work, but iced3 fail in tests with iced-runtime-3 unless the runtime itself is compiled with iced3 (which I believe is not what it's currently precompiled with, in your iced-runtime-3 repo).

Library tests would fail with those warnings:

ICED warning: Entered dead await at Pipeliner.waitInQueue (/Users/max/src/iced/iced-runtime-3/src/library.iced:109)
ICED warning: Entered dead await at <anonymous> (/Users/max/src/iced/iced-runtime-3/src/library.iced:40)
ICED warning: Entered dead await at <anonymous> (/Users/max/src/iced/iced-runtime-3/src/library.iced:40)
ICED warning: Entered dead await at <anonymous> (/Users/max/src/iced/iced-runtime-3/src/library.iced:40)

The solution is, obviously, to get it compiled with iced3, but understanding why this happens gives me hard time. Did the library API change in a way that generated iced2 code "misuses" it and the code fails in non-obvious way?

Again, this would not affect anyone, there is just this chicken-and-egg problem when working on it - you need a new compiler to compile the runtime, but you need some runtime to run the compiler.

Or maybe I am seeing things. Can you try it?

I can make pull request with proper compiled iced-runtime-3 if that makes things easier.

[maxtaco]

I took a brief look, but now this pattern fails to compile. In other words, a library would like to use the defer() semantics as part of its work. It might be related to the problems mentioned above?

[maxtaco]

Sorry, my bad, I hadn't recompiled properly.

[maxtaco]

Ok, I pushed a new iced-runtime-3 with a few hacks to break the initial cyclic build dependency.

[zapu]

I missed one require 'iced-runtime'. Sorry if that gave any trouble.

I'll try to modify package.json now. Are we using same binary names as coffee ones, or should we add a suffix in current unstable stage?

[maxtaco]

I usually keep the binaries the same in the repo (cake, coffee) to simplify merges in the future. I just install them as iced and icake via npm. So let's do the same here.

Do you want to call them iced3 and icake3 on the install path?

Also, I like to version-pin to mainline CoffeeScript, but ran out of decimals, and semver is pretty inflexible here. So I would suggest start numbering 110.0.0. My system for this is, let's say CoffeeScript is a.b.c. Then our version is (a*100 + b).c.d, where d is our patch version for this version of CoffeeScript.

Also, can you change the header liner stamped on output files to say "Generated with IcedCoffeeScript...". I believe that comes from the version function in command.coffee.

Thanks!

[zapu]

Changed package.json. I tested it in a fresh virtualenv and it installs correctly (iced-runtime-3 is on npm! sweet :+1: ).

Later patches contain some repl fixes. The await checking in expression is similar to the one in icedStatementAssertion. I'll refactor both when I figure out how.

[zapu]

I've added support for top level Awaits. Right now this is done by wrapping everything in a function. That is, another wrapping apart from the coffee-script function wrapping... Nasty stuff :frowning:

So, following code:

console.log 'a'
for i in [0..5]
    await setTimeout defer(), 10
console.log 'b'

When compiled, becomes:

// Generated by IcedCoffeeScript 1.10.0
(function() {
  var iced;

  iced = require('iced-runtime-3');

  (function() {
    var __iced_it, __iced_passed_deferral;
    __iced_passed_deferral = iced.findDeferral(arguments);
    __iced_it = (function(_this) {
      var i;
      return function*() {
        var __iced_deferrals, j;
        console.log('1');
        for (i = j = 0; j <= 5; i = ++j) {
          __iced_deferrals = new iced.Deferrals(__iced_it, {
            parent: __iced_passed_deferral
          });
          setTimeout(__iced_deferrals.defer({
            lineno: 3
          }), 10);
          if (__iced_deferrals.await_exit()) {
            yield;
          }
        }
        return console.log('aa');
      };
    })(this)();
    return __iced_it.next();
  }).call(this);

}).call(this);

Compiling with "bare" flag on yields:

// Generated by IcedCoffeeScript 1.10.0

(function() {
  var __iced_it, __iced_passed_deferral;
  __iced_passed_deferral = iced.findDeferral(arguments);
  __iced_it = (function(_this) {
    var i;
    return function*() {
      var __iced_deferrals, j;
      console.log('1');
      for (i = j = 0; j <= 5; i = ++j) {
        __iced_deferrals = new iced.Deferrals(__iced_it, {
          parent: __iced_passed_deferral
        });
        setTimeout(__iced_deferrals.defer({
          lineno: 3
        }), 10);
        if (__iced_deferrals.await_exit()) {
          yield;
        }
      }
      return console.log('aa');
    };
  })(this)();
  return __iced_it.next();
}).call(this);

It would still be a valid javascript if we generated code without the wrapping function, but it needs some changes to Await class and possible others.

[maxtaco]

@zapu, what are your thoughts on getting rid of the iced_passed_deferrals machinery? It would make the generated code a lot cleaner, which would be nice. If we have generators, we'll get nice stack traces, so it doesn't seem as useful.

[zapu]

Ad. iced3/icake3 - I do all development in virtual envs (I use python3 virtualenv and nodeenv). I can write more if you are interested. But we can change names if this would help you.

If iced_passed_deferrals is really obsolete, then sure :+1: I can try to remove it. But to be honest, my understanding is still limited, even though I feel pretty confident in the compiler code itself, so I'd rather leave final judgement to you.

[maxtaco]

Ok, good point about virtualenvs, and speaking of which, we don't want to be in the python2/python3 toss-up.

I can work on stripping out the deferral-chain walking stuff, but it might sit on the back burner for a while. in general, it's safe to strip out that code as long as it still works.

[zapu]

I think you can even use nodeenv without python virtualenv, I'm just used to doing things this way. I nodeenv solution because I can try to install a package "from scratch" in a fresh environment and see if it works.

OK, I'll try to get a better understanding of deferrals and see what I can do. I also have some stacktrace problems in compiler itself (the line numbers are almost always wrong if compiler errors out) and I'm unsure what's at fault. But I remember it has always been hit or miss, even with regular coffeescript :(

[maxtaco]

The reasoning behind iced_passed_deferrals was to reassemble the "iced" call stack after an exception. There's code in the runtime library to walk to chain of callbacks and reassemble which await'ed functions created the current (sliced up) callstack. So it was only a debugging feature all along and wasn't required for correct program execution.

[zapu]

I've been looking into babel today. For the translator to work, we will need to install additional packages: babel-plugin-transform-regenerator, babel-core, and probably babel-polyfill. Then, the translation can be invoked somewhere in coffee-script.coffee. Quick and dirty translation code would look like that:

diff --git a/src/coffee-script.coffee b/src/coffee-script.coffee
index 6ff20b0..6f55e11 100644
--- a/src/coffee-script.coffee
+++ b/src/coffee-script.coffee
@@ -82,6 +82,16 @@ exports.compile = compile = withPrettyErrors (code, options) ->
     # Copy the code from each fragment into the final JavaScript.
     js += fragment.code

+  babel = require 'babel-core'
+  regen_plugin = [
+    require("babel-plugin-transform-regenerator"),
+    { async: false, asyncGenerators: false }
+  ]
+  babel_opts = plugins: [ regen_plugin ]
+  if options.sourceMap
+    babel_opts.inputSourceMap = map.generate(options, code)
+
+  babel_transformed = babel.transform(js, babel_opts)
+  js = babel_transformed.code
+
   if options.header
     header = "Generated by IcedCoffeeScript #{@VERSION}"
     js = "// #{header}\n#{js}"

babel_opts contains options for babel compiler. It looks like they have inputSourceMap option which lets us give it a source map of the input js, and then the output source map will be "based" on it. I have not tested the source maps yet, though.

Example generation looks like this:

(ies6) zapu@weakfox ~/iced_es6/babel-coffee-script » cat es6.coffee                         7:28:35
a = -> await setTimeout defer(), 10

a()
(ies6) zapu@weakfox ~/iced_es6/babel-coffee-script » iced -c -p es6.coffee                  7:28:36
// Generated by IcedCoffeeScript 1.10.0
(function () {
  var a, iced;

  iced = require('iced-runtime-3');

  a = function () {
    var __iced_it, __iced_passed_deferral;
    __iced_passed_deferral = iced.findDeferral(arguments);
    __iced_it = (function (_this) {
      return regeneratorRuntime.mark(function _callee() {
        var __iced_deferrals;

        return regeneratorRuntime.wrap(function _callee$(_context) {
          while (1) switch (_context.prev = _context.next) {
            case 0:
              __iced_deferrals = new iced.Deferrals(__iced_it, {
                parent: __iced_passed_deferral
              });
              setTimeout(__iced_deferrals.defer({
                lineno: 0
              }), 10);

              if (!__iced_deferrals.await_exit()) {
                _context.next = 5;
                break;
              }

              _context.next = 5;
              return;

            case 5:
            case 'end':
              return _context.stop();
          }
        }, _callee, this);
      });
    })(this)();
    return __iced_it.next();
  };

  a();
}).call(this);

We need to somehow require the regeneratorRuntime part of babel. It's in babel-polyfill package.

Since there is no reason to use babel for iced when ran in node, the generation should be somehow limited to use when compiling for browsers. There should probably be an additional command line switch to do babel compilation. A question is whether we shall include regeneratorRuntime as part of iced compilation or remind user to do it themselves as part of their build process. If the runtime switch is browserify, we can require it along with iced-runtime. I am unsure how feasible trying to inline the regenerator-runtime is.

[zapu]

@maxtaco Would it be possible to merge current branch soon? It is in kind of a limbo state right now where I'm not sure what goes in and what doesn't. After a merge I would be able to have a "stable" revision (or maybe a "milestone") from which I could do more tinkering. Thanks!

[maxtaco]

Done! Sorry, I wasn't sure where the ball was.

[zapu]

Thanks for merging this in! Could you try to merge in changes from coffee-script? What should the procedure for this be? Trying to rebase and keep iced patches always "on top" every time is probably too messy. But there should be nothing wrong in "merge/patch" commits every time there are significant changes in coffee-script.

I would like to try to simplify code changes even further (most of await/defer transformations would be much simpler if done by hand, I think), but this needs few architectural changes and wasn't exactly going deep into that yet.

What's your usecase with iced right now? Is there a reason to try to rush iced3 branch out of the door? I could focus on that instead.

[maxtaco]

I think you're right, that merging is the appropriate strategy. I made myself a doc for merging iced2 here and hopefully iced3 will generate way fewer conflicts.

Everything ok with the iced3 merge? I just pushed the button on GitHub for merge, if that left something out, or did the wrong thing, let me know, I can revert and try something else.

[zapu]

I unfortunately cannot check right now, I overwhelmed myself with other things for this evening, but I'll look at it tomorrow. But I don't think both of PRs should have been merged, the other one was the autocb attempt and this one is without autocb. I'm not even sure how those two did not conflict. But again, I'm not looking at the code right now.

Could you try to fill me in on the last point? I had a lot of iced code in the past but I don't work on any iced-powered project at the moment. The "preferred" technology stack has changed quite a bit since then... So I'm kind of in the dark in what people want here.

[zapu]

I've fetched the branch, compiled, ran the tests and also some of my test files and everything seems to work fine :+1:

[maxtaco]

On the final point, our use case is that we make heavy use of iced2 in production, and iced3 is a nice-to-have future-migration strategy, but nothing urgent. No need to rush in other words.

Glad the merge worked ok. As for weirdness around yesterday's merge, it seemed almost like a bug in GitHub...

[maxtaco]

Sorry to be away for so long. Just paging this all back in. I had a couple of questions:

• Seems like the browser tests are all broken. I'm not sure why, but it's likely a runtime issues.
• More important, do you think rebase or merge is a better strategy? If rebasing, we can squash the merge to just one commit. It might be easier to read the git history that way. What do you think? BTW, I just tried it both ways. They both were very smooth with only one trivial conflict.

[zapu]

iced2 used browserify to make browser version, iced3 (and current coffee-script, I guess?) just concats the sources and minifies everything. So runtime is never added, and iced = require('iced-runtime-3) just returns undefined, hence, e.g. iced["const"] fails somewhere along the way in tests.

I'm looking into it, but maybe do you have any ideas how to cleanly add runtime to the browser bundle?

[zapu]

Also about keeping up with mainline coffee-script, I just tried to rebase with squashing everything but last commit (or first? this is confusing). This is a bit outside of my git comfort zone, but it seemed to work in the end, the only manual fix was needed in SWITCHES in command.coffee.

This loses all the iced history, unfortunately, but makes the whole thing looks like mainline coffee with a patch at the end, which is a good thing, maybe?

Now that I'm thinking about this more, I'm not sure if we can use this strategy of constant rebasing - we will be messing with everyone else who forks iced-coffee-script - even us. Every time this is done in iced-coffee-script, everyone might have a problem with updating their local branches.

[zapu]

We also do not support inline runtime generation (see https://github.com/maxtaco/coffee-script/blob/iced3/src/nodes.coffee#L2418). Entire InlineRuntime is missing, and probably for good reason: it would be a big chore to recreate runtime with nodes. I experimented with doing something like https://github.com/maxtaco/coffee-script/blob/iced3/Cakefile#L140-L162 to emit self-contained iced-runtime-3.

You can see it here: https://github.com/zapu/coffeescript/commit/fdf0b39d72e4cbebb5bdb9efa89053546ad7a336

Similar code would be needed to fix browser compilation. What do you think?

[zapu]

https://github.com/zapu/coffeescript/commit/91d763fc04341e3209c622c4d873da756d04e79e

This is the furthest I've gotten so far. Inline runtime is now neatly compiled and saved (optionally minified - does not work right now for some reason) to extras/inline-runtime.js. nodes.coffee will read this and add to code if user requests it.

This still does not fix browser tests. I've tried to change coffee compiler to be ran with runtime: inline during tests, but nodes.coffee has no way of adding the runtime then - no fs module, and in environment which such compiled compiler is dedicated for there would be no extras/inline-runtime.js either.

[maxtaco]

I fired up the keybase server-side, and all tests pass, which is great! Some bugs came up though: #173, #174 and #175. Let's put those on the blocker list before we release iced3.

[doublerebel]

Was just reading through the iced3 updates this afternoon. Awesome work guys.

I see the iced3 runtime is published to npm, is there a version of the iced3 compiler published to an unstable package? What's the best way right now to try out the current state of iced3? I can try to help tackle the blockers.

[maxtaco]

Nothing published just yet, but these steps should allow you to hack:

git clone ...
cd coffee-script
git checkout iced3
npm install iced-runtime-3
./bin/cake build
./bin/cake build:parser
./bin/cake build
./bin/cake build
./bin/coffee ....

At the last step, you have a working version of iced3, disguised as coffee..

[doublerebel]

@maxtaco thanks for the explanation, that's a big help. I will give this a try tomorrow.

[maxtaco]

Sure thing, thanks for your help and interest!

[zapu]

@maxtaco cool! a real test suite!

The InlineRuntime one is related to my 2 previous comments on this issue. I don't know how to do it properly. Can you take a look?

[maxtaco]

@zapu in general I think this is an elegant approach, better than my previous hand-rolled idea.

It feels like there should be someway around the conundrum with all of the various code-inclusion strategies at play here.

[zapu]

So we do not want to pull in something like browserify? I like the "manual" code generation because it's very simple, and it happens to work well with the iced-runtime because it's also relatively simple package (no deps).

[maxtaco]

I was trying to avoid adding browserify. It's a huge huge dependency. In practice, that's how I deal with shipping CoffeeScript code to browsers, but I don't think we should require it.

[zapu]

Eh, didn't managed to even look at it today, my Saturday was kind of busy. More luck tomorrow, I guess!

I agree about browserify, it's got crazy in Javascript world. Browserify alone would be, what, 50MBs of deps? I'll figure something out. I'm glad to be hacking on iced again!