Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: ab0917349c9ff587331ce34d5ffc17f0fb471884
      
https://github.com/WebKit/WebKit/commit/ab0917349c9ff587331ce34d5ffc17f0fb471884
  Author: Alexey Shvayka <[email protected]>
  Date:   2023-07-22 (Sat, 22 Jul 2023)

  Changed paths:
    M JSTests/ChakraCore/test/Closures/bug_OS_2299723.baseline-jsc
    M JSTests/stress/const-not-strict-mode.js
    A JSTests/stress/eval-func-decl-by-the-same-name-as-callee.js
    M JSTests/stress/eval-func-decl-in-eval-within-catch-scope.js
    M JSTests/stress/eval-func-decl-in-global-of-eval.js
    A JSTests/stress/eval-func-decl-within-eval-duplicate-declaration.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/parser/Parser.h
    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.

This change is a re-land of 265614@main with getSloppyModeHoistedFunctions() 
fixed not to set
IsSloppyModeHoistingCandidate bit for `var` declarations. This ensures that for 
a `var` binding,
which shadows a lexical declaration from an outer scope, a SyntaxError is 
raised even if
there is Annex B hoisted function by the same identifer.

---

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-by-the-same-name-as-callee.js: Added.
* 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-duplicate-declaration.js: Added.
* 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/parser/Parser.h:
(JSC::Scope::getSloppyModeHoistedFunctions):
* Source/JavaScriptCore/runtime/JSScope.cpp:
(JSC::JSScope::resolveScopeForHoistingFuncDeclInEval):

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


_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to