Repository navigation
Conversation
…w function Between the "?" and ":" of a conditional, TypeScript `(b) : c => d` is an arrow function only when another ":" follows its body. The attempt that decides this parsed the arrow function, always restored the parser, and the caller parsed a kept body again. A chain of depth n took n * n / 2 parses of a level, and the parser frees nothing before the file is done. The attempt is now the parse. It declares the real arguments and is kept when a ":" follows the body. Only a failed attempt restores the snapshot. - `Lexer::backtrack_on_mismatch` keeps the attempt strict while the log stays on, so a kept parse reports its diagnostics. - The legal comments that wait for the next statement stay in the lexer buffer with a count of the taken ones, so a restore gives them back without a copy. - The memo records only failed attempts. - The restore scans `ref_to_ts_namespace_member` only when the attempt added a key. The shape of the helper follows `parse_possible_arrow_fn` from #44685. Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
…on attempt The type names of decorator metadata are bound as they are parsed. A name that nothing declares becomes a member of the module scope. A failed attempt left that member behind with the index of a symbol that the restore removed, so the parse after it bound `Date` to another symbol or read past the end of the symbol table. The restore now removes the module scope members and the scope uses that the attempt added.
…accesses A decorator metadata type that a sibling namespace block exports is bound to a new symbol in the `property_accesses` of that namespace. The entry stayed after a failed attempt, with the index of a removed symbol.
…hout return types
The closure for the tail was built for every parenthesized arrow function and was a call in the TypeScript parsers. The tail is now an `#[inline(always)]` method, and only the attempt builds a closure. The JavaScript copies of `parse_paren_expr` compile to the size they had. The lookahead after the second ":" is out of line, so its lexer snapshot is not in the frame of the attempt while the body is parsed.
|
Status: ready for review. Reproduced on bun 1.4.2 and on main (11c3671) with the script in the Notes of the description: CI: no failure comes from this diff. Build 134548 is red only on PR: #46311 |
|
Updated 3:12 PM PT - Oct 28th, 2026
❌ @robobun, your commit 2c81583 has 5 failures in
🧪 To try this PR locally: bunx bun-pr 46311That installs a local version of the PR into your bun-46311 --bun |
|
Update: #46270 fixed this abort on main. This PR no longer changes anything for The abort with Diff summary of the branch: the parse pass stores each decorator metadata type name, and I ran 51 decorated shapes on release builds of main (9634520), of this PR (c4484b7) and of the branch (c9ee543), as
If this PR merges first, the branch can rebase onto it and delete the code that removes the module scope members. |
The attempt that decides whether `(b) : c => d` between the `?` and `:` of a conditional is an arrow function parses the body, restores the parser, and the caller parses the body again. An attempt in the body did the same, so a chain of these arrow functions parsed the body at depth n about n times, and the arena kept every parse. An attempt that runs inside another attempt is thrown away with it. When it finds an arrow function it now skips its own restore and hands the arrow function to the caller. The outermost attempt still restores the parser, and the parse after it still parses each body for real, so each body is parsed two times. The attempt is no longer inlined into `parse_paren_expr`, which keeps the parser snapshot out of that frame.
…-conditional-arrow-attempt # Conflicts: # src/js_parser/parse/parse_skip_typescript.rs
The attempt that decides whether `(b) : c => d` between the `?` and `:` of a conditional is an arrow function parsed the arrow function, restored the parser, and the caller parsed it again. The attempt now parses the arrow function with its arguments, with the log on and with a lexer flag that makes a token mismatch end the attempt. When another `:` follows, that parse is the result and nothing is restored. A check that only moves the lexer runs first, so an attempt with no `=>` after the type takes no parser snapshot. With emitDecoratorMetadata the attempt is thrown away as before and the caller parses again: the snapshot does not undo what metadata adds. The snapshot copies the pending legal comments, and they stay in the lexer for the parse that is kept.
robobun
changed the title
a ? (b) : c => d : e attempt, restore only on failurea ? (b) : c => d : e attempt finds
…-conditional-arrow-attempt
…empt Go back to the smaller shape. The attempt for `(b) : c => d` between the `?` and `:` of a conditional is the one from main again: it parses with the log off and without arguments, and the outermost attempt always restores the parser. An attempt that starts with the log off runs inside another attempt, which throws everything away. When such an attempt finds an arrow function, it skips its restore and returns the arrow function. The outer attempt then parses each nested body one time, not one more time for every attempt around it. This takes out the parse that was kept for real: the lexer flag, the arrow tail function, the copy of the legal comments in the snapshot, and the check before the snapshot, which could log outside of it.
robobun
changed the title
a ? (b) : c => d : e attempt findsa ? (b) : c => d : e attempt inside another attempt keeps its arrow function
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Retain parsed arrows in nested attempts src/js_parser/parse/parse_skip_typescript.rs, src/js_parser/p.rs, src/js_parser/parse/mod.rs, test/bundler/transpiler/transpiler.test.js |
The TypeScript return-type check returns a parsed arrow when a nested attempt finds one, while other paths restore the parser snapshot. parse_paren_expr returns the parsed arrow when available. Tests cover nested attempts, errors, comments, follow-on parsing, expected output, and peak RSS. |
Possibly related PRs
- oven-sh/bun#45939: Both changes modify speculative parsing and parser-state restoration for TypeScript conditional arrows.
- oven-sh/bun#46209: Both changes affect TypeScript arrow-versus-expression disambiguation in
parse_paren_expr. - oven-sh/bun#47139: Both changes affect parenthesized-expression versus arrow-function decisions in
parse_paren_expr.
Suggested reviewers: jarred-sumner, alii
Priority:
Merge Risk: 
2c815
-
Nested conditional-arrow parsing avoids repeated speculative work. No material merge-blocking issue was identified.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the parser change and the nested-attempt behavior that the pull request fixes. Description check The description explains the problem, fix, tradeoffs, and verification results. It does not use the template headings exactly, but it provides the required information in equivalent sections. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![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 finding, I also traced the invariant the kept path depends on (log off on entry means an enclosing attempt will restore): every other site that sets is_log_disabled — the two lexer backtrackers in src/js_parser/parse/parse_skip_typescript.rs, next_token_matches in src/js_parser/parse/mod.rs, and look_ahead_next_token_is_open_paren_or_less_than_or_dot in src/js_parser/typescript.rs — only skips types or lexes one token and never reaches parse_paren_expr, so a kept arrow with no outer restore is not reachable. restore_parser_snapshot has a single caller, so the &mut signature change and the dropped non-restored snapshot affect no other path.
Extended reasoning...
The change touches the TypeScript parser's speculative conditional-arrow attempt in src/js_parser/parse/parse_skip_typescript.rs, its caller in src/js_parser/parse/mod.rs, the snapshot restore in src/js_parser/p.rs, and adds transpiler tests including an RSS bound; it touches no security-sensitive surface. The one posted finding is a pre-existing quadratic case the author documents; the kept-arrow path's correctness rests on the nested-detection premise, which I verified against every is_log_disabled writer in the crate.
Problem
a ? (b) : c => <body> : ecosts quadratic time and memory: 48 KB (2,560 levels) takes 4.0 s and 3,639 MB. It is a regression from parser: accept TypeScript arrow return types inside conditionals,export typebefore a newline, andimport type from#40818 (1.4.1).is_type_script_arrow_return_type_after_question_and_before_colon,src/js_parser/parse/parse_skip_typescript.rs:1601) parses the arrow function and restores the parser. The caller parses it again, so depth n takes n(n+3)/2 body parses.Fix
parse_paren_expruses the result. Each body is parsed 2 times: 69 ms and 45 MB at depth 2,560.test/bundler/transpiler/transpiler.test.js(new cost test: 224 MB on main, bound 32 MB). 180,000 generated inputs print the same as main.=>check before the body, a table of cost tests.Background
a ? (b) : c => d,(b): c => dis an arrow function only if a second:follows the body. The parser takes aParserSnapshot, parses with the lexer's log off, and restores.robobun/7ff7b2bf/keep-successful-attempt): +216/-77 source, and 15% more instructions where an attempt fails after the body.Downsides
.text+1,280 B.Notes
Repro
Release builds of main (11c3671) and this PR, linux x64. "Kept" is the chain above. "Failed" is the chain without
: e(bun chain.js 1280 ""), where no attempt finds an arrow function. "No return types" isa ? (b) => ... : e, which runs no attempt.Bun 1.3.13 takes 11 ms and 38 MB for the kept chain of depth 640. Bun 1.4.2 takes 8.9 s and 3,617 MB at depth 2,560. The time on main at depth 2,560 moved between 2.8 s and 4.0 s over my runs. The memory did not move.
What changed
:and can read the token after it (can_read_next_token, the read thatexpectmakes), returns theE::Arrowwith no restore.parse_paren_exprpops the scope and returns it. A debug assertion there checks that the log is off. The memo entry is written as before.Body parses and restores (
gdbbreakpoint counts onparse_arrow_bodyandrestore_parser_snapshot):emitDecoratorMetadatax = a ? (b) : T => d : e;x = a ? (b) : c => d;Stack
The attempt was inlined into
parse_paren_expr, so every TypeScript call of that function had the 416 BParserSnapshotin its frame. To return the arrow function from the inlined attempt made that frame larger. So the attempt is#[cold] #[inline(never)]now. Two more changes keep its own frame small:restore_parser_snapshottakes&mut ParserSnapshot(a by-value call in one branch made a second copy of the snapshot), and the read of the next token is in the non-inlinedcan_read_next_token(its lexer snapshot is 240 B).parse_paren_expr, TypeScriptparse_paren_expr, JavaScriptDeepest input that
transformSyncaccepts atulimit -s 1024. Past the limit both builds reportMaximum call stack size exceeded.a ? (b) : c => ... : ea ? (b) : c => ...a ? (b) => ... : ea ? (x) : e, nested inx((((1))))(a) => (a) => ...((a) => ((a) => ...))Cost for other input
Instructions per line (
perf_event_open,instructions:u, JIT off,scanImportsof a 50,000 line file, the same count on every run):x = a ? (b) : T => b + 1 : e;x = a ? (b) : c => c + 1;x = a ? (b) : c + 1;x = a ? (b) : T => a ? (b) : T => b + 1 : e : e;x = a ? (b) => b + 1 : e;x = (b) => b + 1;x = (b: number): number => b + 1;x = (b + 1);x = b + 1;x = (b) => b + 1;as.jsx = (b + 1);as.jssize -A:.text108,646,665 to 108,647,945 B. The JavaScript copies ofparse_paren_exprhave the same code size (3,064 and 3,050 B).P,LexerandParserSnapshotkeep their layout.perfandvalgrindare not in the container, so the instruction counts come from aperf_event_openwrapper.Same output
:,async, JSX after the:, types that log while they are skipped, classes with decorators. Loaderstsandtsx,trimUnusedImports,minifyWhitespace,scan, and 60,000 of them withemitDecoratorMetadata. 70,041 are valid. The output, the error list and the error positions are the same on both builds.The larger shape
The branch
robobun/7ff7b2bf/keep-successful-attempt(commit 28ed7ee) makes the attempt the real parse, as tsc does: a kept chain takes n body parses, and a kept site outside a chain takes 1 parse (7,896 instructions for the first line of the table above). It was the head of this PR before. It is not the head now for these reasons:expectfail with the log on, the end ofparse_paren_expras a function, and a second arm foremitDecoratorMetadata, because Hardening follow-up: fix regressions from #46021 and more crashes #46270 needs the attempt to be thrown away.a ? (b) : c => d) costs 15% more instructions (9,118 to 10,498) and keeps about 640 B more, because the attempt declares the arguments.x = a ? (b) : <out.Foo />), because its check before the snapshot logs outside of it.The shape in this PR keeps the rule that the attempt's parse is thrown away at the top level, which #40818, #40985 and #46270 rely on.
Not in this PR
restore_parser_snapshotscansref_to_ts_namespace_memberfor each restore that made a symbol. A file with many enums and many attempts with block bodies pays for that.cond ? (x) : ybefore a newline formats an error message that the restore drops. A check for=>before the body would skip it.Related open PRs
These change the same function or the lines around its call: #45743 (
async(...)before:), #45939 (the memo key), #46209 ((b as B)before:), #46553 (decorator metadata), #47133 (draft, more places that run the attempt), #45964 (Lexer::restoreand the log). This PR is output-identical and changes about 30 lines of the attempt, so it can merge before or after them. #46177 is closed.Tests
test/bundler/transpiler/transpiler.test.js, 259 pass. Withsrc/from main the cost test fails: the chain with return types takes 41.6 MB more than the chain without (bound 10 MB).