From b519e62a588e050608a583fa15bee4bb560984bb Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 13 Aug 2026 20:05:21 -0500 Subject: [PATCH] fix: preserve callback binding identity --- .../src/frontend/lowering/lower-calls.ts | 54 +++++++++++++++++-- tests/harness/library-callbacks.test.ts | 28 +++++++++- .../callbacks/lib_ambient_const.ts | 18 +++++++ tests/library-mode/callbacks/lib_builtins.ts | 7 +++ tests/library-mode/callbacks/probe_builtins.c | 15 ++++++ .../callbacks/profile_builtins.json | 21 ++++++++ 6 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 tests/library-mode/callbacks/lib_ambient_const.ts create mode 100644 tests/library-mode/callbacks/lib_builtins.ts create mode 100644 tests/library-mode/callbacks/probe_builtins.c create mode 100644 tests/library-mode/callbacks/profile_builtins.json diff --git a/packages/compiler/src/frontend/lowering/lower-calls.ts b/packages/compiler/src/frontend/lowering/lower-calls.ts index 6eb2488c3..4e6a453cd 100644 --- a/packages/compiler/src/frontend/lowering/lower-calls.ts +++ b/packages/compiler/src/frontend/lowering/lower-calls.ts @@ -2660,7 +2660,10 @@ export interface FfiValidationResult { * Candidate declarations are signature-only functions bearing the manifest * name anywhere in the program. Multiple scoped declarations are all native * bindings under the existing name-based call surface, so every candidate - * must fit the one manifest ABI. */ + * must fit the one manifest ABI. Library callbacks additionally exclude + * declaration files: lib.d.ts/@types names are existing TypeScript ambient + * surface, never program-authored callback declarations merely because a + * profile channel happens to share their spelling. */ export function validateFfiImports(L: Lowerer): FfiValidationResult { const diagnostics: ScrDiagnostic[] = []; const symbolsByName = new Map>(); @@ -2670,6 +2673,7 @@ export function validateFfiImports(L: Lowerer): FfiValidationResult { if (configuredNames.size === 0) return { diagnostics, symbolsByName }; for (const file of L.program.getSourceFiles()) { + if (L.libraryCallbacks && file.isDeclarationFile) continue; ts.walkPreorder(file, (node) => { if (ts.isFunctionDeclaration(node)) { if ( @@ -2709,6 +2713,14 @@ export function validateFfiImports(L: Lowerer): FfiValidationResult { { file: L.entry.fileName, start: 0, end: 0 }, ), ); + } else { + // Preserve the distinction between unused capacity and a binding + // whose program declaration was found but failed validation. The + // latter deliberately leaves no map entry so calls poison without + // duplicating the program-level diagnostic; the empty set lets call + // lowering inspect same-named builtins, implementations, and + // unsupported ambient declaration shapes individually. + symbolsByName.set(binding.name, new Set()); } continue; } @@ -2793,10 +2805,42 @@ export function lowerFfiCall(L: Lowerer, expr: ts.CallExpression): IrExpr | null // binding. Poison the statement without duplicating that diagnostic. if (validSymbols === undefined) throw new PoisonError(); if (!validSymbols.has(symbol)) { - // TypeScript resolved this call to a distinct local declaration. - // The manifest owns only the exact validated ambient binding; a - // same-named function with a body remains ordinary scriptc code. - return null; + if (L.libraryCallbacks) { + const declarations = L.checker.declarationsOf(symbol); + const programDeclarations = declarations.filter( + (decl) => !decl.getSourceFile().isDeclarationFile, + ); + const programAmbient = + programDeclarations.length > 0 && + programDeclarations.every( + (decl) => + (ts.getCombinedModifierFlags(decl as ts.Declaration) & + ts.ModifierFlags.Ambient) !== 0, + ); + if (programAmbient) { + // A called, program-authored ambient with a configured channel + // name is not unused capacity. Validate the resolved symbol now + // so unsupported declaration forms (`declare const cb: ...`) + // refuse SC4024 instead of silently dropping the call. A valid + // declaration missed by the up-front syntax walk may proceed as + // the callback binding after this exact-symbol check. + const diagnostic = ffiDeclarationDiagnostic(L, binding, symbol, loc); + if (diagnostic !== null) { + L.pushDiag(diagnostic); + throw new PoisonError(); + } + } else { + // Declaration-file ambients (including standard-library + // builtins) and same-named program implementations remain their + // ordinary TypeScript bindings; the profile does not claim them. + return null; + } + } else { + // TypeScript resolved this call to a distinct local declaration. + // The manifest owns only the exact validated ambient binding; a + // same-named function with a body remains ordinary scriptc code. + return null; + } } } else { const diagnostic = ffiDeclarationDiagnostic(L, binding, symbol, loc); diff --git a/tests/harness/library-callbacks.test.ts b/tests/harness/library-callbacks.test.ts index 37cd5d71c..db45dfb1a 100644 --- a/tests/harness/library-callbacks.test.ts +++ b/tests/harness/library-callbacks.test.ts @@ -39,8 +39,11 @@ * teaching; a callback-free profile keeps the * ambient ReferenceError lowering (the standing * guarantee), and an unused declared channel is - * legal capacity (SC4001 profile shapes live in - * library-profile.test.ts) + * legal capacity; declaration-file ambient names + * stay builtins rather than becoming channels, and + * unsupported program-authored ambient shapes + * refuse instead of disappearing (SC4001 profile + * shapes live in library-profile.test.ts) * CB7 composition a runtime-localized AND thread-instanced * archive with channels: two embedder threads * register different contexts, chunks route to @@ -308,6 +311,13 @@ survived, sink_calls=1 expect(diags[0]!.message).toContain("does not fit profile class 'bytes'"); }); + test("CB6: a called function-valued ambient refuses SC4024 instead of disappearing", async () => { + const diags = await buildRefusal(emission, { tag: "ambient-const", entry: "lib_ambient_const.ts" }); + expect(diags.map((d) => d.code)).toEqual(["SC4024"]); + expect(diags[0]!.message).toContain("library callback 'orphan'"); + expect(diags[0]!.message).toContain("does not resolve exclusively to signature-only function declarations"); + }); + platformTest("CB6: a callback-free profile keeps the ambient ReferenceError lowering", async () => { // The standing guarantee's behavioral half: without a callbacks // section the same signature-only declaration keeps Node's @@ -346,6 +356,20 @@ stream(3,7) = 42 `); }); + platformTest("CB6: declaration-file ambient names remain builtins or unused capacity", async () => { + const { archive, outDir } = await buildLibrary(emission, { + tag: "builtin-names", + profileFile: "profile_builtins.json", + }); + const probe = buildProbe("probe_builtins.c", archive, outDir); + const run = runProbe(probe); + expect(run.signal).toBeNull(); + expect(run.status).toBe(0); + expect(run.stdout).toBe(`finite: 0 +nan: 1 +`); + }); + localizationTest("CB7: localized + thread-instanced channels route per instance", async () => { const { archive, outDir } = await buildLibrary(emission, { tag: "threads", profileFile: "profile_t.json" }); const probe = buildProbe("probe_threads.c", archive, outDir, { pthread: true }); diff --git a/tests/library-mode/callbacks/lib_ambient_const.ts b/tests/library-mode/callbacks/lib_ambient_const.ts new file mode 100644 index 000000000..0a05766e6 --- /dev/null +++ b/tests/library-mode/callbacks/lib_ambient_const.ts @@ -0,0 +1,18 @@ +// CB6 refusal fixture: the configured channel name resolves to a +// program-authored ambient function VALUE, not the supported signature-only +// function-declaration form. The call must refuse SC4024; it must never be +// mistaken for unused capacity and silently erased. +declare const orphan: (x: number) => void; + +export function stream(n: number, base: number): number { + return n + base; +} + +export function askHost(x: number): number { + return x; +} + +export function pokeOrphan(): number { + orphan(7); + return 0; +} diff --git a/tests/library-mode/callbacks/lib_builtins.ts b/tests/library-mode/callbacks/lib_builtins.ts new file mode 100644 index 000000000..8704986f1 --- /dev/null +++ b/tests/library-mode/callbacks/lib_builtins.ts @@ -0,0 +1,7 @@ +// CB6 builtin-name collision fixture: profile channels may share names with +// declaration-file ambients without claiming those bindings. isNaN must keep +// its standard-library lowering; parseInt is unreferenced channel capacity and +// must not be validated against lib.d.ts's optional-radix signature. +export function checkBuiltin(x: number): boolean { + return isNaN(x); +} diff --git a/tests/library-mode/callbacks/probe_builtins.c b/tests/library-mode/callbacks/probe_builtins.c new file mode 100644 index 000000000..847839807 --- /dev/null +++ b/tests/library-mode/callbacks/probe_builtins.c @@ -0,0 +1,15 @@ +/* CB6 builtin-name collision probe: neither channel is registered. If the + * profile incorrectly claims lib.d.ts's isNaN binding, this call traps; + * ordinary builtin lowering returns false/true and reaches both prints. */ +#include +#include + +extern void cbb_init(void); +extern uint8_t cbb_check_builtin(double x); + +int main(void) { + cbb_init(); + printf("finite: %u\n", (unsigned)cbb_check_builtin(123)); + printf("nan: %u\n", (unsigned)cbb_check_builtin(0.0 / 0.0)); + return 0; +} diff --git a/tests/library-mode/callbacks/profile_builtins.json b/tests/library-mode/callbacks/profile_builtins.json new file mode 100644 index 000000000..a95aaee72 --- /dev/null +++ b/tests/library-mode/callbacks/profile_builtins.json @@ -0,0 +1,21 @@ +{ + "profile_format": 1, + "name": "conformance-callback-builtins", + "entry": "lib_builtins.ts", + "emission": "llvm", + "abi": { + "prefix": "cbb_", + "init_symbol": "cbb_init", + "sink_register_symbol": "cbb_set_panic_sink", + "collect_symbol": null, + "result_reset_symbol": null, + "callback_register_symbol": "cbb_set_callback" + }, + "callbacks": [ + { "name": "isNaN", "params": ["f64"], "returns": "bool" }, + { "name": "parseInt", "params": ["string", "u32"], "returns": "f64" } + ], + "exports": [ + { "export": "checkBuiltin", "symbol": "cbb_check_builtin", "params": ["f64"], "returns": "bool" } + ] +}