Implement proper tail calls. #1727

mnieper · 2015-02-12T08:14:16Z

The following PR implements proper tail calls as demanded by ES6. The are behind the new experimental flag --proper-tail-calls. (Experimental because it necessarily reduces performance of the generated code.)

This probably makes Traceur the first transpiler properly supporting tail calls!

Each function that contains calls in tail position is rewritten. All tests pass with proper tail calls enabled by default. Traceur itself compiled with --proper-tail-calls true also works as expected.

At the moment, the transformer does not touch arrow functions, class declarations and class expressions (so they should be transpiled if their are supposed to be properly tail recursive). Likewise, template literals in tail position are currently not touched, so the corresponding transpilation should be enabled as well.

The changes to runtime.js are far less worse than what the diff shows. I had to wrap most of that code into an extra IIFE and the additional two spaces at the beginning of each line cause the diffs.

It is left as a TODO to check all other transformers that they do not convert tail calls in non tail calls.

Please review this PR; it's hopefully ready to merge.

arv · 2015-02-12T21:50:13Z

src/codegeneration/FromOptionsTransformer.js

@@ -158,7 +159,8 @@ export class FromOptionsTransformer extends MultiTransformer {
      append(ClassTransformer);

    if (transformOptions.propertyMethods ||
-              transformOptions.computedPropertyNames) {
+              transformOptions.computedPropertyNames ||


wrong indentation

arv · 2015-02-12T22:18:15Z

I was thinking a bit more how to better tests this instead of just hoping we would run out of stack. Maybe we could write a non feature test that inspects the continuation somehow. Another option might be to throw an error and look at the error .stack.

mnieper · 2015-02-13T07:38:31Z

I'll see how I can possibly reformulate the tests. --- I have added quite a number of comments above, please review them.

mnieper · 2015-02-13T12:29:06Z

This is one idea how we can measure the stack size:

Error.stackTraceLimit = Infinity;

var stackSize;
function recordStackSize() {
  try {
    Error.prepareStackTrace = function(error, structuredStackTrace) {
      return structuredStackTrace.length;
    }
    stackSize = new Error().stack;
  } finally {
    delete Error.prepareStackTrace;
  }
}

function assertProperTailCalls(limit = 30) {
  assert.isTrue(stackSize < limit);
}

I have checked that the test environment accounts for about 15 stack frames, so the limit of 30 is very safe when one has only about 40 or 50 tail calls and not 100,000.

Could this go in one file of the testing framework in test?

mnieper · 2015-02-13T17:02:25Z

Issues have been taken care of in the new commit.

Tests run much faster now :-)

Although the 100,000 was quite impressive...

P.S.: Who is responsible for updating the table by kangax?

arv · 2015-02-13T17:03:39Z

That would be @kangax ;-)

arv · 2015-02-13T17:07:38Z

src/codegeneration/ObjectLiteralTransformer.js

