[JSC] Parser: in a generator, reject yield in the parameters of an arrow function nested in another's parameters

JSTests/stress/yield-in-arrow-function-nested-in-arrow-function-parameters.js

@@ -0,0 +1,221 @@
+// The parameters of an arrow function in a generator are parsed with [+Yield]: `yield` is not an identifier in them, and a
+// YieldExpression is an early error. That holds for an arrow function in the parameters of another arrow function too.
+//
+// The parser first reads "( ... )" as an expression. To see that it is a parameter list it parses it again, in a scope
+// that is never a generator, and an arrow function in there goes to the source provider cache with `yield` as an
+// identifier. The parse that knows about the generator then skipped that arrow function from the cache, so all of the
+// code below that must throw was accepted. It was always rejected with --useSourceProviderCache=false, and the messages
+// here are the ones that run gives.
+
+function shouldBe(actual, expected, what) {
+    if (actual !== expected)
+        throw new Error(`${what}: expected ${String(expected)} but got ${String(actual)}`);
+}
+
+function shouldThrowSyntaxError(source, message) {
+    let error;
+    try {
+        (0, eval)(source);
+    } catch (e) {
+        error = e;
+    }
+    if (!(error instanceof SyntaxError))
+        throw new Error(`${source}: expected a SyntaxError but got ${String(error)}`);
+    if (message !== undefined)
+        shouldBe(error.message, message, source);
+}
+
+function shouldParse(source) {
+    try {
+        (0, eval)(source);
+    } catch (e) {
+        if (e instanceof SyntaxError)
+            throw new Error(`${source}: ${String(e)}`);
+    }
+}
+
+const parameterName = "Cannot use 'yield' as a parameter name in a generator function.";
+const yieldExpression = "Unexpected keyword 'yield'. Cannot use yield expression out of generator.";
+const shorthand = "Cannot use abbreviated destructuring syntax for keyword 'yield'.";
+const shorthandProperty = "Cannot use 'yield' as a shorthand property name in a generator function.";
+const escaped = "Unexpected escaped characters in keyword token: 'yi\\u0065ld'";
+
+// [text before the expression, text after it]. The messages below are those of the first one.
+const generators = [
+    ["(function* () { (", "); })"],
+    ["(function* () { return ", "; })"],
+    ["(function* (p = ", ") { })"],
+    ["function* g() { (", "); }"],
+    ["({ *g() { (", "); } })"],
+    ["({ *g(p = ", ") { } })"],
+    ["(class { *g() { (", "); } })", "strict"],
+    ["(class { static *g(p = ", ") { } })", "strict"],
+    ["(async function* () { (", "); })"],
+    ["(async function* (p = ", ") { })"],
+    ["(function* () { { let q; (", "); } })"],
+    ["(function* (p = function* () { (", "); }) { })"],
+];
+
+// [expression, message, true if the expression is not valid where `yield` is an identifier either]
+const invalid = [
+    // The arrow function is in the parameters of another arrow function.
+    ["(a = (yield) => 1) => a", parameterName],
+    ["(a = (b = yield) => b) => a", yieldExpression],
+    ["(a = (b = yield 1) => b) => a", "Unexpected number '1'. Expected ')' to end a compound expression.", true],
+    ["(a = (yield = 1) => 1) => a", parameterName],
+    ["(a = (...yield) => 1) => a", parameterName],
+    ["(a = ([yield]) => 1) => a", parameterName],
+    ["(a = ({ yield }) => 1) => a", shorthand],
+    ["(a = ({ b = yield }) => 1) => a", yieldExpression],
+    ["(a = async (yield) => 1) => a", parameterName],
+    ["(a = async (b = yield) => b) => a", yieldExpression],
+    ["(b = (yield) => 1, c) => b", parameterName],
+    ["(c, b = (yield) => 1) => b", parameterName],
+    ["({ a = (yield) => 1 }) => a", parameterName],
+    ["({ k: a = (b = yield) => b }) => a", yieldExpression],
+    ["([a = (yield) => 1]) => a", parameterName],
+    ["(...[a = (yield) => 1]) => a", parameterName],
+    ["async (a = (yield) => 1) => a", parameterName],
+    ["async ({ a = (b = yield) => b }) => a", yieldExpression],
+    // Other ways to use `yield` as an identifier.
+    ["(a = (b = { yield }) => b) => a", shorthandProperty],
+    ["(a = (b = ({ yield } = { })) => b) => a", shorthandProperty],
+    ["(a = (b = [yield] = []) => b) => a", yieldExpression],
+    ["(a = (yi\\u0065ld) => 1) => a", escaped],
+    ["(a = (b = yi\\u0065ld) => b) => a", escaped],
+    // Deeper.
+    ["(a = (b = (yield) => 1) => b) => a", parameterName],
+    ["(a = (b = (c = yield) => c) => b) => a", yieldExpression],
+    ["(a = (b = (c = (d = (yield) => 1) => d) => c) => b) => a", parameterName],
+    ["({ x = 1 }, a = (b = (c = yield) => c) => b) => a", yieldExpression],
+    // "( ... )" is not a parameter list, but the parser has to try that to find out. The enclosing pattern is parsed twice.
+    ["[a = (b = (yield) => 1)] = []", parameterName],
+    ["[a = ({ [(yield) => 1]: 1 })] = []", parameterName],
+    ["[a = (b = (c = yield) => c)] = []", yieldExpression],
+    // One arrow function. These were rejected before, and the messages are the same.
+    ["(yield) => 1", parameterName],
+    ["(a = yield) => a", yieldExpression],
+    ["({ a = yield }) => a", yieldExpression],
+    ["(...[a = yield]) => a", yieldExpression],
+];
+
+for (const [before, after] of generators) {
+    for (const [expression, message] of invalid)
+        shouldThrowSyntaxError(before + expression + after, before === generators[0][0] ? message : undefined);
+}
+
+// `yield` is an identifier in the body of an arrow function, in a function that is not an arrow function, and in an arrow
+// function that is in one of those.
+const valid = [
+    "(a = (b) => yield) => a",
+    "(a = (b) => { var yield; }) => a",
+    "(a = (b) => { (yield) => 1; }) => a",
+    "(a = (b) => (c = (yield) => 1) => c) => a",
+    "(a = function (yield) { yield; }) => a",
+    "(a = function (b = yield) { }) => a",
+    "(a = function () { (b = (yield) => 1) => b; }) => a",
+    "(a = function* () { yield 1; }) => a",
+    "(a = { yield: 1 }.yield) => a",
+    // `yield` as a property name is not an identifier.
+    "(a = ({ yield: b }) => b) => a",
+    "(a = ({ yield: b = 1 }, c = (d = b) => d) => c) => a",
+    "(a = (b = { yield: 1, get yield() { return 1; }, set yield(v) { }, yield() { } }) => b) => a",
+    "(a = (b = ({ yield: c } = { })) => b) => a",
+    "(a = (b = c.yield) => b) => a",
+    "(a = (b = function yield() { }) => b) => a",
+    "({ x = 1 }, a = (b = (c = 1) => c) => b) => a",
+    "[a = (b = (c = 1) => c)] = []",
+];
+
+for (const [before, after, strict] of generators) {
+    // `yield` is a reserved word in strict mode code.
+    if (strict)
+        continue;
+    for (const expression of valid)
+        shouldParse(before + expression + after);
+}
+
+// Outside a generator `yield` is an identifier in sloppy mode, at every level.
+for (const [expression, , neverValid] of invalid) {
+    if (neverValid)
+        continue;
+    shouldParse(`(function () { (${expression}); })`);
+    shouldParse(`(function* () { () => { (${expression}); }; })`);
+    shouldParse(`(function* () { function f() { (${expression}); } })`);
+    shouldThrowSyntaxError(`(function () { "use strict"; (${expression}); })`);
+}
+
+// The arrow functions do what they say. One with `yield` as an identifier in its parameters still has its item in the source
+// provider cache. Only a generator does not use it.
+{
+    const f = (a = (yield) => yield + 1) => a;
+    shouldBe(f()(41), 42, "yield as a parameter name");
+
+    function* inArrowBody() {
+        return () => (a = (yield) => yield * 2) => a;
+    }
+    shouldBe(inArrowBody().next().value()()(21), 42, "yield as a parameter name in the body of an arrow function in a generator");
+
+    const nest = (a = (yield = 1) => (b = (c = yield + 1) => c) => b) => a;
+    shouldBe(nest()()()(), 2, "yield as a parameter name and in a default value");
+
+    function yieldAsVariable() {
+        var yield = 5;
+        return ((a = (b = yield) => b * 2) => a)()();
+    }
+    shouldBe(yieldAsVariable(), 10, "yield as a variable in a default value");
+
+    // "{ x = 1 }" is not an expression, so the nested arrow functions are first parsed in the scope that is not a generator.
+    const captures = `(function* (p) {
+        let local = 10;
+        const first = yield;
+        return ({ x = 1 }, a = (b = (c = x + local + p + first) => c) => b) => a;
+    })`;
+    for (const dropCache of [false, true]) {
+        const generator = (0, eval)(captures)(100);
+        generator.next();
+        const arrow = generator.next(1000).value;
+        // A full collection drops the source provider cache. An arrow function is compiled when it is first called, and
+        // parses its text again then.
+        if (dropCache)
+            fullGC();
+        shouldBe(arrow({ })()(), 1111, "captured variables");
+        shouldBe(arrow({ x: 2 })()(), 1112, "captured variables");
+    }
+
+    // A generator still takes a valid arrow function from the cache: one with `yield` as a property name only, and one with
+    // no `yield` at all. This shows in what the default value reads. It reads N of the generator, and the body declares an N
+    // of its own. With the item the generator knows that its N is captured. When the parser has to parse the arrow function
+    // to get there, the generator does not know it (a bug of that parse), and N is not defined.
+    if (jscOptions().useSourceProviderCache) {
+        function* yieldAsPropertyName() {
+            let N = 40;
+            return ({ x = 1 }, a = ({ yield: b } = { yield: N }) => { var N = 7; return b; }) => a;
+        }
+        shouldBe(yieldAsPropertyName().next().value({ })(), 40, "yield as a property name in a pattern");
+
+        function* noYield() {
+            let N = 40;
+            return ({ x = 1 }, a = (b = N + x) => { var N = 7; return b; }) => a;
+        }
+        shouldBe(noYield().next().value({ })(), 41, "a default value that reads a variable that the body declares again");
+    }
+
+    class Base {
+        name() { return "base"; }
+    }
+    class Derived extends Base {
+        *method() {
+            return ({ x = 1 }, a = (b = (c = [this.field, arguments[0], super.name(), x]) => c) => b) => a;
+        }
+        name() { return "derived"; }
+    }
+    const derived = new Derived;
+    derived.field = "field";
+    shouldBe(JSON.stringify(derived.method("argument").next().value({ })()()), `["field","argument","base",1]`, "this, arguments and super");
+
+    function* withEval(p) {
+        return ({ x = 1 }, a = (b = (c = eval("x + p")) => c) => b) => a;
+    }
+    shouldBe(withEval(41).next().value({ })()(), 42, "eval");
+}

