Inline hoisted, post-return declarations properly

[kangax]

Fixes #192

[kangax]

This is not ideal, of course. But we could revisit later.

[boopathi]

It's actually correct. This is not removed because it's a global.

[boopathi]

Created #392

[kangax]

Considering that DCE has a lot of cases of path.parentPath.getSibling(path.parentPath.key - 1) and path.parentPath.getSibling(path.parentPath.key + 1), I think it makes sense to add helper traversal methods to Babel.

[kangax]

Filed babel/babel#5223

[boopathi]

this will fail for cases like -

function foo() {
  bar = x;
  var x = 1;
}

[kangax]

You're right. But then we have a much "deeper" problem:

function foo() {
  (function() {
    alert(function(){ return x }());
  })();
  var x = 1;
}

This brings us again to CFG and knowing when an identifier is referenced before declaration (having to traverse entire tree before declaration).

I think in this case it would be simpler to just hoist vars before doing DCE. So this:

function foo() {
  bar = x;
  var x = 1;
}

would first become:

function foo() {
  var x;
  bar = x;
  x = 1;
}

which could then be transformed to:

function foo() {
  bar = undefined;
}

[boopathi]

Case 1 can be solved without a CFG ... siblingPath.isAncestor(referencedPath) will tell you this.

function foo() {
  (() => {
    return x // x - ReferencedPath
  })() // CallExpression SiblingPath
  var x = 1;
}

[kangax]

Right, for each binding reference we can traverse a subtree upwards and check if parent is a previous sibling. We also need to check that identifier is not already defined:

function foo() {
  ((x) => {
    return x // x - ReferencedPath but is a local var
  })(2) // CallExpression SiblingPath
  var x = 1;
}

[boopathi]

Yes! But, this case we are dealing with is "one-time-used-constant" - binding.references === 1 and binding.constant, and we already have the path to the reference (binding.referencePaths[0]). So the local variable case wouldn't be this referencePath.

[boopathi]

void 0 is safe. We can't assume that there is no local variable called undefined

[kangax]

Good point, thanks

[boopathi]

a few lines below this, replacementPath = replacementPath.get("init") - we might have to fix this here too. Else replacement and replacementPath might point to different things.

[kangax]

Ok, this was easier than I thought. And your case worked right away. What should we replace init path with?

[boopathi]

Just found one more case - I think we should deopt if the reference is in a different scope - instead of replacing it with undefined.

function foo() {
  Promise.resolve().then(() => console.log(x));
  var x = 1;
}

will log 1 but we will optimize it to undefined

[kangax]

Good point. We don't know if other scopes are executed (a)synchronously. Deopted.