@@ -64,13 +64,25 @@ class FindAdvancedProperty extends FindVisitor {
  }

  visitGetAccessor(tree) {
-    this.visitAny(tree.name);
-    // We do not want to visit object literals in the accessor's body.
+    if (this.options_.transformOptions.properTailCalls) {


Maybe add

this.transformOptions_ = options.transformOptions;

to the constructor?

kangax · 2015-02-13T17:23:44Z

@mnieper please send PR on https://github.com/kangax/compat-table and I'll gladly merge it in! Thanks.

arv · 2015-02-13T17:25:46Z

src/runtime/runtime.js

  var $defineProperties = $Object.defineProperties;
+  var $create = $Object.create;


no need for this change

Wasn't intended.

Fixes google#218.

mnieper · 2015-02-13T18:53:17Z

Pushed the corrections.

mnieper · 2015-02-13T19:07:26Z

@kangax Thanks. I forked your project and took a look at it. But I'd like someone else to run the tests because the only browsers I have running are the latest Firefox and the latest Chromium, so this would probably give false results.

arv · 2015-02-13T19:18:36Z

LGTM

arv · 2015-02-13T19:20:22Z

Committed as d9cc38a

mnieper · 2015-02-13T19:21:35Z

Cool.

arv · 2015-02-13T19:22:12Z

Yes. Extremely cool.

mnieper · 2015-02-13T19:25:42Z

In the end it was easier than I thought. :-)

kangax · 2015-02-13T20:14:14Z

@mnieper I guess I can just change the result of TCO test from false to true for Traceur and that's it, right?

mnieper · 2015-02-13T20:24:47Z

@kangax Both TCO tests should pass. (There are equivalent tests in Traceur's test suite.)

Apart from that, %GeneratorPrototype%.return also does work now in Traceur.

For ES7, async generators have been implemented. (https://github.com/jhusain/asyncgenerator)

Finally, could you please review the test here: #1730. I think most transpilers are broken in this regard and this could be a test worth adding to your table.

…ment)

kangax · 2015-02-14T18:29:46Z

@mnieper thanks, I updated Traceur's ES6 results. For ES7, I don't believe we have a test for async generators (only async functions and async arrow functions). I'll get to that at some point. I'll also take a look at #1730 later if I get a chance — no time at the moment!

kangax · 2015-02-14T18:30:45Z

@sebmck looks like someone needs to step up their game ;)

mnieper · 2015-02-14T18:38:46Z

@sebmck I have already been asked whether I could implement this work and my work on async generators in 6to5. However, as I am doing everything in my spare time, I'd rather like to do something new than just reimplement something (and I'd have to go through another codebase to understand it).

(My new project is currently http://chibi-scheme.appspot.com/.)

Nevertheless, I hope you can take a look at this stuff and maybe find another volunteer. In any case, I will readily answer any question.

sebmck · 2015-02-14T23:25:10Z

@kangax I'm the only one working on the 6to5 internals and I do it all in my spare time so I really wish I could step up my game. 😜 Also don't mean to be pedantic but the Traceur TCO tests should be marked as "flag" on compat-table since you have to explicitly enable the properTailCalls option.

@mnieper Awesome work! Will definently look into it when I get the time. What's your preferred way to be contacted? Would love to talk about it. The main issue we ran into with TCO was it significantly slowed down a lot of calls that didn't need to be, what's the performance impact like?

TCO was disabled because there were issues with nested tail calls returning their tail call instance. TRO is currently enabled by default though and actually makes recursive calls signifcantly faster:

function fact(n) {
  function f(n, acc) { return  n === 0 ? acc : f(n - 1, acc * n); }
  return f(n, 1);
}
fact(20);

Might be worth looking into this to see if it can be implemented in Traceur by default.

mnieper · 2015-02-15T09:23:35Z

@sebmck

I'm the only one working on the 6to5 internals and I do it all in my spare time

Wow!

I haven't implemented special optimisations for self-calling functions - in fact, the code hasn't been optimised in any way yet. Running all tests with proper tail calls turned on takes 10 times the normal time (and from the way it has to be implemented in general, one has to expect a factor in that order).

For easy cases like function calling itself, yes, one should add an extra transformation pass to eliminate the recursion altogether. But as JavaScript has a lot of imperative features to write loops, I don't think this will be the majority of use cases for PTCs. I think they are much more important for programming in continuation passing style, combinator libraries and JavaScript as a compilation target for languages relying on PTCs. In this case, however, just optimising self-calling recursive functions won't help much.

For PTC to be really useful, we will wait for the browser engines to implement it (any news on this by the V8 and SpiderMonkey teams?). But with transpilers like Traceur and 6to5 allowing PTCs (albeit at lower performance) it allows library writers to already use this feature so that the libraries are already ready for production when PTC is implemented in the browser engines.

You can either contact me via email (marc.nieper@gmail.com) or via Google Talk/Jabber at the same address.

kangax · 2015-02-15T12:58:47Z

@sebmck I mostly meant it as a joke — 6to5 is already doing amazing :) We'll take care of flag situation shortly; thanks for noticing!

