[webkit-changes] [WebKit/WebKit] 2838f1: [JSC] Top-level function declarations should be le...

[Commit Queue]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 2838f145f33f94240bcf79e431598adb8625ebbb
      https://github.com/WebKit/WebKit/commit/2838f145f33f94240bcf79e431598adb8625ebbb
  Author: Alexey Shvayka <ashvayka at apple.com>
  Date:   2023-10-18 (Wed, 18 Oct 2023)

  Changed paths:
    R JSTests/modules/async-function-export.js
    M JSTests/stress/modules-syntax-error.js
    M JSTests/test262/expectations.yaml
    M Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
    M Source/JavaScriptCore/parser/Parser.cpp
    M Source/JavaScriptCore/parser/Parser.h

  Log Message:
  -----------
  [JSC] Top-level function declarations should be lexical in module code
https://bugs.webkit.org/show_bug.cgi?id=263269
<rdar://problem/117086061>

Reviewed by Ross Kirsling.

The spec has at least 8 occurrences of

> It is a Syntax Error if the LexicallyDeclaredNames of X contains any duplicate entries.

early error rules that preclude duplicate lexical declarations. For backwards-compatibility,
LexicallyDeclaredNames [1] skips top-level function declarations inside `ScriptBody : StatementList`
by invoking TopLevelLexicallyDeclaredNames [2], which returns an empty list for them [3].

However, the semantics of LexicallyDeclaredNames is entirely different for `ModuleItem`
(also please see note #1). The fact that top-level function declarations are lexical in module code
is also evident during binding initialization [4].

This change makes top-level function declarations in module code behave like `let` rather than `var`,
introducing early errors that come with it, like disallowing duplicates with `var` and `function`.

Since inside declareFunction() we can't rely neither on `m_scriptMode` (which is always `Module`,
even for nested functions that absolutely should not throw SyntaxError for duplicate declarations),
nor on `m_parseMode` (it's already the parse mode of the declared function itself), this change
introduces isModuleCode() [5], refactoring parse mode handling in Scope.

Also, this patch aligns declareFunctionAsLet() with declareVariable(), called for `let` declarations,
by adding `m_declaredVariables` check, which may fail only in module code. Removes now incorrect
(for module code only) ASSERT that isn't particularly useful given how simple declareFunction() now is.

Aligns JSC with V8 and SpiderMonkey.

[1]: https://tc39.es/ecma262/#sec-static-semantics-lexicallydeclarednames
[2]: https://tc39.es/ecma262/#sec-static-semantics-toplevellexicallydeclarednames
[3]: https://tc39.es/ecma262/#prod-HoistableDeclaration
[4]: https://tc39.es/ecma262/#sec-source-text-module-record-initialize-environment (step 24.iii)
[5]: https://tc39.es/ecma262/#sec-types-of-source-code

* JSTests/modules/async-function-export.js: Moved to JSTests/stress/modules-syntax-error.js.
* JSTests/stress/modules-syntax-error.js:
* JSTests/test262/expectations.yaml: Mark 8 tests as passing.
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* Source/JavaScriptCore/parser/Parser.cpp:
(JSC::Parser<LexerType>::Parser):
(JSC::Parser<LexerType>::parseFunctionInfo):
(JSC::Parser<LexerType>::parseMemberExpression):
* Source/JavaScriptCore/parser/Parser.h:
(JSC::Scope::setSourceParseMode):
(JSC::Scope::isGlobalCode const):
(JSC::Scope::isModuleCode const):
(JSC::Scope::declareFunctionAsLet):
(JSC::Scope::setIsGlobalCode):
(JSC::Scope::setIsModuleCode):
(JSC::Parser::declareFunction):
(JSC::Scope::setIsGlobalCodeScope): Deleted.
(JSC::Scope::isGlobalCodeScope const): Deleted.

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