[webkit-changes] [WebKit/WebKit] c0dd36: [JSC] Rest parameter should be evaluated before Va...

[Commit Queue]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: c0dd368287d55af9c01a3ac187167581e95e5c5b
      https://github.com/WebKit/WebKit/commit/c0dd368287d55af9c01a3ac187167581e95e5c5b
  Author: Alexey Shvayka <ashvayka at apple.com>
  Date:   2024-02-05 (Mon, 05 Feb 2024)

  Changed paths:
    A JSTests/stress/regress-268274.js
    M JSTests/test262/expectations.yaml
    M Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

  Log Message:
  -----------
  [JSC] Rest parameter should be evaluated before VariableEnvironment is set
https://bugs.webkit.org/show_bug.cgi?id=268274
<rdar://problem/121961421>

Reviewed by Mark Lam.

If a function has non-simple parameter list, a separate Environment Record is created (step 28.c of [1])
to ensure that a closure, created in default value expression, don't have visibility of declarations
in the function body. This is also the environment where direct eval() declare variables.

Moreover, if there is a `var` by the same name as parameter, it should start out with the value of
that parameter (step 28.c of [1]), but have a distinct binding.

All parameters, including the ...rest one, are evaluated during steps 25-26 of [1].
This change moves rest parameter evaluation to come before VariableEnvironment is created & set,
which matches the steps order in the spec [1] and aligns JSC with V8 and SpiderMonkey.

>From userland perspective, this patch fixes a handful of bugs:
  * direct eval() in default value expression inside rest parameter creates variable in environment
    of the function rather than the separate one of the parameters;
  * ReferenceError is thrown when accessing a binding, which is defined inside rest parameter,
    in eval() / closure created in default value expression of a preceding parameter, but only
    if there is a `var` binding by the same name;
  * a closure, created in default value expression inside rest parameter, is created in different
    VariableEnvironment (of the function) than its counterparts in preceding parameters, which causes
    incorrect environment to be consulted when querying / modifying parameter names that are
    "shadowed" by `var` bindings.

With this change, all parameters are evaluated inside a single try / catch wrapper, which reduces
the number of bytecodes, emitted for an async function with rest parameter, by 2.

[1]: https://tc39.es/ecma262/#sec-functiondeclarationinstantiation

* JSTests/stress/regress-268274.js: Added.
* JSTests/test262/expectations.yaml: Mark 7 tests as passing.
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::generate):
(JSC::BytecodeGenerator::initializeDefaultParameterValuesAndSetupFunctionScopeStack):

Canonical link: https://commits.webkit.org/274109@main