Conversation
…arrow function nested in another's parameters
In a generator the parameters of an arrow function are parsed with [+Yield]:
`yield` is not an identifier in them and a YieldExpression is an early error.
parseFunctionInfo() does that with MaybeParseAsGeneratorFunctionForScope.
isArrowFunctionParameters() parses "( ... )" in a scope that is never a
generator. An arrow function in that text is parsed there with `yield` as an
identifier, and parseFunctionInfo() adds it to the SourceProviderCache. When
the enclosing arrow function is then parsed with [+Yield], parseFunctionInfo()
finds the nested arrow function in the cache and skips it. So
`function* g() { (a = (yield) => 1) => a }` and
`function* g() { (a = (b = yield) => b) => a }` were accepted. They were
rejected with --useSourceProviderCache=false. The same happened when the text
was not a parameter list but was parsed twice for another reason:
`function* g() { [a = (b = (yield) => 1)] = []; }`.
A cache item now records that the parameters of its arrow function were parsed
with [+Yield]. In a generator, parseFunctionInfo() does not use an item without
that mark: it parses the arrow function as it does without the cache, and the
new item replaces the old one (SourceProviderCache::add() now replaces). The
result and the error message are those of a parse without the cache.
Making the scope in isArrowFunctionParameters() a generator fixes this too,
but then that pass fails on `yield`, and the parser reports the error of the
expression pass: `({ a = yield }) => a` becomes "Unexpected token '='.
Expected a ':' following the property name 'a'."
|
Reproduced with the The change is the last commit, 54004fe: The bun side is oven-sh/bun#43497 (a draft, blocked on this PR). It pins the preview build of this branch and adds a test. |
|
Preview build of 54004fe: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 5 included reviews per hour; 1 remains after this review. WalkthroughThe parser now detects invalid ChangesGenerator-aware arrow parameters
Priority: ➖ Normal 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Warning Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked the SourceProviderCache::add change from HashMap::add to HashMap::set: the cache keeps no byte-size accounting to update, and set on a unique_ptr value frees the replaced item, so the only behavioral change is last-writer-wins at a given parametersStart, which the new arrowParametersParsedAsGenerator refusal at Parser.cpp:2634 depends on. The new 1-bit field is added to both the creation-parameters struct and the item and copied in the constructor, so the two stay in sync.
Extended reasoning...
Findings are posted inline, so this note only records what else was examined. SourceProviderCache.cpp has no memory accounting (no byteSize field; the map is UncheckedKeyHashMap<int, std::unique_ptr<SourceProviderCacheItem>>), so replacing an item via set frees the old one and needs nothing else. The SourceProviderCacheItem field was added consistently in SourceProviderCacheItemCreationParameters, the item, and the constructor initializer. The reverse direction (a [+Yield] item replayed by a non-generator parse) is benign because the [+Yield] parse is strictly more restrictive and the body is parsed identically; the ruled-out list covers this and the short-function no-cache path.
…parameters stays out of the SourceProviderCache The previous commit recorded in a cache item that the parameters of its arrow function were parsed with [+Yield], and did not use an item without that mark in a generator. That rejects what it has to reject, but it also parses valid arrow functions again in a generator, and that parse is not the one that made the item: - The new item replaced the old one and did not hold the same captured variables, so the generator around it got other bytecode. - The parse ran into the `await` bug that #430 fixes. Nine valid programs of the differential corpus became SyntaxErrors, for example `(async function* () { ((...[a = () => await => 1]) => a); })`. An arrow function is valid with [Yield] off and an error with it on only when `yield` is an identifier in its parameters, which includes the parameters of an arrow function in them. So that is what the parser looks at now. canUseIdentifierYield() is only asked at a `yield` token, and notes that it said yes. parseFunctionInfo() takes that note for the parameters of an arrow function, hands it on to the arrow function around it, and does not add the arrow function to the cache. With no item, the parse that knows about the generator parses the arrow function and reports the error, as it does with --useSourceProviderCache=false. For every other function nothing changes: the same items, results, messages and bytecode. SourceProviderCache and SourceProviderCacheItem are as they were.
…with `yield` as an identifier in its parameters, and do not use it in a generator The previous commit kept such an arrow function out of the cache. That is right in a generator, where it is an error. But outside a generator it is valid sloppy mode code, and without an item the parser parses it in full each time. That parse does not give the enclosing code the same captured variables as the item does (oven-sh/bun#43569), so `function g() { var yield = 1; let N = 40; return ({ x = 1 }, a = (b = N + yield) => { var N = 7; return b; }) => a; }` threw "ReferenceError: N is not defined" from `g()({ })()`, as it does with --useSourceProviderCache=false. Now the item is made as before and records what the parser noted: `yield` is an identifier in the parameters of this arrow function, or of an arrow function in them. parseFunctionInfo() does not use such an item where the parameters are parsed with [+Yield], parses the arrow function there, and reports the error, as it does with --useSourceProviderCache=false. Everywhere else it uses the item, and hands the note on to the arrow function around it. So the parser parses again only what is an error, and a valid program keeps its items, results and bytecode.
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I checked the new flag's propagation through parseFunctionInfo() (Source/JavaScriptCore/parser/Parser.cpp:2621-2736): the scope exit restores it on every exit path including errors; the generator return false at line 2640 runs before the flag is copied from the item, so a refused item never marks the re-parse; and yield in a nested arrow's body or in a non-arrow function's parameters is dropped at that function's exit, so it never marks the enclosing item. One description/code mismatch: the description says SourceProviderCache::add() now replaces an item, but SourceProviderCache.cpp:57 is still HashMap::add, which keeps the first item, and this diff does not touch it — which is why the marked item is re-parsed on every visit as the inline finding describes.
Extended reasoning...
The third push changed the design from "keep the arrow out of the cache" to "cache it with a bit and refuse it in a generator", so the propagation of m_seenYieldAsIdentifier and the refusal path were re-read from scratch. The save/restore mirrors m_seenTaggedTemplateInNonReparsingFunctionMode; arrowParametersUseYieldAsIdentifier is captured right after parseFunctionParameters() so body uses do not leak into the item, and non-arrow functions never record it. All three canUseIdentifierYield() callers (Parser.cpp:81, 4383, 5366 and the escaped-keyword helper in Parser.h) are only reached at a yield token, so the side effect is not spuriously set. The one thing the code does not do that the description claims is replace the cache item: SourceProviderCache::add() is unchanged HashMap::add, so a marked item stays and the generator re-parses at every visit; that is the mechanism behind the confirmed inline finding rather than a separate bug, so it is noted here as a description mismatch only.
Still open from earlier reviews (1):
- Unresolved: 1 minor or pre-existing.
…7b36463) The WebKit change no longer parses valid arrow functions again. The cache item of an arrow function records that `yield` is an identifier in its parameters, and only a generator does not use such an item. The valid async generator programs that the first preview rejected parse again. The test also calls an arrow function whose default value captures a variable that its body declares again, in a generator and next to `yield` as a variable. Both go through the parser's function cache.
…renced, not in canUseIdentifierYield()
The previous commit set m_seenYieldAsIdentifier in canUseIdentifierYield().
The parser also asks that for a property name: the key of an object pattern
(`({ yield: b }) => b`, parseDestructuringPattern()) and the name of a getter
or setter. A property name is valid in a generator, so the item of a valid
arrow function got the mark, a generator did not use it, and the parser
parsed valid code again. That parse loses a captured variable
(oven-sh/bun#43569):
function* g() {
let N = 40;
return ({ x = 1 }, a = ({ yield: b } = { yield: N }) => { var N = 7; return b; }) => a;
}
g().next().value({ })() // ReferenceError: N is not defined
canUseIdentifierYield() is a plain question again. The parser sets the member
where `yield` becomes an identifier: a binding in createBindingPattern(), a
reference in parsePrimaryExpression() (the YIELD token and the escaped
keyword), and a shorthand property of an object literal in parseProperty().
The test has the property name cases, the other identifier cases (shorthand
property, shorthand in an assignment pattern, escaped keyword), and, where the
cache is on, the program above.
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline nit, I checked that the latest commit's move of the mark out of canUseIdentifierYield() (Parser.h:1972) leaves yield as a plain property key unflagged: ({ yield: b }) => b, obj.yield and { yield: 1 } no longer set m_seenYieldAsIdentifier, only createBindingPattern (Parser.cpp:1118), the YIELD/escaped primary-expression paths (Parser.cpp:5371-5383) and the shorthand property (Parser.cpp:4772) do. I also checked that the flag cannot leak from preceding siblings or from an arrow's body into its cache bit: parseFunctionInfo resets it on entry and captures it at Parser.cpp:2739 before the body is parsed, and the scope exit at Parser.cpp:2626 restores only the enclosing value ORed with that capture.
Extended reasoning...
This run's only finding is the ESCAPED_KEYWORD nit posted inline. The concrete things examined beyond it were the two questions left open by the previous review round: whether the narrowed marking still fires for yield used only as a property name (it does not, per the grep of every write to m_seenYieldAsIdentifier), and whether the save/reset/restore in parseFunctionInfo isolates the arrow's cache bit from sibling code and from its own body (it does, because the capture at line 2739 happens before the body and the scope exit discards the body's writes). The pre-existing await twin raised on an earlier version remains as it was and is not restated here.
…s a property name The engine change now marks a cache item only where `yield` is bound or referenced in the parameters of the arrow function. A property name `yield` is valid in a generator, and the parser still takes such an arrow function from its cache there. The preview before this one did not, and the new case in the last test failed with it. More cases: `yield` as a shorthand property, in an array assignment pattern and with an escape must throw in a generator. `yield` as a property name, as the name of a getter or a method, and as the name of a function expression must parse.
…k the SourceProviderCache item
parseProperty() set m_seenYieldAsIdentifier for `{ yi\u0065ld }` too. The
check before it (semanticFailureDueToKeywordCheckingToken) looks at keyword
tokens, and the escaped keyword is not one, so that shorthand is accepted in
a generator as well, with and without this change. The mark made a generator
parse the arrow function again, and that parse accepts it too. So the mark
was on code that the [+Yield] parse does not reject, which is the one thing
it must not be on: a second parse of accepted code loses a captured variable
(oven-sh/bun#43569).
Now only the `yield` token sets it there. The other places that set it are
rejected with [+Yield], escaped or not: a binding does not get past
matchSpecIdentifier(), and parsePrimaryExpression() fails for both tokens.
One more commit there: an escaped `yield` as a shorthand property does not mark the cache item of the arrow function. JavaScriptCore accepts that shorthand in a generator, so the parser must not parse the arrow function again for it.
…[+Yield] in a generator This replaces the five commits before it. They left isArrowFunctionParameters() as it was and made parseFunctionInfo() refuse a SourceProviderCache item that was made there with `yield` as an identifier. For that the parser had to know every place where `yield` becomes an identifier, and the list was wrong three times: twice it marked valid code, which then was parsed again and lost a captured variable (oven-sh/bun#43569), and it missed the name of a generator expression with an escape, `function* yi\u0065ld() {}`, which is an error only where the code around it is a generator. The cause is that the scope in isArrowFunctionParameters() is never a generator, while parseFunctionInfo() parses the same parameters with [+Yield] in a generator (MaybeParseAsGeneratorFunctionForScope). Now both do. Every check that depends on [Yield] gives the same answer in both, so a function that goes to the cache from the first one is valid for the second one. There is no new state, nothing is parsed that was valid and not parsed before, and SourceProviderCache and SourceProviderCacheItem are untouched. One thing needs care. When the parameters fail with [+Yield] and the text before them failed as an expression too, parseArrowFunctionCandidate() used to report the error of parseFunctionInfo(): "Cannot use yield expression out of generator." for `({ a = yield }) => a`. With the parameters failing earlier it would report the error of the expression: "Unexpected token '='. Expected a ':' following the property name 'a'." So when the parameters fail in a generator, isArrowFunctionParameters() parses them once more as before this change. If they are the parameters of an arrow function then, it tells the caller that the error of the parameters is the one to report. That second parse runs only for code that is an error. Two kinds of programs that were an error before get another message, because the parameters now fail earlier or later than they did: `(a = async yield => 1) => a` ("Expected ')' to end a compound expression" for "Expected a ')' or a ',' after a parameter declaration") and `(a = class { [yield]() { } }) => a` ("Cannot use yield expression within parameters" for "Cannot use yield expression out of generator").
There was a problem hiding this comment.
I reviewed the latest push, which replaces the cache-marking design with a symmetric [+Yield] speculative parse in isArrowFunctionParameters(), and found no bugs; since this is a parser change on a hot path of the engine with a no-reparse-of-valid-code invariant that only a build can confirm, a human look is still worthwhile.
What was reviewed:
- The new
parseParameterslambda and its error path: the earlyreturn falseonhasError()leaves the fake scope toAutoPopScope, as the oldpropagateError()did;swapSavePointForErrorclears the error and returns the lexer to the(before the secondnext(), andrestoreSavePointWithErrorputs the [+Yield] error and position back only when the [-Yield] parse says the text is an arrow function. - Whether a [-Yield] re-parse can leave a stale
SourceProviderCacheitem in a program that still parses: every path after a [+Yield] failure ends in a SyntaxError (flag set, second parse errored, or!lhs/isArrowFunctionTokeninparseArrowFunctionCandidate), so the item cannot be used by a successful parse; valid code is never re-parsed, which is what the earlier threads on this PR were about. - The generator decision matches
parseFunctionInfo(): both readcurrentScope()->isGeneratorFunction()before pushing the function scope, and everyyieldcheck involved (canUseIdentifierYield,parseYieldExpression, the shorthand-property checks) keys on the current scope, so the flag on the fake scope is sufficient. - The stress test: it is a new file (no
BUN-TEST-DIFFERENCES.mdentry needed), asserts the correct values on the bun#43569-sensitive cases so it stays valid if that bug is fixed, and its ~750 indirect evals plus onefullGC()look comfortably under the 200 ms budget. I could not run it here (no jsc build in this checkout).
Extended reasoning...
Overview
The net diff touches Source/JavaScriptCore/parser/Parser.cpp (+61/-15), the matching declaration in Parser.h, and adds JSTests/stress/yield-in-arrow-function-nested-in-arrow-function-parameters.js. The last commit (54004fe) is a different design from the five versions reviewed earlier: instead of marking SourceProviderCache items that saw yield as an identifier and refusing them in a generator, isArrowFunctionParameters() now performs its speculative parseFormalParameters under Scope::MaybeParseAsGeneratorFunctionForScope(fakeScope, currentScope()->isGeneratorFunction()), the same parameterization parseFunctionInfo() uses at Parser.cpp:2748. So the function cached from the speculative pass is parsed with the same [Yield] as the pass that later skips it, which removes the root cause directly. When the [+Yield] pass fails inside a generator, the parser saves that error with swapSavePointForError, re-parses with [-Yield], and if that succeeds restores the first error and sets the new isArrowFunctionWithInvalidParameters out-parameter so parseArrowFunctionCandidate reports it rather than the expression-pass error.
Security risks
None specific to this change: it makes the parser stricter (rejects programs it wrongly accepted) and does not touch memory management, JIT, or any privileged surface. The one class of risk in parser changes, a save-point/scope-stack imbalance, was checked: createSavePoint is only called when no error is pending, AutoPopScope pops the fake scope on the early-return path, and both SavePoint restores follow the existing parseDestructuringAssignment / parseArrowFunctionCandidate pattern.
Level of scrutiny
Moderate-to-high. The change is small but sits on the path every parenthesized expression takes, and a mistake would be either accepting invalid programs (the bug being fixed) or, worse for Bun users, rejecting or re-parsing valid ones. I traced the four outcomes of the generator error path (second parse errors; second parse succeeds with =>; second parse succeeds without =>; non-generator) and confirmed that each ends in a SyntaxError for the enclosing program, so a cache item created during the [-Yield] re-parse can never be consumed by a successful parse. Outside a generator the code path is unchanged. This addresses the substance of the earlier inline threads (valid code re-parsed and losing captures, escaped shorthand, property-key false positives), which were concerns specific to the abandoned design; the await twin remains a separate, pre-existing issue and is not made worse because the fake scope's async-ness is inherited exactly as before. I could not build jsc or run the new test in this environment (no build tree; running bun/node was not permitted), so the verification claims in the description (corpus agreement with node, exact error messages) are unconfirmed here, which is the main reason to keep a human in the loop rather than approve outright.
Other factors
The new stress test is thorough: 12 generator contexts times 39 invalid expressions with exact message checks in the first context, 17 valid expressions across the sloppy contexts, a sweep confirming yield stays an identifier outside generators, and runtime checks for captured variables, this/arguments/super, eval, and the cache-on/cache-off paths via fullGC(). It is a new file, so JSTests/BUN-TEST-DIFFERENCES.md does not need an entry, and the assertions at lines 194-206 test the correct values rather than the bug they are sensitive to. The remaining open item in the timeline is the pre-existing await twin, which the author resolved themselves; that is not evidence it was addressed, but it is also not a regression introduced by this PR and was already raised, so it does not need restating.
…of a generator expression
The engine change is now in isArrowFunctionParameters(): it parses the
parameters with [+Yield] in a generator, as the parse that follows it does.
The cache item marks of the earlier previews are gone.
New case: `function* yi\u0065ld() {}` in the parameters of an arrow function
in arrow function parameters in a generator. It was an error only without the
parser's function cache, also with the preview before this one.
1 participant
Problem
yieldin arrow function parameters is a SyntaxError. JavaScriptCore acceptsfunction* g() { (a = (yield) => 1) => a }, where that arrow function is in another one's parameters. With--useSourceProviderCache=falseit throwsCannot use 'yield' as a parameter name in a generator function.V8 rejects it.isArrowFunctionParameters()(parser/Parser.cpp:361) parses( ... )in a scope that is never a generator. The nested arrow function parses there withyieldas an identifier and goes to theSourceProviderCache. The parse that knows the generator finds that item (Parser.cpp:2627) and skips it.Fix
isArrowFunctionParameters()now parses the parameters with [+Yield] in a generator, with the sameMaybeParseAsGeneratorFunctionForScopethatparseFunctionInfo()uses for them.Cannot use yield expression out of generator.for({ a = yield }) => a.JSTests/stress/yield-in-arrow-function-nested-in-arrow-function-parameters.js. Of 1,409,400 generatedyieldprograms, the old parser accepts 169,390 that node rejects. Now they agree on all.Background
ArrowParameters[?Yield]: arrow function parameters take the [Yield] of the code around them. The body does not.( ... ). It parses the text as an expression, then as parameters inisArrowFunctionParameters(), then inparseFunctionInfo().SourceProviderCachemaps a function start offset to what lets the parser skip that function. It does not record the context.Notes
Repro (
jsc, orbunwith plaineval):jscofebd5a6145baccepts the first four.jsc --useSourceProviderCache=falseand node 26 reject all five.Why the third pass did not catch it. In
parseFunctionInfo()of the outer arrow function the scope is a generator. At the nested arrow function the expression pass fails onyield,isArrowFunctionParameters()(own scope, not a generator) says yes, andparseFunctionInfo()of the nested arrow function finds the item that the outerisArrowFunctionParameters()pass made.Not only nested parameters. The text does not have to be a parameter list. It is enough that the parser tries it as one and parses the text once more after that.
[a = (b = (yield) => 1)] = []in a generator:(b = ...)is tried as parameters, which caches(yield) => 1, then the array is parsed again as an assignment pattern. The test covers this.History of this PR. Self-reviewed: 1 concern raised (fix the place that makes the wrong cache item, do not refuse the item), 1 addressed. The last commit is the change. The five commits before it are an approach that this PR no longer uses: they left
isArrowFunctionParameters()alone and madeparseFunctionInfo()refuse a cache item whose arrow function hasyieldas an identifier in its parameters. For that the parser has to know every place whereyieldbecomes an identifier, and the list was wrong three times. Twice it marked valid code (({ yield: b }) => b, the escaped shorthand{ yi\u0065ld }), which was then parsed again, and a second parse of valid code is not safe today: it loses a captured variable (oven-sh/bun#43569) and runs into #430. Once it missed a case: the name of a generator expression with an escape,function* yi\u0065ld() {}, is an error only where the code around it is a generator. With the scope fixed, no list is needed. I did not start with this version because without the second parse of the parameters it makes the messages worse.Messages. For the 1,409,400 programs of the
yieldcorpus, the result and the message are those of the old parser without the cache. In the wider corpus, 1,226 programs that were an error before get another message, because the parameters now fail earlier or later than they did. Two kinds:(a = async yield => 1) => a(Expected ')' to end a compound expressionforExpected a ')' or a ',' after a parameter declaration), andyieldin a computed key of a class in the parameters,(a = class { [yield]() { } }) => a(Cannot use yield expression within parametersforCannot use yield expression out of generator). No existing test has these messages.Cost. Nothing for valid code: the scope gets a flag that
parseFunctionInfo()sets too. The second parse of the parameters runs only when they failed in a generator, which is code that is an error.The
yieldcorpus. 17 expression templates with a hole, nested to depth 3, 27 leaves, 10 contexts (generator bodies and parameters, an async generator, a method,forheads, an arrow function body in a generator, a plain function): 1,409,400 programs. The leaves useyieldin and around arrow function parameters: as a parameter name, in default values, in patterns, as a shorthand property, with an escape, and where it is valid in a generator (property names,a.yield, a function expression namedyield, the body of a nested function).jscwithout the change accepts 169,390 of them with the cache and rejects them without it. With the change, with and without the cache, each program gives the result and the message of the old parser without the cache. node 26 accepts and rejects the same programs (1,080,050 accepted, 329,350 rejected).Three runtime corpora of 142,740 valid programs each: nests of arrow functions to depth 3 with
this,arguments,new.target,eval, captured variables, and default values that capture a variable that the body declares again. In the first one the nests are returned from a generator. In the second one they are in plain functions and useyieldas a parameter name and as a variable in default values. In the third one they are returned from a generator and useyieldas a property name and as an escaped shorthand property. Each level is called, so each arrow function is compiled, which parses it again. With and without the change the results are the same. For the nests to depth 2 of each corpus,--dumpGeneratedBytecodesgives the same bytecode with and without the change, addresses aside (113 MB, 88 MB and 122 MB).A wider corpus. 30 expression templates with a hole (parameter default values, patterns, bodies, functions, classes, destructuring assignments), nested to depth 2, 61 leaves (
yield,await,arguments,new.target,super,let,staticin arrow function parameters and bodies, and plain errors), 26 contexts (script, functions, generators, async functions, methods, parameters, strict mode): 1,452,360 programs.yieldin it, and node 26 rejects all of them. No program that node accepts is rejected, and no program changes the other way.await, and this change does not touch them: 1,635 are the Parser: allow await as an identifier in a non-async function nested in an async function's parameters #430 case (valid, rejected without the cache). 30 are theawaittwin of this bug, for example(async function () { [a = (b = (await) => 1)] = []; }): accepted with the cache, rejected without it and by node (JavaScriptCore acceptsawaitas an arrow function parameter name in an async function when the parser's function cache holds the arrow function bun#43476, [JSC] Parser: isArrowFunctionParameters() parses the parameters with [+Await] whereawaitis not an identifier #703). The same change in the same place can fix it: parse the parameters inisArrowFunctionParameters()with theallowAwaitthatparseFunctionInfo()uses.Tests run. The local build is Release with assertions. The new test passes in all 17 modes of
run-javascriptcore-tests, includingno-cjit-validate-phases, which runs without the cache, andbytecode-cache. It fails with each of the five earlier commits or with the old parser. test262: every test undertest/languageandtest/annexB/language, in the scenarios thattest262-runneruses (strict, default, module, raw), compared by exit code and output between the two builds, with and without the cache: no difference in 45,388 runs.run-javascriptcore-testswith--filter 'arrow|async|await|yield|generator|destructur|default-param|syntax|parser|class-field|static-block|eval|function|es6|reparse|cache': about 19,000 runs, one failure,stress/bigint-inc-dec-in-place.js, which exceeds the memory limit of the test in every mode in the same build without the change.With #699. That change skips the expression pass at a known arrow function start, and still runs
isArrowFunctionParameters()andparseFunctionInfo()there. Both PRs changeparseArrowFunctionCandidate()near the same lines, so the second one to land needs a rebase.