oxc-project / oxc-project/backlog

Semantic handle sloppy-mode function declaration hoisting correctly

Open
#115 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
No language data
Stars
7
Forks
0
PR merge metrics
No merged PRs in 30d

Description

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 (https://github.com/oxc-project/oxc/issues/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

https://github.com/oxc-project/oxc/issues/5627#issuecomment-2338471415
https://github.com/babel/babel/pull/14203#issuecomment-1038187168
https://github.com/overlookmotel/livepack/issues/224

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read crates/oxc_semantic/src/binder.rs around lines 166-175 and review the linked scope issue, then compare the current behavior with the Annex B examples and NodeJS results described here. Done means sloppy-mode function declarations create the correct bindings without the bogus scope ID, including the nested binding cases, while preserving correct references and avoiding scope lookup panics.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.