sebmck · 2015-02-15T13:00:05Z

@kangax Sorry! In hindsight that much was obvious, had only just woken up 😜

mnieper · 2015-02-15T13:44:53Z

@kangax How do you handle the flag thing? At the moment, the more options a transpiler offers, the worse seems to be its display on your table, doesn't it?

You would also have to add flags to all ES7 language elements Traceur supports because they are also hidden behind a flag (for the benefit of the user).

sebmck · 2015-02-15T13:50:49Z

@mnieper There's a separate table for ES7: https://kangax.github.io/compat-table/es7/

mnieper · 2015-02-15T14:02:06Z

@sebmck Yes, but all ES7 features of Traceur are behind flags, which isn't shown on that table.

arv · 2015-02-16T01:46:00Z

@sebmck 6to5 is really getting good. Like 6to5, traceur is also all done in our spare time (I try to use my 20% time but it is rare that I actually can do that). @johnjbarton is in a similar position.

@mnieper I work on V8 but PTC has not gotten on our radar yet. We have a lot of other important work to do before we will get to that.

sebmck · 2015-02-16T01:53:21Z

@arv Thanks! I always get people thinking that I have some vendetta against Traceur when really it's the opposite and I really appreciate what you guys do and I'm super happy that both projects exist. Truth is that 6to5 (now Babel) and Traceur are targeting completely different ecosystems, Traceur with AtScript/Angular while 6to5 focuses on JSX/React.

arv · 2015-02-16T04:08:01Z

@sebmck I didn't mean to imply that. I'm sorry if it came out that way.

I also don't feel happy with saying that Traceur is targetting AtScript/Angular. They liked how easy Traceur is to extend so they decided to add the transformation passes they wanted.

To me Traceur was a test bed for harmony features. We implemented a lot of the ES6 features before we brought them to TC39. Today I feel like we are still struggling to make traceur a useful tool for real world projects, something it seems Babel (nee 6to5) is doing much better at.

sebmck · 2015-02-16T04:57:21Z

@arv No, you didn't imply that at all. It's just that people always assume there's some sort of bitter rivalry between projects in the same space.

I also don't feel happy with saying that Traceur is targetting AtScript/Angular. They liked how easy Traceur is to extend so they decided to add the transformation passes they wanted.

I wasn't trying to say that these are in anyways locked into particular frameworks. Both projects can find use anywhere, but Traceur has become most popular within the Angular community as 6to5 has become in the React community.

Today I feel like we are still struggling to make traceur a useful tool for real world projects, something it seems Babel (nee 6to5) is doing much better at.

Thanks, it means a lot! Babel owes a great deal to Traceur and the standards it's created, it wouldn't exist without it and I'm incredibly grateful.

mnieper · 2015-02-16T07:49:44Z

@arv @sebmck
I think the whole developer community can consider itself fortunate that there are at least two excellent ES6 to ES5 transpilers. The more tools there are that are able to prove that some suggested features for ES7, 8, 9, etc. is actually working and will make coding in JavaScript easier, the more likely it is that this feature will be accepted in a new standard of the language.

So I really hope that you both continue your work on your tools so that both will be used a lot in the JavaScript community.

