You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
AOT: module instances in test runs, code without types, generators, BigInt, smaller builds - #1036
The JavaScriptCore side of a round of work on Bun's ahead-of-time compiler (bun build --compile --aot and bun test --aot, both experimental). Bun's side is a pull request in oven-sh/bun that pins this one's preview. This follows #1028 and #1030 and lies on the upgrade of #1015 and on #1032.
Merge order
This merges first; then Bun's pull request pins the merged commit and runs CI once more.
What's in it
1,035 files, +90,379 −9,452 against main.
Area
Files
+
−
aot/walk: the main pass
124
29,941
3,101
aot/runtime
96
14,808
1,370
aot/tests
230
12,082
755
aot/codegen, stubs, machine: code generation
246
14,564
2,109
aot/image: the image
56
2,158
389
The rest of aot/
230
16,066
1,574
Outside aot/
53
760
154
Outside aot/
Behind ENABLE(AOT) or in code that compiled programs alone reach, but for the rows that say otherwise.
File
+
−
Change
assembler/LinkBuffer.cpp, assembler/MacroAssemblerARM64.h and 4 more
167
21
Shorter jumps: a jump to a short return becomes that return. Compiled code alone
assembler/testmasm.cpp
96
0
Tests
b3/B3Generate.cpp
2
1
Frequencies that only code for an image reads
bytecode/RepatchInlines.h
9
3
An iteration of what is not iterable says so in the DFG and the FTL too. Plain Bun too
dfg/DFGSafeToExecute.h
9
1
Which nodes may move. Plain Bun too
dfg/DFGFixupPhase.cpp, dfg/DFGClobberize.h and 4 more
20
9
Values that join a double, constants of loads, arguments of tail calls, and nodes that take over uses, as the interpreter has them. Plain Bun too
Wrong answers and stopped builds that main has, fixed
Program
On main
typeof x === "object" of a built-in object (JSON, console, process) that comes out of a Map, a WeakMap or a list
Silent wrong answer at default flags
A loop over a generator whose consumer is handed an iterator of a Map
The build could stop
A closure over a parameter that is stored in a variable and called, with four or more subclasses handed to it
The build stopped
A getter that is laid over a method of a class in a closure that calls the method by a key
The build stopped
A literal with a getter and a setter in nested loops, between calls whose callees the loops choose
The build stopped
A loop that steps by hand what a replaced method gives
The build stopped with --aot-harden=proof-checks
A test of a list's length against its length before a push() or pop()
As above
A closure with two captured variables that is copied in a loop and called by value beside it
As above
row[i] += factor * b[i] where factor is undefined in one call
As above
A method that is replaced by that of another class and that nobody enters
As above
A getter that switches on a variable of its file that holds a constant
As above
A method of a record in a list that adds strings to its parameters in loops
As above
new Set(x) where x is a Map or a string
The program stopped
The effects of a body that nobody enters count as those of unknown code now, so a build can also assume less at default flags. stress/new-largeish-contiguous-array-with-size.js: the bound of the test holds for the heap growth fractions of builds without the memory footprint API; the test names them, as the review asked.
Code without types and code of libraries
Objects: fields that are created late, under a key or on a prototype that is assigned later keep their slots; numbers in fields stay unboxed; an object that does not leave its loop is not allocated.
Keys: reads and stores under a key that is computed, joined, an element or an integer; in, delete, for-in, spreads and rest objects.
Calls: callees that are chosen by a test, stored in a field, a list or a table, bound, or handed to a callback; functions that call themselves run as loops; callbacks of array methods run in one loop.
Loops: lists and typed arrays whose kind is not proven, strings beside numbers, integers past 2 ** 32, lists of lists.
BigInt: short values are computed in place, and products that only a remainder reads are not allocated.
Regular expressions
A match of a written pattern in a loop is compiled where it stands, and the statics of RegExp are kept only where something may read them.
Generators, async code and iteration
A generator is stepped inside its consumer, also behind yield*, destructuring, Array.from(), spread and using. Awaited calls, returned promises and suspended frames take fewer steps.
Test runs
The original of a mocked module is compiled code too. Mocks that end while a graph loads, CommonJS files under a mock, spies, resets.
Build
Less time and memory: facts are computed once, records are packed, sets of values grow in segments, large files are compiled in parts.
Checks
A fuzzer of the machine layer that runs what it generates, and injected wrong facts (--aot-check) that the checks of a build must refuse.
Known issues
None of the engine alone that main does not have. What differs from bun run and bun test is listed in Bun's pull request.
Tests
IR tests and native tests are in aot/tests; stored programs are in Bun's pull request. CI's run of this pull request is the first of this tree.
…nder proof-checks
On 7f020d2. 14 files, +198 -14: Source/JavaScriptCore/aot 13 files +197 -13 . JSTests 1 file +1 -1.
`typeof x === "object"` was false, and `typeof x === "function"` true, for a built-in object such as
JSON, console or process that comes out of a Map, a WeakMap or a list, where the test stands in
front of a branch. The test asked the container for the part of the type that passes; an object that
may be a function was counted into it. Fixed.
A build could stop in a loop over a generator whose consumer is walked in place and is handed an
iterator of a Map. While the generator's yield walks that consumer, its frame lies above the
generator's, and a question about its caller looked the call up in the generator's body, past its
end where that is the shorter one. Fixed.
With `--aot-harden=proof-checks` six kinds of program stopped a build that default flags build
right:
- A loop that steps by hand what a replaced method gives: a guess guarded a callee that is bound to
nothing.
- A test of a list's length against its length before a push() or pop(): the verifier asked the
older length, which is a number in that relation, for a write behind it.
- A closure with two captured variables that is copied in a loop and called by value beside it: the
check of entry parameters asked a capture that the body reads from its variable.
- `row[i] += factor * b[i]` where factor is undefined in one call: an element that is a raw number
already was tested for a number again.
- A method that is replaced by that of another class and that nobody enters: no store of its body
was counted. The effects of such a body are those of unknown code.
- A getter that switches on a variable of its file that holds a constant: the check walks the getter
again without folding and entered cases that the scan never enters.
stress/new-largeish-contiguous-array-with-size.js: the test's bound holds for the heap growth
fractions of builds without the memory footprint API; the test names them.
…at sums of strings
On 902c306. 5 files, +13 -4: Source/JavaScriptCore/aot 5 files +13 -4.
A build could stop at default flags where a closure over a parameter is stored in a variable that is
called and four or more subclasses are handed to it: Link's flow ended while members were still to
be resolved. Fixed.
A build could stop at default flags where a getter is laid over a method of a class in a closure
that calls the method by a key. Fixed.
With `--aot-harden=proof-checks`, a method of a record in a list that adds strings to its parameters
in loops stopped a build: an addition that is built as a join of strings was asked where Link counts
what callers pass. Fixed.
We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
No actionable comments were generated in the recent review.
Walkthrough
The PR updates AOT link processing, array-access verification, control-flow selection, and walker checks. It also changes the RAM fractions in a JavaScript stress-test directive.
Changes
AOT analysis and walking
Layer / File(s)
Summary
Link-call work scheduling Source/JavaScriptCore/aot/facts/AOTLinkCalls.cpp, Source/JavaScriptCore/aot/facts/AOTLinkFlow.cpp, Source/JavaScriptCore/aot/facts/AOTLinkFlow.h
LinkFlow exposes whether members await resolution and starts an observer-processing round when the resolution queue is nonempty.
The verifier continues ground checks for swept accesses and applies test-side checks based on whether the test is an array-presence test. New fixtures cover accesses using earlier length values across storage conversion.
When folding is disabled, the walker can select a branch or switch target from eligible constant-written values and convert the selected terminator to a jump.
The walker adds type checks for branch threading and generator steps, skips removed entry parameters in call judgments, and updates frame, loop-guard, and walked-callee handling. An answer fixture checks function-type results for values selected from a mixed list.
The test directive adds small- and medium-heap RAM fractions of 0.25 and 0.5.
Possibly related PRs
oven-sh/WebKit#979: This PR changes AOT pass verification, while this PR also changes CodeVerifier::verifyTestsPassed and adds verifier fixtures.
Merge Risk: Minimal · previous review f9c80
The changes affect only the experimental AOT mode and are described as leaving behavior unchanged without it. No concrete merge-blocking risk has been identified.
Pre-merge checks: 3 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Description check
The description covers the main required parts of the WebKit template. It names the related bug and PR context, gives a merge order, and lists the changed areas with file-level detail. It also lists t…
Add a Bugzilla bug title and URL in the form https://bugs.webkit.org/show_bug.cgi?id=#####. Add the 'Reviewed by' line, or state that review is pending. Replace or supplement the tables with a short explanation of why each fix works. Add th…
Passed checks: 3 passed
Check name
Status
Explanation
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.
Title check
The title names several real change areas, including generators, test-run behavior, code without types, and smaller builds. It is broad and list-like, but it remains related to the pull request.
Full details: Description check
Explanation
The description covers the main required parts of the WebKit template. It names the related bug and PR context, gives a merge order, and lists the changed areas with file-level detail. It also lists the behavior fixes and the tests location. It does not follow the template structure. It has no Bugzilla bug link in the WebKit format (https://bugs.webkit.org/show_bug.cgi?id=#####), no 'Reviewed by' line, and no per-file changelog entries in the ChangeLog-style format (path, function names). The description also says the work is linked to GitHub PRs rather than a WebKit Bugzilla bug. The explanation of the fix is spread across tables and lists, which makes it hard to map each change to a bug. Overall the description is mostly complete in content but misses required template elements.
Resolution
Add a Bugzilla bug title and URL in the form https://bugs.webkit.org/show_bug.cgi?id=#####. Add the 'Reviewed by' line, or state that review is pending. Replace or supplement the tables with a short explanation of why each fix works. Add the per-file entries in the template format (path, then function names). Keep the GitHub PR references as context, if needed. If the PR is built with git-webkit, use its generated commit message as the description.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 path_filters to narrow the review scope.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: f29284d8-8522-488d-a5a9-bb798f4c8726
Commits
Reviewing files that changed from the base of the PR and between 7f020d2 and f9c8026.
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
…igInt, smaller builds
On f9c8026. 1,015 files, +91,001 -9,622: JSTests 2 files +4 -4 . Source 1,013 files +90,997 -9,618.
`bun test --aot`
- Each load of an ES module in a test run has its own module instance. `bun test` loads a module
again when a mock on it, or on something that it imports, begins or ends, and under --rerun-each;
the code of an earlier load goes on with the modules that this load was linked to.
- Originals of mocked modules are compiled. A CommonJS file that a mock factory takes by import() or
require() is one instance with what vi.importActual() gives. A load that failed under a mock is
tried again. Mocks that end while a graph loads, resets and spies answer as in `bun test`.
Code without types and code of libraries
- Fields that are created late, under a key or on a prototype that is assigned later keep their
slots; numbers in fields stay unboxed; an object that does not leave its loop is not allocated.
- Keys that are computed, joined, elements or integers; `in`, `delete`, `for-in`, object spreads,
rest objects, `Object.entries()`, own keys.
- Callees that are chosen by a test, stored in a field, a list or a table, bound, or handed to a
callback. Functions that call themselves run as loops, and so do chains of callbacks of array
methods.
- Lists and typed arrays whose kind is not proven, strings beside numbers, integers past 2 ** 32,
lists of lists.
- BigInt: short values are computed in place, and a product that only a remainder reads is not
allocated.
- A match of a written pattern in a loop is compiled where it stands. The statics of RegExp are kept
only where something may read them.
- A generator is stepped inside its consumer, also behind `yield*`, destructuring, `Array.from()`,
spread and `using`. Awaited calls, returned promises and suspended frames take fewer steps.
Size and build
- Rare blocks take closed forms, linked numbers are patch constants, resumed frames keep no dead
words. Regular expressions and strings of an image are decoded in fewer steps.
- Facts are computed once, records are packed, sets of values grow in segments, large files are
compiled in parts.
Plain Bun too
- require() of a module whose load an import() has begun no longer throws.
- One node fewer counts as safe to move without its checks in the DFG.
- A store of a name that an array or a function lacks compares the receiver before it asks for
immutable properties. SlowPutArrayStorageShape is 0x0E, so one bit tells the arrays with boxed
elements.
Tests and names
- A fuzzer of the machine layer that runs what it generates. Plainer names in the main pass, the
facts, the stubs and the tests.
- The review note on stress/new-largeish-contiguous-array-with-size.js.
`Wire::schemaHash` changes, so this engine and Bun's side go together.
Jarred-Sumner
changed the title
AOT: typeof of built-in objects out of containers, builds that stop under proof-checks
AOT: module instances in test runs, code without types, generators, BigInt, smaller builds
Nov 11, 2026
The reason will be displayed to describe this comment to others. Learn more.
The one earlier inline note (the // Bun: comment and BUN-TEST-DIFFERENCES.md entry for the JSTests edit) is addressed in 9fd6899; this automated review of the full range found no bugs, but at ~1000 files and ~90k added lines, with changes to shared engine code outside aot/, a human look is still worthwhile.
What was reviewed:
Shared runtime outside aot/: MicrotaskQueue GC marking (new m_mustMarkFromFront and taskWillNotRun barrier on dequeue/clear paths), JSModuleLoader synchronous-queue isTaken/next handling and takeOuterLoadStep, JSONObject lazy clearing of the prepared-structure tables, SetPrototype compact-storage fast paths.
IndexingType.h renumbering of SlowPutArrayStorageShape (0x0C to 0x0E) and NumberOfIndexingShapes 7 to 8: checked the LLInt bia range test in LowLevelInterpreter64.asm and the cellButterflyStructures indexing in VM.h/FTL, which stay consistent since no array uses 0x0C.
LinkBuffer/X86 jump-shortening record changes and the DFGSafeToExecute StrCat symbol check.
No build or test run was performed in this review; the AOT subsystem itself (builtins, codegen, machine, image) was read only in part given its size.
Extended reasoning...
The range adds three commits touching 1014 files (about 90k insertions, 9k deletions), almost all under Source/JavaScriptCore/aot (builtins, codegen, facts, image, IR, machine, object), plus about 45 shared engine files: assembler (X86 jump shortening, LinkBuffer), DFG safeToExecute, LLInt AOT asm, module loader and microtask queue, JSON stringifier, Set prototype fast paths, IndexingType constants, Yarr JIT. It touches no auth, injection or data-exposure surface; the sensitive surface is memory safety and GC correctness in shared engine code. The prior run's single inline finding on the JSTests edit was addressed by the latest commit, and no new bugs were found, but the size and the shared-runtime scope rule out approving without a human reading the AOT subsystem in full.
… accessors, import() by a table
On 9fd6899. 12 files, +143 -31: Source 12 files +143 -31.
The first four are faults of this pull request that main does not have.
- A build could stop at default flags where a loop stores into an array and iterates another one
inside a try, as in `for (const z of [1, 2])` or `for (const z of rows[i])`: the iterated array
was asked in front of the loop, where it is not made or chosen yet. Such an array is asked there
only under a name that enters the loop.
- `w[i] = m[j] >>> 0` and `| 0` left a hole in `w` where `m` has no element `j`, and not 0: the word
of an element that may be a hole is boxed anew.
- `new Sub(n)` of a class below Array skipped a setter that is defined at an index of a prototype
above it. What is remembered of an allocation is now compared with the structure of the next one.
- `import(table.a)` of a `.cjs` file without exports loads its module form, also where a `require()`
by the same table has the other form.
Also
- `bun test --aot --rerun-each`: the next run of a test file begins while a file is still being
read, as in `bun test`. Bun's side has the other half.
- A class keeps the guards of the members that it declares where another class stands above it, as
after a second evaluation of its mixin. Before, every guard of the class ended there. The test
answers/a-declared-member-is-found-while-the-guards-hold.ir says so.
…imports under mocks
On 8f29965, with main ab89cb6 (#1032) merged in. Against 8f29965, beside what main
brings: 53 files, +136 -1,724: Source 53 files +136 -1,724.
Faults of this pull request that main does not have
- A match whose elements a loop reads by index is an array again. What stood in its place gave wrong
elements, where `exec` is stored on a RegExp and where a later match comes in between.
- Nests of two loops are compiled row by row again. What computed four rows side by side could read
the values of the first row in the others.
- A suspended function that compiled code queues is queued as on main. What stood in its place could
stop a program.
- A generator that a method makes in a loop is made again. The test whether it is a plain one stood
in front of the arguments, which may replace what generators inherit, and a build could stop at a
class below Map.
- A step of a generator that yields NaN runs once, also behind a store.
- `bun test --aot`: a mock with the original, of a file in a cycle of imports, is told at the end of
the run.
Also
- `bun test --aot`: the factory of a hoisted `vi.mock()` over a package that earlier test files
loaded is called when something asks for the package, also behind `vi.resetModules()`, as in `bun
test`.
- A test that stands behind a store of its iteration is asked in front of the loop, where such
stores are undone. Counts in fields stay integers where the function compares or keys by the
field.
- `new Set(x)`, where `x` is a Map or a string, no longer stops a program. Main has this fault.
…espaces of originals
On 93d4328. 26 files, +132 -22: Source 26 files +132 -22.
Faults of this pull request that main does not have
- The lanes with the compiler did not build: AOTRuntimeHooks.h names CallerFrameAndPC without
declaring it, two files use a header that they don't include, and on Windows a call lacked an
argument that the upgrade added.
- An array that an inner and an outer loop both push onto lost elements: a second change of its
length ends the append of the first.
- Half of a wide product of digits, raised by a shift, could leave int32 and answered wrongly.
- A build could stop at a remainder beside a BigInt in a `try`: a double was kept as a likely BigInt
in a rare block.
- `import * as ns` and `export * as ns` of the original of a mocked module, where the original is
compiled code, read the namespace of its record: such a record has no environment. Found by the
review.
- A test in front of nested loops whose bounds are calls read a block of another function. The
answer was not used.
- A store under an integer into an instance of a class that holds its other properties is served by
its stub again, and the growth of a walk over a loop that reads many lists under one counter is
linear again.
- `bun test --aot`: source files that import the original of a mocked module are told at the end of
the run.
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the latest push (cd237a7) and found no new bugs; at this size (1018 files, ~89k lines of a new AOT compiler plus runtime hooks) a human look is still worthwhile.
What was reviewed:
The namespace-of-compiled-original fix: Resolution::isNamespaceOfCompiledOriginal is now checked at JSScope.cpp:110 (goes Dynamic), JSModuleEnvironment.cpp:150 (answers getModuleNamespace), and the two JSModuleRecord.cpp binding sites, so neither reader reaches the null environment.
The other items in that commit (AOTBigIntValues, AOTStubsElements/PropertyExtraProperties, AOTTestRunOriginals) read as consistent with their stated intent; no candidate issues came out of this run's hunt.
Not done: no build or test run was performed in this review.
Extended reasoning...
The diff adds an ahead-of-time compiler under Source/JavaScriptCore/aot (builtins, codegen, facts, image, IR, machine, object, runtime, stubs) and threads hooks through JSC runtime files such as CyclicModuleRecord, JSScope, JSModuleEnvironment, JSModuleRecord, VM and Yarr. It touches no auth, injection or data-exposure surface, but it generates and links machine code, which is correctness-critical for the engine. The bug hunt ran to a dry streak with no findings, and the one prior inline finding is addressed by the latest commit. The sheer scope (1018 files, ~89k insertions) and the absence of any build or test run in this review are what decide defer over approve.
…ks, a missing include
On cd237a7. 16 files, +246 -63: Source 16 files +246 -63.
Faults of this pull request that main does not have
- A promise whose prototype was set to null, which a handler of another promise returns, was taken
for its value.
- The first object of a function that replaces its own prototype in its body, made in nested loops,
got the new prototype.
- Objects of a class that get fields after their birth could be many times slower, as in the parser
of ESLint: a class that is built where its callers are not all seen keeps its word for extra
properties.
- AOTRuntimeIntrinsicEmitter.cpp uses ConstantEmitter and includes its header now. The test
view-is-read-by-index-and-asked-for-its-memory-at-each-step.ir asks for what both tables of
built-ins give.
- `bun test --aot`: names behind a later star of a mocked barrel and originals of CommonJS and JSON
files that a mock patches are as in `bun test`. A mock that is laid or ends beside an import in
flight, a require() that was refused under a removed mock, and a file that a hook of a preload
loads are told at the end of the run.
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the two pushes since my last pass (cd237a7, 2e57454) and found no bugs; given this PR adds a whole AOT compiler (~89k lines across ~1000 files), a human look is still worthwhile.
What was reviewed:
The namespace-of-compiled-original fix in cd237a7: the new Resolution::isNamespaceOfCompiledOriginal is checked at every reader that previously dereferenced the environment (JSScope.cpp abstractAccess, JSModuleEnvironment::getOwnPropertySlot, JSModuleRecord::importedBindings/liveImports, WebAssemblyModuleRecord::initializeImports), so the null-environment path I raised earlier is closed.
The new JSPromiseType checks in AOTStubsCoroutinePromises.cpp and AOTStubsCoroutineSuspension.cpp: both sit behind a cell/object guard, so the type load is safe on non-cells.
The AOTSlotPlan.cpp and AOTSweepStores.cpp heuristics changes: both only tighten conditions (fewer layouts get hidden words / fewer births move), so they are conservative.
Not done: no build or test run was possible in this review; the compiler lanes and bun test --aot runs named in the commit messages were not independently executed.
Extended reasoning...
The range adds the AOT compiler under Source/JavaScriptCore/aot (codegen, facts, image, IR, machine, object, plan, runtime, stubs, walk) plus small hooks in the module-record runtime (AbstractModuleRecord, CyclicModuleRecord, JSModuleEnvironment, JSScope, WebAssemblyModuleRecord) and YarrJIT, about 89k insertions over 1018 files. It touches no auth, crypto or injection surface, but it emits machine code and changes module linking and promise/coroutine stubs, which are correctness-critical. The two commits since my last review are small (+132/-22 and +246/-63) and one of them closes the inline finding I posted earlier; the bug hunt ran dry on this version. The sheer size and the fact that the compiler lanes could not be built or tested here are what decide defer over approve.
…on, packages behind resets, the DFG
On 2e57454. 23 files, +174 -69: Source 23 files +174 -69.
For CI
- AOTWalkFusedChains.cpp finds the function of a chain by the row of its method. It read the names
of functions, which are for tests and may not ship.
- growth-of-work/a-resumption-costs-what-was-kept-however-large-the-frame-is allows machines without
instruction counters, as its neighbors do.
Faults of this pull request that main does not have
- An object of a constructor function that replaces its own prototype in its body was allocated
behind the body: a store or a read by name met the new prototype.
- `bun test --aot`: behind vi.resetModules(), a later mock of a built-in module loaded the packages
that the reset keeps again. A CommonJS file that required the original of a mocked file while a
mock over it was pending ran again in the next test file. A mock over an original that is awaited,
and a file that a hook of a preload loads before a reset, are told at the end of the run.
Faults that main has
- A call without a receiver under the name of a list's method, as (0, list.push)(value), in a loop
that counts stopped the build.
- `--aot-harden=proof-checks`: a loop that stores a variable of its module and is entered before the
first store stopped the build, or the executable ended with an error.
The DFG and the FTL, with and without `--aot`
- DFGFixupPhase.cpp: a store into a double variable or a double array takes null, undefined, true
and false as they are. Arithmetic alone converts them.
- DFGClobberize.h, DFGPreciseLocalClobberize.h: an arguments object of the machine frame that the
FTL does not allocate reads the stack, so a tail call that became a jump stores its arguments.
- DFGValueRepReductionPhase.cpp, DFGConstantFoldingPhase.cpp: a load of several structures with a
double result keeps a constant that is not a number, which then fails the number check.
- DFGNode.cpp: a node that takes over the uses of another node takes over how they see its value.
…ns with a gate, programs and suites
On 4c1deb0. 265 files, +28,294 -445: scripts 3 files +28 -1 . src 11 files +277 -60 . test 251
files +27,989 -384. Pins the preview build of oven-sh/WebKit#1036's commit 569fa472.
[skip size check], as in 607519a.
The review's findings
- A SQLite database in write-ahead mode is embedded with what its log has committed
(src/bundler/aot/database_log.rs).
- A plugin that fails on an asset that is read behind the load fails the build.
- The list of removed paths holds only what the executable has something at or below, once.
- chown() with -1 keeps the user or the group.
- Asynchronous cp(), recursive readdir() and, on Windows, open() answer for embedded files as their
synchronous forms do.
For CI
- scripts/verify-baseline-static: five functions of compiled programs use AVX2 or BMI2 after they
have asked the processor. The lists name their gates.
- On Windows, a folder that moves out of the executable is written with the platform's separators,
and a package.json is asked for once.
- Counts, marks and lists of known differences that the compiler moved are restated. A store to
undefined or null under a key is compared with `bun run`. Two traces no longer hold positions
inside the engine.
Also
- BunPlugin.cpp: a CommonJS package that an earlier test file loaded does not run again where a
later file mocks it and asks for its original.
- import_functions.rs: a line end behind `return` in the text of a function ends the statement.
- test/js/bun/jsc: four tests for the engine's corrections of the DFG.
- test/aot: suites of test files as people write them, alone and as one project; 75 stored programs
that are longer than the others; stored programs and test runs for what the engine's commit
corrects.
…guments, mocks
On 569fa47. 23 files, +197 -66: Source 23 files +197 -66.
For CI
- AOTStubsCoroutineResumption.cpp, AOTJobs.cpp: the run time makes the promise that a handler's
suspended frame needs, so an executable without promises does not keep their operations.
- AOTOperatorEmitterEquality.cpp: an equality whose operands' types decide it is one operation.
Builds that check their statistics stopped.
- AOTObjectBuiltinOperations.cpp: Object.values() and its neighbors ask whether an object that has
lost its elements can have any. Builds with assertions stopped.
- AOTScan.cpp, AOTMachineFolding.h: two arguments that each make a node are no longer evaluated in
the order of the platform, and a rotation is found from either side.
- AOTShapeTestEmitter.cpp, arm64 alone: the second look at a marked shape has one size whichever
number the shape has, so a test run and an executable have the same code.
- AOTWalkTypedElements.cpp: each read of an element counted all operations of its loop, so the build
of a loop of many statements took the square of its length.
- AOTFactsOfProgram.cpp: an import of bun:test by name keeps the test runner in a trimmed
executable, whichever name it is.
Faults of this pull request that main does not have
- AOTWalkGuesses.cpp: an async for-of that adds to an object's number or string stopped the build.
- AOTWalkChoices.cpp: a loop that fills an array literal by its stores left its version at the 26th
element.
- AOTStraightEmitter.cpp, under --aot-testing=ForceStraightCompilation: boxing an integer parameter
lost a number beside it.
- `bun test --aot`: an import of a file that re-exports a mocked CommonJS file no longer throws the
error of an earlier factory. A mock over a loaded mock whose factory waits for importActual(), and
a mock that was named by import() behind a reset, are told at the end of the run.
Also
- AbstractModuleRecord.h declares bindingName() in every build.
- RepatchInlines.h, with and without `--aot`: an iteration of what is not iterable says so in the
DFG and the FTL, as in the other tiers.
…st runs, traces of awaits
On f37cd00. 17 files, +119 -14: Source 17 files +119 -14.
- AOTWalkTypedElements.cpp: a push that fills a list of its own in its loop is taken in line again.
- AOTWalkChoices.cpp: a literal that loops fill with numbers of their counter alone, or that is born
in a loop, is allocated with storage of its own again, so the turns can be laid out in a row.
- AOTWalkEscapeAnalysis.cpp, `--aot-harden=proof-checks`: an object of a constructor function is
allocated once, and each of its fields is compared with what is believed of it.
- AOTLinkReach.cpp: a CommonJS file that loads no other file has run to its end before another can
find it, so a test run folds its constants as an executable does.
- A stack trace that is taken while an await reads `then` names the frames that `bun run` names:
over the elements of a list the function that steps it, and no frame of a function that has ended
with its call.
- AOTTestRunMocksLetGo.cpp: whether a module is imported by a statement, for a notice of `bun test
--aot`.
…e engine's commit
On ccc2905. 50 files, +737 -242: scripts 1 file +1 -1 . src 18 files +264 -226 . test 31 files +472
-15. Pins the preview build of oven-sh/WebKit#1036's commit f37cd001. [skip size check], as in
607519a.
For CI
- mordant's findings and clippy's line, without a change of behavior: parameters that bear one name,
flags that are one state, functions that stood twice, lines that only tests use.
- A test run's expectation that was older than `bun test`'s own behavior, a control of
independence.test.ts that names ARM64's call, and one option less in two tests that builds with
assertions refuse.
Also
- A file that a package has written is read by every form of its path: text, bytes, URL, relative,
not normalized.
- Tests for what the engine's commit corrects.
…essor pairs in nested loops
On 3cb11e4. 16 files, +79 -19: Source 16 files +79 -19.
- Ten helpers of the code generator (an operand's place, the arena, the numbering, the encoder) are
always compiled in line, which an `-Os` build did not do.
- AOTBuiltinTable.cpp: the first lookups of a built-in by its token compare the last name before
they spell the token, and the maker of filled arrays is looked up once.
- AOTObjects.cpp: a condition that the two before it contain is taken out.
- AOTWalkReplacements.cpp: accessors of a literal are defined together only within the part of a
block that is walked at once, so a getter in front of a nested loop does not wait for the setter
behind it.
- AOTCodeGenerator.cpp: a number that 255 or more blocks of a loop hand to their handler is boxed
where it is computed. The count of its uses ends there, and the build stopped.
- AOTCompiledFrameIterator.cpp: findFunctionFrame() asks for the instance before it looks up the
call site.
… builds on any number of threads
On 5f61130. 59 files, +1,371 -168: scripts 1 file +1 -1 . src 16 files +286 -80 . test 42 files
+1,084 -87. Pins the preview build of oven-sh/WebKit#1036's commit 8f8aeee8. [skip size check], as
in 607519a.
- decl.rs: `bun check` keeps the halves of a surrogate pair apart where a constant joins them, as
TypeScript 7 does. A compiled program joins them.
- lower_stage.rs: a build that would report a repeated name asks whether `bun check` reports it.
- alternatives.rs: where the values of a path are followed again, the expressions and calls that
hold nothing by themselves are remembered, so functions that hand paths to each other take fewer
steps.
- node_fs.rs, jsc_hooks.rs, BunObject.rs, mv.rs, compiled_program.rs: a file of the executable that
the program has written is the one on disk for `Bun.mmap()`, the shell, `fs.openSync()` and
`fs.cp()` too.
- services.rs, bind_stage.rs: a file that a program only asks about is embedded with its bytes where
the program also reads a path that the build does not know.
- written_type.rs: the annotation of a value that a type names by `typeof` is looked at with the
types around it, so the checker's answer does not depend on which thread asks first.
- `bun test --aot`: a data file or a file of a package that a reset keeps under a mock is named
where its next load shows another object.
- Tests for what the engine's commit corrects.
- The folders of test runs are in twenty-four test files, where one of sixteen could pass its limit
on CI.
… under checks
On 8f8aeee. 10 files, +79 -57: Source 10 files +79 -57.
- AOTStubsCoroutinePromises.cpp: a promise that a job of compiled code fulfills notes that its
resolving functions have been called, so one that is called later changes nothing.
- AOTWalkListReadsInLoops.cpp, AOTWalkLoopsBuiltTwice.cpp, `--aot-harden=proof-checks`: what is
known of a list's storage is forgotten in front of code that runs after the list was converted.
- AOTWalk.cpp, `--aot-harden=proof-checks`: a read whose hole would be read as NaN is not moved
where arithmetic was folded onto it.
- AOTImageBuilder.cpp: the global object counts as handed out only where the code of every built-in
that is handed out is in the image. A `dispatchEvent` without a receiver behind an
`addEventListener` that is found by a name ends with an error that can be caught, where the
program aborted.
- JSModuleLoader.cpp, VM.h: a `require()` of a module whose load an outer `require()` has begun is
answered as on main again. The steps that it took out of the outer load's queue could settle a
CommonJS module that was still running.
…conversions
On 6741c5d. 11 files, +55 -53: Source 11 files +55 -53.
- JSModuleLoader.cpp, ModuleRegistryEntry.cpp, JSMicrotask.cpp: a `require()` of a module whose load
an `import()` has begun loads it again only where the loader is running no CommonJS file. Below
one, the load could lead back to that file, and its names were `undefined`. The rest of the change
goes back to main's text.
- AOTCodeVerifier.cpp: a conversion of a list to doubles keeps its length, so an access that rests
on a length from before it is in form. The effects that are declared of the conversion are
asserted.
…es, macros beside tests
On 1977ee4. 68 files, +2,099 -1,634: docs 2 files +19 -2 . scripts 1 file +1 -1 . src 30 files
+1,110 -915 . test 35 files +969 -716. Pins the preview build of oven-sh/WebKit#1036's commit
4e544510. [skip size check], as in 607519a.
- Merges main with #48297, whose lines were here. The size bounds stay as they are
restated here, and the pin is the preview.
- compiled_program.rs, StandaloneModuleGraph.rs, node_fs.rs, standalone_graph_jsc.rs: where the
sources are gone, embedded files are read, and what would change one is refused as on main.
Listings show what the disk has. The overlay of written, renamed and removed files goes out, and
with it the laying of files and the writes inside the executable that the review asked about.
- database_log.rs: a commit that claims more pages than the database and the log have ends the use
of the log.
- mv.rs, Blob.rs, write_file.rs, copy_file.rs, Cmd.rs: what the program has written or moved is
marked behind the write or the move that succeeded, for that file alone.
- Blob.rs, FileRoute.rs, ByteBlobLoader.rs: a slice of an embedded file keeps its range, and a route
or a stream of one reads it as it is. The text of a blob is decoded again behind the text of a
slice.
- NodeVM.cpp, BunGlobalScope.cpp: in a context over a Proxy, or over the global object of another
context, `delete name` of a name that the script has not assigned leaves it alone, as on main and
in Node. Deletes through an object keep asking the sandbox.
- module_mock.rs: a macro's module loads do not ask whether a preload runs.
- BunPlugin.cpp includes the header of a class that it uses, so a debug build compiles.
- A `require()` of a module whose load an outer `require()` has begun is answered as on main again;
its tests go with it.
- compatibility.mdx, troubleshooting.mdx: unused built-ins.
- mock-ends-for-require.test.ts: two projects of the review. Tests for what the engine's commit
corrects.
- Tests: `bun test` is read on one thread in the files whose answer depends on its threads; a
Worker's case waits for its answer, a program of three parts is three tests, a child sleeps as
long as it is told.
… mocks
On 4e54451. 6 files, +52 -7: Source 6 files +52 -7.
- AOTModules.cpp, AOTTestRunOriginals.cpp: the original of a mocked file that was written since the
build comes from the disk, as in `bun test`. The original of a file that the program has replaced
in `require.cache` stays compiled.
- AOTModules.cpp, AOTTestRunFactories.cpp: `importActual()` of a mocked file counts as an import in
flight, so a mock that is laid over it is told. The calls in flight are swept when their list has
doubled, where every call swept the whole list.
- AOTWalkLoops.cpp: what a generator carries in its consumer is set back with the parameters where
the versions of a loop are built.
…in, a notice behind stars
On 3d7aba1. 4 files, +5 -19: Source 4 files +5 -19.
- AOTModules.cpp, AOTTestRunOriginals.cpp: where an original runs is asked as before 3d7aba1, and
it comes from the disk if its file was written since the build. What a program stores in
`require.cache` decides nothing else.
- AOTModuleLinking.cpp: a CommonJS file that a name behind two stars runs early, after a mock had
set its earlier run aside, is told.
…hrough load plugins
On ff1a33f. 5 files, +18 -9: Source 5 files +18 -9.
- AOTWalk.cpp, AOTPromotedEnvironments.cpp: a captured value that a function which runs once stores
at its definition is held to the types of its captures where the closures are made. `if (x >= 0)
return () => x` stopped the program for a negative `x`. Closures share the stored value only if
they read it under one type.
- AOTModules.cpp, AOTModules.h, AOTTestRunOriginals.cpp: the original of a mocked file goes through
a load plugin that was added after the file had run.
…s, Android
On 3126527. 26 files, +292 -588: docs 1 file +1 -1 . scripts 1 file +1 -1 . src 13 files +87 -413 .
test 11 files +203 -173. Pins the preview build of oven-sh/WebKit#1036's commit fdc97d48.
[skip size check], as in 607519a.
- Blob.rs, FileRoute.rs, ByteBlobLoader.rs, standalone_graph_jsc.rs: the blobs that followed the
path of an embedded file go out, as the review asked. They hold the bytes that they were made
with. What the program has written is marked behind the write, and the text of a blob is decoded
again behind the text of a slice, as before.
- mv.rs: the destination is asked where it is named. private_copy.rs: a mapping that was not found
is asked for again.
- BunPlugin.cpp: `importActual()` of a mocked file is noted as a call in flight, so a mock that is
laid over it is told.
- compiled_program.rs: `dup3()` is called by its number, which the libc crate has for Android too.
Both Android builds stopped there.
- spawn-stdin-readable-stream.test.ts: the child whose output is locked waits for the end of its
stdin: the bytes of a child that has ended were taken ahead of the getter.
- Clippy's lines. Tests for what the engine's commits correct; two more projects whose references
are read on one thread.
…udged by their tests
On fdc97d4. 4 files, +95 -58: JSTests 1 file +0 -55 . Source 3 files +95 -3.
- AOTWalkKeyedAddresses.cpp: a text that the compiler folds, such as `given.slice(1)`, narrows no
effects of a keyed access. A build stopped where such a text was the key of a store in a loop.
- AOTWalk.cpp, AOTCodeVerifier.cpp: the number that a store read out of its value answers the next
read of the element. Builds under `--aot-harden=proof-checks` stopped at some loops that ask for
the truth of such an element. A new rule checks that such a number is tested before it is taken.
- error-stack-trace-memory-is-reported.js: a test of the memory of stack traces is removed.
On 29fd0a7. 1 file, +4 -0: Source 1 file +4 -0.
- AOTScan.cpp: a hole that an array literal appends behind a spread stores `undefined` into the
list's contents. `[...more, , "b"]` was taken for a list without holes, and a program that read
the hole could end by a signal.
On 43489f1. 5 files, +40 -8: Source 5 files +40 -8.
- AOTLinkFlow.cpp: an object whose prototype becomes null may lack the names of `Object.prototype`.
The reads under such names and the reads under any name each learn it now; a program that made
such reads in closures could end by an abort.
- AOTTestRunFactories.cpp, AOTModules.h, AOTModules.cpp, AOTModuleInstances.cpp: a mock of a file
that only an `import defer` has linked waits for the file's run, as in `bun test`. Namespaces of
`import defer` show the values of mocks.
The reason will be displayed to describe this comment to others. Learn more.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
🟡 JSTests/stress/error-stack-trace-memory-is-reported.js — nit: Maintainers lose the only regression test for ErrorInstance reporting stack-trace memory to the collector, while the feature it covered is still in the tree. The file is deleted whole, and commit 29fd0a7 only says "a test of the memory of stack traces is removed", with no reason or replacement. Fix: keep the test (or a replacement that exercises reportStackTraceMemory at Source/JavaScriptCore/runtime/ErrorInstance.cpp:293 and the compiled-frame path at ErrorInstance.cpp:255), or state in the commit why the behaviour no longer needs a test.
Why this was flagged
The diff deletes JSTests/stress/error-stack-trace-memory-is-reported.js, a fork-added test from commit bcbefd0 ("ErrorInstance reports the memory of a long stack trace to the collector"). The code it covers still exists after the change: ErrorInstance::captureStackTrace calls reportStackTraceMemory(vm) at Source/JavaScriptCore/runtime/ErrorInstance.cpp:293, and tryRetainReturnAddresses at ErrorInstance.cpp:246-261 gains a new early return for a zero stackTraceLimit at lines 251-254. No replacement test is added and JSTests/BUN-TEST-DIFFERENCES.md has no entry for it (it is a fork test, so none is required). On the base branch a regression in the accounting of stack-trace memory (for example an error whose frames are retained as return addresses but never reported) fails this test; after the merge it passes silently, so a maintainer gets no signal when the new zero-limit path or the compiled-frame capture stops reporting memory. The commit message for 29fd0a7 gives no reason for the removal, so it cannot be told from a flaky-test cleanup.
Verification: nit. Triggering condition: any future regression in ErrorInstance's stack-trace memory reporting now goes uncaught. The file JSTests/stress/error-stack-trace-memory-is-reported.js is deleted whole, and commit 29fd0a7 gives no reason. The feature it exercised is still in the tree: ErrorInstance::reportStackTraceMemory is still called at Source/JavaScriptCore/runtime/ErrorInstance.cpp:293. No replacement exists.
…, names in texts, node:vm as on main
On f1d9833. 27 files, +597 -400: scripts 1 file +1 -1 . src 13 files +148 -260 . test 13 files +448
-139. Pins the preview build of oven-sh/WebKit#1036's commit 326e59ee. [skip size check], as in
607519a.
- node_fs.rs, Blob.rs, FileSink.rs, Builtin.rs, Cmd.rs: one function refuses each open that would
write what the executable holds, as the review asked: appends, writers and redirects of the shell
too. mv.rs refuses what `rename()` refuses. FileSink.rs tells whether its flags truncate.
- compiled_program.rs: a file on the disk where an embedded folder lay is the disk's.
- BunPlugin.cpp: a spy mock of a file with a top-level await has an original of its own, and
`require()` of such a file waits for it, as in `bun test`. A spy mock over a file that loaded
again patches its later run. A CommonJS file that `vi.importActual()` loaded stays the original
for the next test file.
- shown_names.rs: `--minify` leaves the names that a text of `new Function` or a direct `eval`
declares.
- NodeVM.cpp, NodeVM.h, BunGlobalScope.cpp, BunGlobalScope.h are main's again.
- Stored programs for what the engine's last commits correct: texts that the compiler folds as keys,
stored sums that are asked for their truth, holes behind spreads, names that objects below null
lack.
…luated strings
On ae74514. 6 files, +131 -14: scripts 1 file +1 -1 . src 3 files +25 -13 . test 2 files +105 -0.
Pins oven-sh/WebKit's commit 7101dff5, the merge of oven-sh/WebKit#1036. [skip size check], as in
607519a.
- mv.rs, node_fs.rs: a move of the shell notes which sources were on the disk. Behind it, also where
it fails part-way or leaves some under `-n`, those that are gone are forgotten in the executable
and their targets are noted, as the review asked.
- errors.rs: what the checker finds in a string that the build knows to be evaluated is dropped
while the file's tree is there. Beside a class whose members have no written types, a string that
only sloppy code accepts stopped the build with TS1102 or TS2300.
#1036 had conflicts in four files, in six places. Each place takes the code of main and the async context of this
branch.
- startImportWhoseTextHasCome() has main's loaderPromise behind the async context of the call.
- holdTextOfCall() inserts the BatchedFetch at main's place, with the async context of the call.
- startImport() notes the import in flight, as main does, and queues its step with the async context.
- loadModuleSync() loads the root again as a synchronous load. main does that in plain Bun too now.
Three places of #1036 that merged without a conflict take the async context too.
- tryRestartReleasedImport() makes the call of an import again where the instance of its module was released. It
passed the promise as the fourth argument of startImportWhoseTextHasCome(), which is the async context on this
branch. It takes the async context of its job now and hands it on.
- evaluateOriginalAsync() is a third executeAsync(), for a compiled original. The job behind its top-level await gets
asyncContextOfModulesThatWait(), as the jobs of the other two do.
- ModuleRegistryFetchSettled can run a second time, behind the source of a module that was released during its
fetch. The first run gives the async context back to the registry entry (giveBackAsyncContext()), and the second
run takes it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The JavaScriptCore side of a round of work on Bun's ahead-of-time compiler (
bun build --compile --aotandbun test --aot, both experimental). Bun's side is a pull request in oven-sh/bun that pins this one's preview. This follows #1028 and #1030 and lies on the upgrade of #1015 and on #1032.Merge order
This merges first; then Bun's pull request pins the merged commit and runs CI once more.
What's in it
1,035 files, +90,379 −9,452 against main.
aot/walk: the main passaot/runtimeaot/testsaot/codegen,stubs,machine: code generationaot/image: the imageaot/aot/Outside
aot/Behind
ENABLE(AOT)or in code that compiled programs alone reach, but for the rows that say otherwise.assembler/LinkBuffer.cpp,assembler/MacroAssemblerARM64.hand 4 moreassembler/testmasm.cppb3/B3Generate.cppbytecode/RepatchInlines.hdfg/DFGSafeToExecute.hdfg/DFGFixupPhase.cpp,dfg/DFGClobberize.hand 4 morellint/LLIntThunks.h,llint/LowLevelInterpreterAOT.asmparser/ParserArena.h,runtime/CodeCache.cppand 2 moreruntime/AbstractModuleRecord.cpp,runtime/AbstractModuleRecord.hand 3 moreruntime/JSModuleLoader.cpp,runtime/JSModuleLoader.hand 3 morerequire()of a module whose load animport()has begun. Plain Bun tooruntime/SetPrototype.cpp,runtime/ObjectConstructor.cppand 3 moreruntime/JSONObject.cpp,runtime/LiteralParser.cppJSON.stringify()uses only for objects are cleared only for objects. Plain Bun tooruntime/JSObject.cpp,runtime/JSFunction.cppand 1 moreruntime/Error.cpp,runtime/ErrorInstance.cppand 1 moreruntime/IndexingType.hSlowPutArrayStorageShapeis 0x0E, so one bit tells the arrays with boxed elements. Plain Bun tooyarr/YarrClassTable.cpp,yarr/YarrJIT.cppJSTests/BUN-TEST-DIFFERENCES.md,JSTests/stress/new-largeish-contiguous-array-with-size.jsJSTests/stress/error-stack-trace-memory-is-reported.jsruntime/JSModuleEnvironment.cpp,wasm/js/WebAssemblyModuleRecord.cppChanges
Wrong answers and stopped builds that main has, fixed
typeof x === "object"of a built-in object (JSON, console, process) that comes out of a Map, a WeakMap or a list--aot-harden=proof-checkspush()orpop()row[i] += factor * b[i]wherefactoris undefined in one callnew Set(x)wherexis a Map or a stringThe effects of a body that nobody enters count as those of unknown code now, so a build can also assume less at default flags.
stress/new-largeish-contiguous-array-with-size.js: the bound of the test holds for the heap growth fractions of builds without the memory footprint API; the test names them, as the review asked.Code without types and code of libraries
in,delete,for-in, spreads and rest objects.Regular expressions
RegExpare kept only where something may read them.Generators, async code and iteration
yield*, destructuring,Array.from(), spread andusing. Awaited calls, returned promises and suspended frames take fewer steps.Test runs
Build
Checks
--aot-check) that the checks of a build must refuse.Known issues
None of the engine alone that main does not have. What differs from
bun runandbun testis listed in Bun's pull request.Tests
IR tests and native tests are in
aot/tests; stored programs are in Bun's pull request. CI's run of this pull request is the first of this tree.