Source/JavaScriptCore/parser/Parser.cpp

@@ -1115,6 +1115,9 @@ template <class TreeBuilder> TreeDestructuringPattern Parser<LexerType>::createB
 
     ASSERT(name.impl()->isAtom() || name.impl()->isSymbol());
 
+    if (name == m_vm.propertyNames->yieldKeyword) [[unlikely]]
+        m_seenYieldAsIdentifier = true;
+
     switch (kind) {
     case DestructuringKind::DestructureToVariables: {
         DeclarationResultMask declarationResult = declareVariable(&name);
@@ -2613,6 +2616,16 @@ template <class TreeBuilder> bool Parser<LexerType>::parseFunctionInfo(TreeBuild
         m_seenTaggedTemplateInNonReparsingFunctionMode |= enclosingCodeContainsTaggedTemplate;
     });
 
+    // ArrowParameters[?Yield]: in the parameters of an arrow function, `yield` is an identifier only where [Yield] is off. It is always
+    // off in the scope of isArrowFunctionParameters(), and on below in a generator. So the same arrow function can be valid in one parse
+    // and an error in the next, where its item in the SourceProviderCache would skip it. The item says that the arrow function, or one in
+    // its parameters, has such a `yield`, and a generator does not use it.
+    bool enclosingCodeSawYieldAsIdentifier = std::exchange(m_seenYieldAsIdentifier, false);
+    bool arrowParametersUseYieldAsIdentifier = false;
+    auto propagateYieldAsIdentifier = makeScopeExit([&] {
+        m_seenYieldAsIdentifier = enclosingCodeSawYieldAsIdentifier || arrowParametersUseYieldAsIdentifier;
+    });
+
     auto tryLoadCachedFunction = [&] () -> bool {
         if (!Options::useSourceProviderCache()) [[unlikely]]
             return false;
@@ -2625,6 +2638,11 @@ template <class TreeBuilder> bool Parser<LexerType>::parseFunctionInfo(TreeBuild
 
         // If we know about this function already, we can use the cached info and skip the parser to the end of the function.
         if (const SourceProviderCacheItem* cachedInfo = TreeBuilder::CanUseFunctionCache ? findCachedFunctionInfo(parametersStart) : nullptr) {
+            // Not valid here. Parse the arrow function again for the error, as without the cache.
+            if (cachedInfo->arrowParametersUseYieldAsIdentifier && parentScope->isGeneratorFunction())
+                return false;
+            arrowParametersUseYieldAsIdentifier = cachedInfo->arrowParametersUseYieldAsIdentifier;
+
             // If we're in a strict context, the cached function info must say it was strict too.
             ASSERT(!strictMode() || (cachedInfo->lexicallyScopedFeatures() & StrictModeLexicallyScopedFeature));
             JSTokenLocation endLocation;
@@ -2718,6 +2736,7 @@ template <class TreeBuilder> bool Parser<LexerType>::parseFunctionInfo(TreeBuild
             parseFunctionParameters(syntaxChecker, functionInfo);
             propagateError();
         }
+        arrowParametersUseYieldAsIdentifier = m_seenYieldAsIdentifier;
 
         matchOrFail(ARROWFUNCTION, "Expected a '=>' after arrow function parameter declaration");
 
@@ -2907,6 +2926,7 @@ template <class TreeBuilder> bool Parser<LexerType>::parseFunctionInfo(TreeBuild
         parameters.expectedSuperBinding = expectedSuperBinding;
         parameters.implementationVisibility = implementationVisibility;
         parameters.containsTaggedTemplate = m_seenTaggedTemplateInNonReparsingFunctionMode;
+        parameters.arrowParametersUseYieldAsIdentifier = arrowParametersUseYieldAsIdentifier;
         if (functionBodyType == ArrowFunctionBodyExpression) {
             parameters.isBodyArrowExpression = true;
             parameters.tokenType = m_token.m_type;
@@ -4749,6 +4769,8 @@ template <class TreeBuilder> TreeProperty Parser<LexerType>::parseProperty(TreeB
 
         if (match(COMMA) || match(CLOSEBRACE)) {
             semanticFailureDueToKeywordCheckingToken(identToken, "shorthand property name");
+            if (*ident == m_vm.propertyNames->yieldKeyword) [[unlikely]]
+                m_seenYieldAsIdentifier = true;
             JSTextPosition start = tokenStartPosition();
             JSTokenLocation location(tokenLocation());
             currentScope()->useVariable(ident, m_vm.propertyNames->eval == *ident);
@@ -5346,16 +5368,21 @@ template <class TreeBuilder> TreeExpression Parser<LexerType>::parsePrimaryExpre
     case BACKQUOTE:
         return parseTemplateLiteral(context, LexerType::RawStringsBuildMode::DontBuildRawStrings);
     case YIELD:
-        if (canUseIdentifierYield()) [[likely]]
+        if (canUseIdentifierYield()) [[likely]] {
+            m_seenYieldAsIdentifier = true;
             goto identifierExpression;
+        }
         failDueToUnexpectedToken();
     case LET:
         if (!strictMode()) [[likely]]
             goto identifierExpression;
         failDueToUnexpectedToken();
     case ESCAPED_KEYWORD:
-        if (matchAllowedEscapedContextualKeyword()) [[likely]]
+        if (matchAllowedEscapedContextualKeyword()) [[likely]] {
+            if (*m_token.m_data.ident == m_vm.propertyNames->yieldKeyword)
+                m_seenYieldAsIdentifier = true;
             goto identifierExpression;
+        }
         [[fallthrough]];
     default:
         failDueToUnexpectedToken();

Source/JavaScriptCore/parser/Parser.h

@@ -2150,6 +2150,7 @@ class JSC_CACHE_LINE_ALIGNED Parser {
     bool m_seenTaggedTemplateInNonReparsingFunctionMode { false };
     bool m_seenPrivateNameUseInNonReparsingFunctionMode { false };
     bool m_seenArgumentsDotLength { false };
+    bool m_seenYieldAsIdentifier { false }; // `yield` was bound or referenced, not only a property name. See parseFunctionInfo().
     bool m_parsingBuiltin;
     bool m_isEvalContext;
     JSTextPosition m_lastTokenEndPosition; // on a later line than m_lastTokenLocation when that token is a template literal

Source/JavaScriptCore/parser/SourceProviderCacheItem.h

@@ -65,6 +65,7 @@ struct SourceProviderCacheItemCreationParameters {
     bool needsSuperBinding : 1 { false };
     bool isBodyArrowExpression : 1 { false };
     bool containsTaggedTemplate : 1 { false };
+    bool arrowParametersUseYieldAsIdentifier : 1 { false };
     // Where the lexer stood after the function's last token, which for a token spanning lines is not lastTokenLine.
     unsigned lastTokenEndLine { 0 };
     unsigned lastTokenEndLineStartOffset { 0 };
@@ -123,6 +124,8 @@ class SourceProviderCacheItem final : public TrailingArray<SourceProviderCacheIt
     unsigned implementationVisibility : 2; // ImplementationVisibility
     bool usesImportMeta : 1 { false };
     bool containsTaggedTemplate : 1 { false };
+    // An arrow function that is valid only where [Yield] is off. Parser::parseFunctionInfo() does not use the item in a generator.
+    bool arrowParametersUseYieldAsIdentifier : 1 { false };
     unsigned lastTokenEndLine;
     unsigned lastTokenEndLineStartOffset;
 
@@ -159,6 +162,7 @@ inline SourceProviderCacheItem::SourceProviderCacheItem(const SourceProviderCach
     , implementationVisibility(static_cast<unsigned>(parameters.implementationVisibility))
     , usesImportMeta(parameters.usesImportMeta)
     , containsTaggedTemplate(parameters.containsTaggedTemplate)
+    , arrowParametersUseYieldAsIdentifier(parameters.arrowParametersUseYieldAsIdentifier)
     , lastTokenEndLine(parameters.lastTokenEndLine)
     , lastTokenEndLineStartOffset(parameters.lastTokenEndLineStartOffset)
 {