(By way of comparison, if only V8 but no SpiderMonkey, or vice versa, were in daily use, JavaScript and its engines wouldn't have seen such a rapid progress recently. asm.js is one example that readily comes to my mind.)

mnieper mentioned this pull request Feb 12, 2015

Add bundle option --preamble to prefix bundle with file content. #1725

Closed

mnieper force-pushed the proper-tail-calls branch from 134a9cc to 9da38ca Compare February 12, 2015 09:50

mnieper mentioned this pull request Feb 12, 2015

Avoid unnecessary object literal transformations. #1729

Closed

mnieper force-pushed the proper-tail-calls branch 2 times, most recently from a6a8f8b to b99a430 Compare February 12, 2015 19:43

arv reviewed Feb 12, 2015
View reviewed changes

mnieper force-pushed the proper-tail-calls branch from b99a430 to 46ea88f Compare February 13, 2015 17:01

arv reviewed Feb 13, 2015
View reviewed changes

Implement proper tail calls.

8d46594

Fixes google#218.

mnieper force-pushed the proper-tail-calls branch from 46ea88f to 8d46594 Compare February 13, 2015 18:52

Update runtime.js

520e6fb

arv closed this Feb 13, 2015

kangax added a commit to compat-table/compat-table that referenced this pull request Feb 14, 2015

Update Traceur results according to google/traceur-compiler#1727 (com…

fd0580f

…ment)

kangax mentioned this pull request Feb 14, 2015

Add test for async generators compat-table/compat-table#432

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement proper tail calls. #1727

Implement proper tail calls. #1727

mnieper commented Feb 12, 2015

arv Feb 12, 2015

arv commented Feb 12, 2015

mnieper commented Feb 13, 2015

mnieper commented Feb 13, 2015

mnieper commented Feb 13, 2015

arv commented Feb 13, 2015

arv Feb 13, 2015

kangax commented Feb 13, 2015

arv Feb 13, 2015

mnieper Feb 13, 2015

mnieper commented Feb 13, 2015

mnieper commented Feb 13, 2015

arv commented Feb 13, 2015

arv commented Feb 13, 2015

mnieper commented Feb 13, 2015

arv commented Feb 13, 2015

mnieper commented Feb 13, 2015

kangax commented Feb 13, 2015

mnieper commented Feb 13, 2015

kangax commented Feb 14, 2015

kangax commented Feb 14, 2015

mnieper commented Feb 14, 2015

sebmck commented Feb 14, 2015

mnieper commented Feb 15, 2015

kangax commented Feb 15, 2015

sebmck commented Feb 15, 2015

mnieper commented Feb 15, 2015

sebmck commented Feb 15, 2015

mnieper commented Feb 15, 2015

arv commented Feb 16, 2015

sebmck commented Feb 16, 2015

arv commented Feb 16, 2015

sebmck commented Feb 16, 2015

mnieper commented Feb 16, 2015

		var $defineProperties = $Object.defineProperties;
		var $create = $Object.create;

Implement proper tail calls. #1727

Implement proper tail calls. #1727

Conversation

mnieper commented Feb 12, 2015

arv Feb 12, 2015

Choose a reason for hiding this comment

arv commented Feb 12, 2015

mnieper commented Feb 13, 2015

mnieper commented Feb 13, 2015

mnieper commented Feb 13, 2015

arv commented Feb 13, 2015

arv Feb 13, 2015

Choose a reason for hiding this comment

kangax commented Feb 13, 2015

arv Feb 13, 2015

Choose a reason for hiding this comment

mnieper Feb 13, 2015

Choose a reason for hiding this comment

mnieper commented Feb 13, 2015

mnieper commented Feb 13, 2015

arv commented Feb 13, 2015

arv commented Feb 13, 2015

mnieper commented Feb 13, 2015

arv commented Feb 13, 2015

mnieper commented Feb 13, 2015

kangax commented Feb 13, 2015

mnieper commented Feb 13, 2015

kangax commented Feb 14, 2015

kangax commented Feb 14, 2015

mnieper commented Feb 14, 2015

sebmck commented Feb 14, 2015

mnieper commented Feb 15, 2015

kangax commented Feb 15, 2015

sebmck commented Feb 15, 2015

mnieper commented Feb 15, 2015

sebmck commented Feb 15, 2015

mnieper commented Feb 15, 2015

arv commented Feb 16, 2015

sebmck commented Feb 16, 2015

arv commented Feb 16, 2015

sebmck commented Feb 16, 2015

mnieper commented Feb 16, 2015