michaelficarra / CoffeeScriptRedux

:sweat: rewrite of the CoffeeScript compiler with proper compiler design principles and a focus on robustness and extensibility
https://michaelficarra.github.com/CoffeeScriptRedux/
BSD 3-Clause "New" or "Revised" License
1.84k stars 111 forks source link

Helper functions from previous API call outputs in next call too #323

Open lydell opened 10 years ago

lydell commented 10 years ago
$ node -pe "var cs = require('./lib/module'); cs.cs2js('a=c in d') + '\n--------------\n' + cs.cs2js('a=1')"
// Generated by CoffeeScript 2.0.0-beta9-dev
void function () {
  var a;
  a = in$(c, d);
  function in$(member, list) {
    for (var i = 0, length = list.length; i < length; ++i)
      if (i in list && list[i] === member)
        return true;
    return false;
  }
}.call(this);
--------------
// Generated by CoffeeScript 2.0.0-beta9-dev
void function () {
  var a;
  a = 1;
  function in$(member, list) {
    for (var i = 0, length = list.length; i < length; ++i)
      if (i in list && list[i] === member)
        return true;
    return false;
  }
}.call(this);

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

lydell commented 10 years ago

I think this happen because enableHelpers is a module-global variable. It should be created on each invocation.

lydell commented 10 years ago

Are there any API tests? In case I end up making a PR…

lydell commented 10 years ago

Does it make sense to pass helpers to each rule, just like inScope, ancestry etc.?

michaelficarra commented 10 years ago

I don't think so. helpers is something that needs to be mutated (or threaded through return values monadically), right? inScope and ancestry are entirely informational. They are built up in the traversal. Rules do not mutate them or pass out new values for them.

lydell commented 10 years ago

Then I don't know how to fix this issue :(

michaelficarra commented 10 years ago

Yeah, it's a shitty one. You can keep the state in the Compiler instance and mutate it there instead of threading it through the compilation rules. It's cleaner. Unfortunately, JavaScript/CoffeeScript do not allow for very powerful abstraction.

lydell commented 10 years ago

I thought about keeping the state on the Compiler instance as well, but the rules don't seem to have access to it?

lydell commented 10 years ago

Just found https://github.com/michaelficarra/CoffeeScriptRedux/commit/efaa09b7183f0d8e43a63c5ae4a9548f42582f08:

next up, remove {h,enabledH}elpers statefulness