Repository navigation
Conversation
A --define whose value is a name or a dotted path (NAME=target, ROOT=holder.value) was read from the global object under --aot, also where the file binds the first name. Plain Bun reads the binding. The binder looks the first name up in scope at each expression that such a define can replace and records it as an implicit reference (ImplicitReferenceKind::DefineHead), as it does for a JSX factory. The fold puts the record into a new decision, Decision::NamedPath. With no record the decision stays Decision::GlobalPath, so a first name that is free is read from the global object as before. Lowering reads, calls, stores to and deletes through the recorded binding, in JavaScript and TypeScript, for one-name and dotted values: a defined name does what the same name does when the program spells it there. The readers that count the uses of a binding count a define head where the fold applied the define, and the finders of required files and of Worker files follow a define as they follow the spelled name. The build stops with TS100050 only where the compiler cannot do what the spelled text does: a call of eval, of require.resolve and its like, or of feature() through a define, a store to a name of CommonJS, and a store or delete that strict code forbids. A file in which the fold applied no define head takes the path that it took before: lowering and the scan of import sites keep one flag for each file. test/aot/run-parallel.ts starts the plain reference run of a program with the --define flags of its Flags line, and compares a program that stops with TS100050 with what plain Bun prints.
|
How this was reproduced, on main at 3f3a9ff (release build, Linux x64): // ident.js
const target = "the binding of the file";
globalThis.target = "the global";
console.log(NAME);bun --define NAME=target ident.js
# the binding of the file
bun build --compile --aot --define NAME=target ident.js --outfile aot && ./aot
# the globalA dotted value does the same. With With this PR both executables print what plain Bun prints. CI: every test that is red on this PR is red on main too, for example PR: #47718 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the new stored-name collection for define paths at assignment targets in src/aot/load/worker_files.rs (find_name_stored_through_define) — it can only add stored names, so it errs toward keeping more worker code reachable, never toward dropping a file; and the TS2588/TS2632 const/import-assignment check in src/sema/check/aot/early_errors/declarations.rs for stores routed through a define, which does not bypass the existing run-time TypeError.
Extended reasoning...
The change is a ~10k-line compiler PR across src/sema and src/aot that teaches the fold, binder, import-site scanner and lowering to follow --define paths whose head is a binding of the file, plus hundreds of fixture tests; it touches no auth, crypto or injection surface. Four confirmed findings about wrong-file bundling and missed stops are posted inline, so a human review is already signalled; this note only records two adjacent concerns examined and ruled out.
Findings marked 🟡 are optional suggestions and need no follow-up push.
…ead only below its key
The finder of required files reads the pushes to a list in the order of
the text. A define names the list between the identifiers that spell
it, so the finder sorts the places that name the list.
The binder records the first name of a define value only where the
accesses above the root spell the whole key of the define. DefineHead
has the member names of the key. A file that writes only the root of a
key, such as process.argv for the key process.env.A, gets no record, no
use of arguments and no mark for require.
The readers of call sites take the decisions of the fold. Where a
define puts a name or a path in place of a callee, they read that name
or path. They do not read the member that the text spells there. So
require("node:url") under --define require=load is no request for a
module, and import.meta.require(..) under --define
import.meta.require=load calls load. The finders of module members, of
resolver answers, of changed files and of lists that stay as written
ask with the decisions.
The build stops with TS100050 at a call that is written eval(..) under
a --define of the name eval, also where the value names a global.
|
The findings of the first review pass are handled in c31b115:
Two notes stay open and are listed under "Not addressed" in the PR notes: a define at a member that is no callee, and |
|
Closing: #47942 rewrites the front end of the ahead-of-time compiler and its tests for the engine's new compiler (oven-sh/WebKit#997, merged), so this no longer applies and would conflict with it. If the problem still shows once that is in, please open a new pull request against |
|
Understood. When #47942 is in, I will run the two programs of the description against |
Related to #40857
Question for a maintainer. Where
bun --definereads no binding (return NAME; const target = 1),--aotnow does what the spelled name does. Should it stop withTS100050?Problem
bun build --compile --aot --define NAME=targetreadsglobalThis.targetwheretargetis a binding. Plain Bun reads the binding. With no such global the executable throwsReferenceError: target is not defined.Folder::try_decide_define(src/sema/fold/folder_names.rs) makesDecision::GlobalPathfrom the value's text, and lowering reads its first name from the global object.Fix
Decision::NamedPath.Decision::GlobalPath.test/aot/parallel(285 fail on main) andtest/aot/build-flags.test.ts. Self-reviewed: 42 concerns raised, 36 addressed.Background
run-parallel.tsnow passes a program's--defineflags to its plain reference run.Downsides
eval,feature()orrequire.resolvethrough a define, a store to a CommonJS name.Notes
The question, with a program
bun --define NAME=targetprintsthe global: its transpiler removes the declaration behindreturn. The spelled program (return target;) prints the same underbunand throwsReferenceError: Cannot access 'target' before initializationunder--aot. With this change the defined name throws the same error under--aot. 30 of the new programs record such a difference with "Plain Bun prints another output" and an/* Output: */block, as 41 programs on main do for spelled names.An earlier version of this change stopped the build at such places. It needed about 640 lines that predict what the transpiler of
bun runremoves, and it stopped builds that are right on main. It is gone.The same question holds for
--define RUN=eval: plain Bun prints a directeval, main's--aotmakes an indirect one, which is right only when the text needs no local. This change stops the build whereRUNis called by its name.What a defined name does now
--aoton main--aotnowvar,let,const, function, class, parameter, catch parameter, importReferenceErrorarguments,require,module,exports,__filename,__dirnameReferenceError, orundefinedundertypeofwithor a directevalmay declareROOT=holder.value)Stores,
++, compound assignment, destructuring targets,deleteof a member, calls (a dotted value keeps its holder asthis),typeofandnewgo through the same binding.require,createRequireandWorkerreached through a define load the file as the spelled name does.The stops
All have the form
TS100050: --aot does not support --define <key>=<value> where <key> <reason>:is called: the value or the key isevaland the place is called by its name, the value is theresolve,callorapplyof arequire(ormodule.require.bind), or the value isfeatureofbun:bundle.is stored to: the value ismodule,exports,require,__filenameor__dirname.is stored to in strict mode,is deleted in strict mode: plain Bun gives aSyntaxErrorfor the printed text.A program that stops carries a
/* Plain Bun prints: */block, andrun-parallel.tscompares it. One more stop has no program: lowering reports the define where a function cannot reach the storage of the binding. No input was found that gets there.Costs, measured
User-mode instructions from the CPU's counter (
perf_event_open, the process with all threads and each thread alone), release builds of main 3f3a9ff and of this commit, runs in turns.valgrind,perf,bloatyandstraceare not installed where this ran. The program has 301 modules. Each has onetypeof USEDin a function that declarestarget.bun build --compile --aot --aot-threads=1--define USED=absentName, front end--define USED=target, front endsize). File: +61,440 bytes.test/aot/tools/same-output.ts --before <main> --after <this PR>printsIDENTICAL: 289 programs(IR and machine code).test/+4,129 in 455 files.What ran
bun bd test:test/aot/build-flags.test.ts16 pass,test/aot/names-of-bindings-in-minified-executables.test.ts12 pass,test/aot/spies-through-helpers.test.ts8 pass. A release build of main fails 6, 6 and 2 of them.test/aot/run-parallel.tswith a release build over every program undertest/aot/parallel: 7,431 pass, 12 skip, 30 fail. A sample of 16 of the 30, at least one of each folder, fails the same way with main's build and main's runner on this machine.cargo test -p bun_sema -p bun_sema_parser,cargo test -p bun_aot --libandcargo test -p bun_aot --test ir_testspass, but for two--libtests and two IR files that nest very deeply. Those fail on main too in a debug test build.RUN=evalis called). Main: none equal.Not addressed from the self-review
TS100050with the reason in the message.test/aot/toolsthat compare a define with the spelled name.test/aot/fuzzer-judging.test.tsfails 2 tests on a debug build of main, so this PR does not change it.yield,await,let,static).withor a directeval, which reads the global on main in the same way.REQ=requirepassed to a function that calls it with a variable) build and then fail to load the file. The spelled programs do the same on main.bun test --aot --define IT=testfor a name of the test runner. An earlier version rewrote the key in the tree. That part is out of this PR.typeof requireunder a define whose key isrequirefolds to"function", as on main.Order and history
--aotagainst plain Bun. No user reported it.bun --define. Where a key matches follows the fold as on main.no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/aot/names-of-bindings-in-minified-executables.test.ts, test/aot/build-flags.test.ts