Deal with `this` and `arguments` about continuation HOT 8 CLOSED

I believe this can be solved by compiling

async(stuff, cont(err, result));
...

To the following:

async(stuff, (function(arguments, err, result) {
    ...
}).bind(this, arguments));

So, in words: Add a preceding parameter arguments to the generated
continuation and call .bind() with arguments this and arguments.

I'm afraid the source for the compiler is a bit opaque to me. If you can point
me to the relevant bits I can implement it myself. I don't write
much javascript :-)

from continuation.

I forgot that you compile

async(stuff, cont(err, result));
...

to

async(stuff, function() {
  var err = arguments[0];
  var result = arguments[1];
  ...
});

You'd then have to change that as well to compile to

async(stuff, function(err, result) {
  ...
});

Maybe there's more I'm missing...

from continuation.

Okay, I think I figured out how to make the changes. I made a fork. Check out this commit. Note that it will fail if you feed it garbage like

async(stuff, cont(err, result + 1));
...

The compiler should probably report an error in that case.

from continuation.

Hi,

Thank you very much for your contribution. I have tried your commit just now, but there is something wrong that breaks the unit test (if.js, pi.js, list.js, parallel.js, parallel_exception.js, multiple_parallel.js). Please rebase your repo to the latest commit and run npm test and see if all tests under Compile and run pass.

Thanks

from continuation.

Hello,

The cause of the errors is because I don't handle parameters that are not
identifiers (i.e. r[0] in cont(err, r[0])). I thought handling
non-identifiers was a bug, not a feature. I can see now by the test cases that
it's a feature.

It can be implemented, but not in the same way where you do r[0] = arguments[1]; because arguments refers to the outer functions's arguments.
It can be implemented by introducing (hopefully) fresh variables for parameters
in the same style as helpers.getLoopFunctionName().

The transformation would then look like this:

async(stuff, cont(err, r[0]));
...

async(stuff, (function(arguments, err, _$param0) {
  r[0] = _$param0;
  ...
}).bind(this, arguments));

With all respect, I think this is a misfeature, but it's not my project. If you want
I can implement it like I explained.

PS: Only list.js, parallel.js, and parallel_exception.js fail for me (besides
the difference in "expected" output, but that's expected).

from continuation.

I implemented the above. Here's the commit.

Take care,
Reynir

from continuation.

Thank you very much. I have merged your commit. The one thing I modified is I replaced all parameters with temporary variables instead of non-lvar only. This could fix the problem in unit test multiple_parallel.js.

from continuation.

While looking at issue #14 I realized that this bug hasn't been fixed fully. The continuations generated by try {..} catch() {...} still have this bug. I think there are other places as well that generate continuations / functions where .bind() isn't called. The same 'trick' can be used.

from continuation.