[CS2] Fix #4413: Generated variables in function parameters

[GeoffreyBooth]

See #4413. If a function parameter is an expression that triggers the creation of a generated variable (e.g. a variable named ref or ref1 etc.), that variable needs to be declared in the parent scope, not the function scope.

[vendethiel]

Seems dangerous WRT recursive calls and the like.

[GeoffreyBooth]

How so?

[connec]

Would the code I proposed in #4413 not be a bit simpler (e.g., compile parameters with the enclosing scope)?

[GeoffreyBooth]

Would the code I proposed in #4413 not be a bit simpler (e.g., compile parameters with the enclosing scope)?

I tried that, but I got lots of failed tests. Also I don't think we want all function parameter variables declared in the parent scope, only generated ones. New variables due to destructuring, for example, shouldn't get declared in the parent scope.

[GeoffreyBooth]

This might help illustrate. This code:

'use strict'

foo = -> null
bar = -> 33

f = (a = foo() ? bar()) -> a
g = (a = foo() ? bar()) -> a + 1

console.log f()
console.log g()

compiles to:

'use strict';
var bar, f, foo, g, ref, ref1;

foo = function() {
  return null;
};

bar = function() {
  return 33;
};

f = function(a = (ref = foo()) != null ? ref : bar()) {
  return a;
};

g = function(a = (ref1 = foo()) != null ? ref1 : bar()) {
  return a + 1;
};

console.log(f());

console.log(g());

and when executed prints:

33
34

[connec]

Fair enough, looking back at it I see why it may cause failures.

This LGTM I think, though I'd be interested to know which cases @vendethiel was thinking of above.

[vendethiel]

I need to do some testing but I'm on holidays with no way to reliably clone/test, I'm afraid.

[vendethiel]

I was thinking of some very contrived case, e.g.

./bin/coffee -bce 'f = (rec, a = (if rec then (f false) ? 0 else 0), ref) -> rec'

Where the ref would be poisoned by a call to the function itself in an argument default.

Probably not an actual issue, so LGTM :).

[GeoffreyBooth]

@vendethiel Your example becomes:

var f, ref1;

f = function(rec, a = (rec ? (ref1 = f(false)) != null ? ref1 : 0 : 0), ref) {
  return rec;
};

And when run with e.g. f(1, 2) it returns 1. Does this look good to you?

[vendethiel]

I think it's fine, yes.

[GeoffreyBooth]

Thanks @vendethiel.

@lydell, any notes? @connec, does this look good to you?