[webkit-changes] [WebKit/WebKit] d0d057: [JSC] Align duplicate declaration checks in EvalDe...

[Commit Queue]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: d0d05739899c9a8d76d166bbc1afbd116fa5eaf8
      https://github.com/WebKit/WebKit/commit/d0d05739899c9a8d76d166bbc1afbd116fa5eaf8
  Author: Alexey Shvayka <ashvayka at apple.com>
  Date:   2023-06-29 (Thu, 29 Jun 2023)

  Changed paths:
    M JSTests/ChakraCore/test/Closures/bug_OS_2299723.baseline-jsc
    M JSTests/stress/const-not-strict-mode.js
    M JSTests/stress/eval-func-decl-in-eval-within-catch-scope.js
    M JSTests/stress/eval-func-decl-in-global-of-eval.js
    M JSTests/stress/eval-func-decl-within-eval-with-reassign-to-var.js
    A JSTests/stress/eval-let-const-redeclararion.js
    M JSTests/stress/global-lexical-var-injection.js
    M JSTests/stress/lexical-let-not-strict-mode.js
    M JSTests/test262/expectations.yaml
    M Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
    M Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
    M Source/JavaScriptCore/interpreter/Interpreter.cpp
    M Source/JavaScriptCore/runtime/JSScope.cpp

  Log Message:
  -----------
  [JSC] Align duplicate declaration checks in EvalDeclarationInstantiation with the spec
https://bugs.webkit.org/show_bug.cgi?id=167837
<rdar://problem/111328974>

Reviewed by Yusuke Suzuki.

For the sloppy-mode eval(), this change:

1. Removes slowish TypeError-throwing logic from executeEval() that also wasn't spec-compliant
   (SyntaxError should be raised instead), harmonizing error messages.

2. Expands resolveScopeForHoistingFuncDeclInEval() to be called for all declared variables, which
   currently includes function declarations as well, ensuring SyntaxError is thrown for duplicates
   with upper yet non-top lexical scopes [1], all while skipping CatchScopeWithSimpleParameter [2].

3. Introduces emitPutToScopeDynamic(), which circumvents default ResolveType resolution that isn't
   correct wrt skipping CatchScopeWithSimpleParameter as resolveScopeForHoistingFuncDeclInEval() does.

   We can't possibly tweak BytecodeGenerator::resolveType() to account for eval().

   This fixes both top-level and block-level function declarations to be hoisted correctly from eval()
   within simple parameter catch block by the same name.

4. Removes isExtensible() check from resolveScopeForHoistingFuncDeclInEval() because for declared
   variables, CanDeclareGlobalVar [3] is already implemented, while for Annex B hoisted functions,
   the implementation doesn't appear correct to unconditionally rely on isExtensible() even if the
   property is already present.

   Furthermore, performing CanDeclareGlobalVar in resolveScopeForHoistingFuncDeclInEval() is kinda
   superfluous given we put jsUndefined() variables in executeEval(), and results in incorrect
   error being thrown (SyntaxError instead of TypeError) if global object is non-extensible.

[1]: https://tc39.es/ecma262/#sec-evaldeclarationinstantiation (step 3.d.i.2.a.i)
[2]: https://tc39.es/ecma262/#sec-variablestatements-in-catch-blocks
[3]: https://tc39.es/ecma262/#sec-candeclareglobalvar

All JSTests changes were proven to align JSC with V8 and SpiderMonkey.

* JSTests/ChakraCore/test/Closures/bug_OS_2299723.baseline-jsc:
* JSTests/stress/const-not-strict-mode.js:
* JSTests/stress/eval-func-decl-in-eval-within-catch-scope.js:
* JSTests/stress/eval-func-decl-in-global-of-eval.js:
* JSTests/stress/eval-func-decl-within-eval-with-reassign-to-var.js:
* JSTests/stress/eval-let-const-redeclararion.js: Added.
* JSTests/stress/global-lexical-var-injection.js:
* JSTests/stress/lexical-let-not-strict-mode.js:
* JSTests/test262/expectations.yaml: Mark 160 tests as passing.
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::generate):
(JSC::BytecodeGenerator::hoistSloppyModeFunctionIfNecessary):
(JSC::BytecodeGenerator::emitPutToScopeDynamic):
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:
* Source/JavaScriptCore/interpreter/Interpreter.cpp:
(JSC::Interpreter::executeEval):
* Source/JavaScriptCore/runtime/JSScope.cpp:
(JSC::JSScope::resolveScopeForHoistingFuncDeclInEval):

Co-authored-by: Oleksandr Skachkov <gskachkov at gmail.com>
Canonical link: https://commits.webkit.org/265614@main