Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Semantic handle sloppy-mode function declaration hoisting correctly #115

Open
overlookmotel opened this issue Sep 19, 2024 · 0 comments
Open

Comments

@overlookmotel
Copy link

overlookmotel commented Sep 19, 2024

We have some pretty wacky code to handle this case:

// Sloppy mode
if (true) function f() {}

https://github.com/oxc-project/oxc/blob/f9e3a41dd2407bfadd050f3abe0d71238a083e19/crates/oxc_semantic/src/binder.rs#L166-L175

Setting a bogus scope ID ScopeId::new(u32::MAX - 1) is going to cause trouble at some point, as trying to get e.g. scope flags for this scope will panic.

It's also not correct. f in example above does create a binding, and it is observable:

// Sloppy mode
if (true) function f() {}
console.log(typeof f); // Logs 'function'

Specific case

This code:

// Sloppy mode
if (true) function f() { return f; }

console.log(typeof f); // Logs 'function'

const f2 = f;
f = 123;
console.log(typeof f2()); // Logs 'function' NOT 'number'

is equivalent to this:

'use strict';
var f;
if (true) f = function f() { return f; };

console.log(typeof f); // Logs 'function'

const f2 = f;
f = 123;
console.log(typeof f2()); // Logs 'function' NOT 'number'

i.e. There are TWO bindings for f in different scopes. f in return f refers to binding for f in function f.

We can represent this correctly as follows:

  • Create 1st binding f in outer scope.
  • Create 2nd binding f in the function declaration's scope (i.e. treat it like the id in a function expression).
  • Don't create that 2nd binding if function declaration also has either (a) a param also called f or (b) a var / let / const binding in body of function (same as function expressions behave).

Strictly speaking, we need a separate scope for the function body (oxc-project/oxc#5017) to handle this case where there are THREE bindings for f:

if (true) function f(f2 = f) { let f = 456; return [f, typeof f2]; }
const copy = f;
f = 123;
console.log([f, ...copy()]); // Logs [123, 456, 'function']
  • Binding 1 = f in top level scope
  • Binding 2 = f for function declaration's name (which f in f2 = f refers to).
  • Binding 3 = f in function body let f = 456.

General case

The above example is a specific case of sloppy mode function declaration hoisting, which we're not handling correctly in general.

The rules are a bit complex. I think this covers it:

  • Function declaration creates a binding in block which function is defined in.
  • Also creates a binding "hoisted up" to scope for body of enclosing function/top level.

The 2nd is similar to how var statements have their binding "hoisted up". Hoisted binding is not created if:

  1. The enclosing function/top level scope already has a var, let or const binding with same name.
  2. Any intermediate scope has a let or const binding with same name.
  3. Enclosing function has a param with same name.

The difference from var statements is that cases (1) and (2) are a syntax error for var statements, but they're not for hoisted sloppy function declarations. The 2nd binding just doesn't get created, silently.

How to implement this

We can likely handle this using same machinery as we use for hoisting var statements.

Do we need to implement this?

The if (true) function f() {} example above is silly - no-one would write such code. But this isn't quite so ridiculous:

if (condition) {
    function transform() { /* do something */ }
} else {
    function transform() { /* do something else */ }
}
arr = arr.map(transform);

It would not surprise me if code like this exists in some old NPM packages.

Secondly, Oxc aims to be completely spec-compliant. And this is in the spec (Annex B).

Note: My observations above of the correct behavior is not based on reading the spec, but on running snippets of code in NodeJS and seeing what they do. I am assuming that V8 is spec-compliant.

Further info

oxc-project/oxc#5627 (comment)
babel/babel#14203 (comment)
overlookmotel/livepack#224

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant