diff --git a/CHANGELOG.md b/CHANGELOG.md index 51b58196..5fa03bf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Breaking +- An `external fn`'s identity is now the **declaration** (the declaring `DefId`), resolved in the scope the call is written in — the declaring file, and the `spec` block within it — rather than the bare name looked up program-wide. Two shapes now compile that did not: two files may each declare `external fn f` and bind it to a different module, with each file's calls reaching its own binding; and a file's own bound extern is no longer broken by an unrelated same-named declaration elsewhere in the program. Two now error that did not: a `use … from` clause no longer reaches across files to bind another file's declaration, so one naming an extern its own file does not declare is the dangling-import error `ExternImportNotDeclared`; and a free function and an `external fn` sharing one bare name is the new `ExternFunctionNameCollision`, whether the two sit in one file or in different files: one bare name may not denote both a local function and an external declaration in one program. Each call in such a program resolves — to the declaration its own file names — and the pair is rejected as a rule about the spelling, because a call site does not say whether its callee is compiled here or linked in. `AmbiguousExternModule` is correspondingly a within-file rule — one name, one module, per file — and `extern_origins()` deduplicates on `(logical_module, export_field, decl)` rather than on `(module, field)`, so every declaration is still validated against the library's signature instead of only the first per `(module, field)` pair. Migration: rename either half of a colliding free-function/`external fn` pair, and give each file the `use … from` clause binding its own declaration; a program that does not reuse an extern name across files is unaffected ([#423]) - A specification may now call a **bound** `external fn`, so a linked external `.wasm` is reasonable about in proof mode: the call becomes a `T_app`/`HA_app_ok` naming the body the static-merge linker splices in (`mathlib.double` for `use { double } from mathlib;`), which is the symbol the merge writes into the name section, and `wasm-to-v` resolves it against the **linked** module. Previously every extern call in a spec was **P005** ("external functions carry no verified body"), which made the linker envelope unusable for its stated purpose — an external could be linked and executed but nothing could be claimed about it. The declaration is resolved by `DefId`, not by name: two `external fn`s may share a name across scopes (a bound one at file scope, an unbound one inside a `spec`) and only the declaration the call site actually names decides whether it has a linked body, so a name-keyed lookup would hand a spec-inner declaration the top-level one's origin and name a merged body the call does not reach (`A024` resolves unbound-extern calls through the same scope walk and rejects such a call first, so this is defense in depth everywhere except a pipeline that skips analysis — which the proof-mode test gates are). An **unbound** extern keeps **P005**, with a message naming the missing `use … from` binding rather than the extern; through `infc`/`infs` that message is not what a user sees, since **A024** fires on the same call first — P005 is what an embedder or an analysis-skipping pipeline gets. Two `wasm-to-v` rejections come with it, both of which could previously produce a well-formed but meaningless `.v`: an obligation whose application arity differs from the resolved function's parameter count is now a hard error (`T_app`'s arguments are a `seq term`, so a wrong-width application elaborates and passes `coqc` while describing a different function), and an obligation naming a function the module *imports* now says the merge has not run instead of reporting the symbol as unknown. Unaffected: every obligation that names no extern is byte-identical, and no committed `.wasm`/`.wat`/`.v` golden moves. Known limitation, unchanged and now pinned by a test that asserts it: a **cross-file** `T_app` symbol still does not resolve, because code generation writes bare item names into the name section while the obligation writes the file-qualified `FnKey::Display` ([#363]) - Proof mode now compiles `exists`- and `unique`-quantified spec functions end to end instead of rejecting them as **P001**. The body is *reachability-lowered*: compiled to vanilla WASM with one hidden trailing choice parameter per scalar `@` (name-section label `__choice{k}`; narrow-type domain normalization kept) and `assume`/`assert` as trap-on-false filters; the function is *retained* in the emitted `.v` module record — `forall`/plain spec functions stay omitted, and every executable reference to a retained function is rejected (it is the subject of a judgment, not a callable) — and its obligation is emitted as a `reachability_spec` record (`reach_func`/`reach_entry_arity`/`reach_visible_locs`/`reach_payload`) under a kind-selected theorem: `ValidExistsSpec` for `exists`, `ValidUniqueSpec` for `unique`, with the preamble import gaining ` Exists` only in reachability-bearing modules. The payload binds each `@` to its choice parameter's frame slot — no `HA_ex` binder, no `HA_has_type` guard: the judgment runs the retained body, so the frame supplies value and typing — while `_specs`/`ValidSpec` stays unconditional per spec name and forall-only output is byte-identical. The quantifier kind travels in `inference.hspecs` wire **v2** (per-entry kind byte plus reachability metadata; encode is always v2, decode rejects v1 — the section is proof-mode intermediate data, so recompile rather than migrate). **P001** narrows to standalone `assume` bodies, with a message explaining why an `assume` body states no property; new fatal diagnostics **P011** (any spec body calling an `exists`/`unique` spec function — the callee's compiled form carries hidden choice parameters no call site supplies) and **P012** (an anonymous `@` in a `unique` body — a choice nothing names cannot distinguish source-visible exit states; bind it first with `let c: i32 = @;`). Compound `@` in a reachability body keeps **P008** with a reachability-specific message; reachability bodies must be void and `return`-free (contract-forced: the downstream judgment reduces the retained body without an activation frame, so a `return` could never take a step), enforced by a codegen pre-scan even on analysis-skipping pipelines. `unique` compares source-visible exit states — entry parameters plus named `let x = @` choices, declared in `reach_visible_locs`; a `unique`-quantified *body* is a deliberate extension ahead of the language-spec amendment, while nested `unique` *blocks* stay **P002**. Reachability translation hard-depends on the WASM name section, and the obligations are dischargeable only for import-free modules (the always-link pipeline satisfies this). Also fixes the A002/A005 message article (`a 'exists' block` → `an 'exists' block`). Full contract in `core/wasm-to-v/ROCQ_CONTRACT.md` ([#354]) - New fatal proof-mode diagnostic **P010**: a spec function whose obligation collapses to the vacuous `HA_true` now aborts code generation instead of being emitted. Such an obligation is discharged by any proof without reading the program, so a green `Qed` over it carried zero verification content and no warning said so — `spec Caller { fn caller() -> i32 { return helper(); } }` emitted `Definition …_hspec1 : hassert := HA_true.` and the meaning the author wrote ("caller equals helper") was simply dropped. The check is the translated result, not the body shape: `HAssert::and`/`imp`/`or`/`ex` absorb `⊤`, so every vacuity path collapses to exactly `HA_true` and one equality catches them all — an empty or assert-free body, a body that only computes (`return`, pure `let`/`const`), a trailing `assume` (`Imp(p, ⊤) = ⊤`), and an `if` whose branches all vacuate, several of which look like they contribute. The message names what the body claimed and what to write instead. A spec function may still `return` — a body that also asserts is unaffected. Consequence: a helper that only computes can no longer live inside a `spec` block; move it to file scope, where a spec function still applies it as a `T_app`. A plain spec *method* keeps its helper exemption because it produces no obligation either way, so the same helper wrapped in a spec-inner struct still compiles — but a plain method that *states* a property — one carrying an `assert` at any depth, a non-deterministic block that asserts nothing excluded — now raises **P009** rather than being dropped with no output at all, which is the widening that closes the worst silent path in the area. Unaffected: compile mode, which has no obligations; every spec function that already stated a property, whose obligation is byte-identical. Migration: give the spec function an `assert`, or move it out of the `spec` block ([#356]) @@ -621,3 +622,4 @@ Initial tagged release. [#416]: https://github.com/Inferara/inference/issues/416 [#355]: https://github.com/Inferara/inference/issues/355 [#420]: https://github.com/Inferara/inference/issues/420 +[#423]: https://github.com/Inferara/inference/issues/423 diff --git a/apps/infs/tests/cli_integration.rs b/apps/infs/tests/cli_integration.rs index 8a0b11d6..dee50539 100644 --- a/apps/infs/tests/cli_integration.rs +++ b/apps/infs/tests/cli_integration.rs @@ -2384,15 +2384,23 @@ fn logged_probes(log: &std::path::Path) -> Vec { /// what these tests link must be what `infc` emits today, or a change to the /// module shape would leave them linking a stale fixture. fn build_arith_library(infc_path: &std::path::Path) -> Vec { + build_library(infc_path, "arith", ARITH_LIB_SRC) +} + +/// Compiles `source` on its own with the real `infc` and returns the `.wasm` +/// bytes, for vendoring into a dependent project as a `[wasm-dependencies]` +/// entry. +fn build_library(infc_path: &std::path::Path, name: &str, source: &str) -> Vec { let lib = assert_fs::TempDir::new().unwrap(); - lib.child("arith.inf").write_str(ARITH_LIB_SRC).unwrap(); + let entry = format!("{name}.inf"); + lib.child(&entry).write_str(source).unwrap(); let mut cmd = Command::new(infc_path); - cmd.current_dir(lib.path()).arg("arith.inf"); + cmd.current_dir(lib.path()).arg(&entry); cmd.assert().success(); - std::fs::read(lib.child("out").child("arith.wasm").path()) - .expect("infc must produce out/arith.wasm for the library") + std::fs::read(lib.child("out").child(format!("{name}.wasm")).path()) + .unwrap_or_else(|e| panic!("infc must produce out/{name}.wasm for the library: {e}")) } /// Scaffolds a project that binds the `arith` external: the manifest declares the @@ -2800,6 +2808,76 @@ fn project_run_executes_a_linked_manifest_dependency() { ); } +/// Two files of one project each declare `external fn scale` and bind it to a +/// *different* library. Executed end to end through the project front end. +/// +/// The project route is the one seam where the merged arena is built from a real +/// source tree rather than assembled in a test, and the whole program is one +/// `infc` invocation — so both files' `scale` calls pass through a single +/// compilation with a single import table. Nothing before execution distinguishes +/// a per-file resolution from a program-wide one: the declarations agree on name +/// and signature, so either wiring type-checks, links and validates. Only the +/// value says which library ran. `libA` multiplies by 999 and `libB` by 7, and +/// `main` reports both at once as `1998 * 100 + 14`. +#[test] +fn project_run_resolves_a_same_named_extern_per_file() { + let Some(infc_path) = require_infc_and_wasmtime() else { + return; + }; + + let lib_a = build_library( + &infc_path, + "libA", + "pub fn scale(a: i32) -> i32 {\n return a * 999;\n}\n", + ); + let lib_b = build_library( + &infc_path, + "libB", + "pub fn scale(a: i32) -> i32 {\n return a * 7;\n}\n", + ); + + let temp = assert_fs::TempDir::new().unwrap(); + scaffold_project_with_manifest( + &temp, + "demo", + "use sib;\n\n\ + external fn scale(a: i32) -> i32;\n\ + use { scale } from libA;\n\n\ + pub fn main() -> i32 {\n return scale(2) * 100 + sib::via_b(2);\n}\n", + "[wasm-dependencies]\n\ + libA = { path = \"libs/libA.wasm\" }\n\ + libB = { path = \"libs/libB.wasm\" }\n", + ); + temp.child("src") + .child("sib.inf") + .write_str( + "external fn scale(a: i32) -> i32;\n\ + use { scale } from libB;\n\n\ + pub fn via_b(x: i32) -> i32 {\n return scale(x);\n}\n", + ) + .unwrap(); + temp.child("libs") + .child("libA.wasm") + .write_binary(&lib_a) + .unwrap(); + temp.child("libs") + .child("libB.wasm") + .write_binary(&lib_b) + .unwrap(); + + let mut cmd = Command::new(assert_cmd::cargo::cargo_bin!("infs")); + cmd.env("INFC_PATH", &infc_path) + .env_remove("INFERENCE_WASM_LIB_PATH") + .current_dir(temp.path()) + .arg("run"); + let stdout = stdout_of(&cmd.assert().success()); + assert!( + stdout.contains("199814"), + "each file's `scale` must reach its own library (1998 via libA, 14 via libB); \ + 201798 would mean both calls funnelled into libA, got:\n{stdout}" + ); +} + /// Scaffolds a throwaway project with a logging `infc` stub, runs `infs run` /// with `args` from the project root, and returns the argv the stub recorded. /// diff --git a/book/src/external-functions-and-wasm-linking.md b/book/src/external-functions-and-wasm-linking.md index 5eecea16..60d0a21e 100644 --- a/book/src/external-functions-and-wasm-linking.md +++ b/book/src/external-functions-and-wasm-linking.md @@ -99,8 +99,12 @@ example above produces: Imported functions occupy the lowest WASM function indices. The local `add_three` is shifted to index 1 (after the one import at index 0). The call target `call 0` -is the import index, resolved statically from the `extern_name_to_idx` table built -during the pre-scan phase. +is the import index, resolved statically during the pre-scan phase: the compiler +resolves the callee name to the `external fn` declaration in scope where the call +is written — its file, and the `spec` block enclosing it — and takes the import +that declaration reserved. Identity is the declaration, not the name, so two files +may each declare `scale` and bind it to a different module, and each file's calls +reach its own. ## The Link Step diff --git a/core/analysis/README.md b/core/analysis/README.md index 4567847e..19201310 100644 --- a/core/analysis/README.md +++ b/core/analysis/README.md @@ -91,7 +91,7 @@ These rules cover constructs that are valid in the type system but cannot yet be | A019 | `ArrayIndex64Bit` | error | 64-bit integer used as an array index | | A022 | `LiteralOutOfRange` | error | numeric literal is outside the valid range for the type its position gave it | | A023 | `UzumakiInReassignment` | error | uzumaki (`@`) used in a variable reassignment (only `let` initializers are supported) | -| A024 | `ExternFunctionCall` | error | call to an external (`extern`) function (not yet implemented in codegen) | +| A024 | `ExternFunctionCall` | error | call to an **unbound** `external fn` — one no `use … from` clause binds to a source module | | A026 | `NestedCompoundDepth` | error | struct field is itself a nested compound type beyond one level of nesting | | A027 | `UzumakiOnNestedStruct` | error | uzumaki (`@`) assigned to a struct whose fields include another struct or an array of structs | | A028 | `UzumakiOnStructInArray` | error | uzumaki (`@`) assigned to an array whose element type is a struct | @@ -178,7 +178,7 @@ A045 rejects *values* of a struct with no fields. Such a struct occupies zero by Rejecting a field-less struct as the type of a *field* is what closes the hole: a struct all of whose fields are zero-sized would itself be zero-sized, so forbidding a zero-sized field collapses that composition into the base case. In an accepted program a struct is therefore zero-sized if and only if it has no fields, which lets the predicate be `fields.is_empty()` plus array recursion — no transitive size computation, no visited set, no cycle handling. With every value-introducing position rejected, assignments, reads, and method calls on such values need no checks of their own (each requires a binding, parameter, or field that is already rejected), so a program reports one diagnostic per offending declaration rather than one per use. A module-scope `const` is checked here in its own right rather than left to A032, which rejects *every* top-level `const` as not yet implemented: A032 is a gate on an unimplemented feature, and a closure resting on it would go silently incomplete the day that feature lands. Both fire on such a declaration; there is no cross-rule suppression. -*Declaring* a field-less struct stays legal. A field-less struct with associated functions is the supported method-namespace idiom (`E::helper()`) and compiles unchanged; the `self` receiver is rejected because once no value of the struct can exist the method is uncallable by construction, and the fix — dropping `self` — produces exactly that idiom. `external fn` signatures are checked for their ABI surface rather than for the closure (A024 rejects every call to an extern function, so no value can flow through one). Two documented non-scopes: generics, since a type parameter never resolves to a struct, so a generic signature (`fn id T'(x: T) -> T`) is outside the predicate — nothing is missed by that today, because the compiler does not monomorphize and codegen rejects a generic type outright, so there is no instantiation at a field-less struct to check; and local type aliases, which are non-transparent in Inference and so are a dead end rather than a route to a value. +*Declaring* a field-less struct stays legal. A field-less struct with associated functions is the supported method-namespace idiom (`E::helper()`) and compiles unchanged; the `self` receiver is rejected because once no value of the struct can exist the method is uncallable by construction, and the fix — dropping `self` — produces exactly that idiom. `external fn` signatures are checked for their ABI surface rather than for the closure (A024 rejects a call to an *unbound* extern outright, and a bound extern's declared parameter and return types are themselves in scope above, so no field-less value flows through either). Two documented non-scopes: generics, since a type parameter never resolves to a struct, so a generic signature (`fn id T'(x: T) -> T`) is outside the predicate — nothing is missed by that today, because the compiler does not monomorphize and codegen rejects a generic type outright, so there is no instantiation at a field-less struct to check; and local type aliases, which are non-transparent in Inference and so are a dead end rather than a route to a value. ### Spaced Negative Literals (errors) diff --git a/core/analysis/src/rules/extern_function_call.rs b/core/analysis/src/rules/extern_function_call.rs index 24393195..fc6bf156 100644 --- a/core/analysis/src/rules/extern_function_call.rs +++ b/core/analysis/src/rules/extern_function_call.rs @@ -16,14 +16,20 @@ //! declaration is unbound. A purely name-keyed check would let an unbound //! same-named declaration poison every call to a bound extern (round-2 H-1). //! +//! Resolution itself is not implemented here. The walk below tracks only +//! *where* each body sits — the file it belongs to and the `spec` enclosing it, +//! if any — and hands that scope to +//! [`ExternIndex`](inference_type_checker::ExternIndex), the whole-program +//! index type checking already built. Sharing that index is what keeps this +//! rule and the specification translator from disagreeing about which +//! declaration a call names. +//! //! NOTE: This rule only matches direct calls by name (`foo()`). External //! functions cannot currently be struct members or passed as values, so //! name-based matching within a scope is sufficient. If the language later //! allows extern functions in structs or as first-class values, this rule //! will need to be extended. -use std::collections::HashMap; - use inference_ast::arena::AstArena; use inference_ast::ids::DefId; use inference_ast::nodes::{Def, Expr}; @@ -43,14 +49,13 @@ crate::rule! { fn check(ctx: &TypedContext) -> Vec { let arena = ctx.arena(); let mut errors = Vec::new(); - let mut scopes: Vec> = Vec::new(); for source_file in ctx.source_files() { check_defs( arena, ctx, &source_file.module_path, + None, &source_file.defs, - &mut scopes, &mut errors, ); } @@ -58,72 +63,55 @@ crate::rule! { } } -/// Walks the definition tree, maintaining a stack of extern declarations in -/// scope, and flags every call that resolves to an *unbound* extern. +/// Walks the definitions of one scope, flagging every call in their bodies that +/// resolves to an *unbound* extern. /// /// A file's top level and a `spec` inside it are the only two places an -/// `external fn` can be declared, and each pushes the declarations it introduces -/// as a new scope layer before the bodies at that level are checked, so a -/// spec-inner `external fn` shadows a same-named top-level one for calls inside -/// that spec. The layer is popped on exit, keeping sibling specs isolated from -/// one another. Specs do not nest, so the stack is at most two deep. -fn check_defs<'a>( - arena: &'a AstArena, +/// `external fn` can be declared, so a body's scope is fully described by the +/// file's `module_path` plus `spec` — `None` at the top level, the spec's name +/// inside one. Specs do not nest, so the recursion descends at most one level +/// and sibling specs stay isolated by construction. +fn check_defs( + arena: &AstArena, ctx: &TypedContext, module_path: &[String], + spec: Option<&str>, defs: &[DefId], - scopes: &mut Vec>, errors: &mut Vec, ) { - scopes.push(collect_extern_decls(arena, defs)); for &def_id in defs { match &arena[def_id].kind { Def::Function { body, .. } => { - check_function_body(arena, ctx, module_path, *body, scopes, errors); + check_function_body(arena, ctx, module_path, spec, *body, errors); } Def::Struct { methods, .. } => { for &method_id in methods { if let Def::Function { body, .. } = &arena[method_id].kind { - check_function_body(arena, ctx, module_path, *body, scopes, errors); + check_function_body(arena, ctx, module_path, spec, *body, errors); } } } - Def::Spec { defs, .. } => { - check_defs(arena, ctx, module_path, defs, scopes, errors); + Def::Spec { name, defs, .. } => { + check_defs( + arena, + ctx, + module_path, + Some(arena[*name].name.as_str()), + defs, + errors, + ); } _ => {} } } - scopes.pop(); -} - -/// Records the `external fn` declarations introduced directly by `defs`, -/// mapping each extern name to its declaring [`DefId`]. Keeps the first -/// declaration for a name; a same-name redeclaration in one scope is a type -/// error caught earlier, so the choice is immaterial to a valid program. -fn collect_extern_decls<'a>(arena: &'a AstArena, defs: &[DefId]) -> HashMap<&'a str, DefId> { - let mut decls = HashMap::default(); - for &def_id in defs { - if let Def::ExternFunction { name, .. } = &arena[def_id].kind { - decls.entry(arena[*name].name.as_str()).or_insert(def_id); - } - } - decls -} - -/// Resolves a callee name against the scope stack, innermost first, returning -/// the declaring [`DefId`] of the nearest `external fn` of that name, or `None` -/// if the name does not resolve to any extern in scope (a regular function). -fn resolve_extern_decl(scopes: &[HashMap<&str, DefId>], name: &str) -> Option { - scopes.iter().rev().find_map(|scope| scope.get(name).copied()) } fn check_function_body( arena: &AstArena, ctx: &TypedContext, module_path: &[String], + spec: Option<&str>, body: inference_ast::ids::BlockId, - scopes: &[HashMap<&str, DefId>], errors: &mut Vec, ) { walker::walk_block_stmts(arena, body, &mut |stmt_id| { @@ -133,7 +121,7 @@ fn check_function_body( && let Expr::Identifier(ident_id) = &arena[*function].kind { let callee_name = &arena[*ident_id].name; - if let Some(decl) = resolve_extern_decl(scopes, callee_name) + if let Some(decl) = ctx.extern_index().lookup(module_path, spec, callee_name) && ctx.extern_origin_by_decl(decl).is_none() { errors.push(LabeledDiagnostic::new( diff --git a/core/inference/src/wasm_link/driver.rs b/core/inference/src/wasm_link/driver.rs index d00dbdf4..89e4381f 100644 --- a/core/inference/src/wasm_link/driver.rs +++ b/core/inference/src/wasm_link/driver.rs @@ -162,6 +162,12 @@ impl std::error::Error for ExternalResolutionError {} /// A program with no externs yields an empty vector, and the build proceeds /// without invoking the linker. /// +/// Every bound *declaration* is signature-validated, including two declarations +/// in different files that name the same module and field. The linker satisfies +/// an import on `(module, field)` alone and compares no signatures, so a +/// declaration whose signature never reached validation here would be linked +/// against a library it does not match. +/// /// # Errors /// /// Returns an [`ExternalResolutionError`] if any extern fails to resolve, diff --git a/core/type-checker/src/errors.rs b/core/type-checker/src/errors.rs index 2fd36b4f..64561bd1 100644 --- a/core/type-checker/src/errors.rs +++ b/core/type-checker/src/errors.rs @@ -923,6 +923,33 @@ pub enum TypeCheckError { module: String, location: Location, }, + + /// A top-level `external fn` and a top-level function share one bare name. + /// + /// Not a resolution failure. An `external fn` is identified by its + /// declaration and a bare call resolves in the scope it is written in, so + /// every call in such a program reaches exactly one callee and the program + /// would compile and run correctly. It is rejected as a rule about the name: + /// a local function that shadows a foreign-boundary declaration is hard to + /// read, because nothing at a call site distinguishes a call this program + /// compiles from a call that leaves it. The rule is program-wide for the + /// same reason — the two spellings are indistinguishable to a reader + /// wherever the declarations sit. + /// + /// Reported against the `external fn`, with the colliding function named as + /// a note: `function_file` is the [`inference_ast::nodes::file_label`] of the + /// file defining it, `None` for the entry file. Both sites appear because + /// neither is at fault on its own — renaming either one resolves it. + #[error( + "{location}: `external fn {name}` and the function `{name}` share one name; a name is either a local function or an `external fn`, never both in one program, so rename one\nnote: the function `{name}` is defined at {function_location} in {}", + function_file.as_ref().map_or_else(|| "the entry file".to_string(), |file| format!("file `{file}`")) + )] + ExternFunctionNameCollision { + name: String, + location: Location, + function_location: Location, + function_file: Option, + }, } impl TypeCheckError { @@ -994,6 +1021,7 @@ impl TypeCheckError { | TypeCheckError::DuplicateEnumVariant { location, .. } | TypeCheckError::AmbiguousExternModule { location, .. } | TypeCheckError::ExternImportNotDeclared { location, .. } + | TypeCheckError::ExternFunctionNameCollision { location, .. } | TypeCheckError::SpecFunctionShadowsTopLevel { location, .. } => location, } } @@ -2016,6 +2044,39 @@ mod tests { ); } + /// A second location renders as a `line:col` alone, so the file it belongs + /// to has to be spelled out or a cross-file note reads as a same-file one. + #[test] + fn display_extern_function_name_collision_in_a_named_file() { + let err = TypeCheckError::ExternFunctionNameCollision { + name: "scale".to_string(), + location: test_location(), + function_location: Location::new(40, 45, 7, 1, 7, 6), + function_file: Some("lib::math".to_string()), + }; + assert_eq!( + err.to_string(), + "1:5: `external fn scale` and the function `scale` share one name; a name is either a local function or an `external fn`, never both in one program, so rename one\nnote: the function `scale` is defined at 7:1 in file `lib::math`" + ); + } + + /// The entry file has no label, so it is named in words rather than left + /// unqualified — an unqualified note beside a file-prefixed error would read + /// as pointing into that same file. + #[test] + fn display_extern_function_name_collision_in_the_entry_file() { + let err = TypeCheckError::ExternFunctionNameCollision { + name: "scale".to_string(), + location: test_location(), + function_location: Location::new(40, 45, 7, 1, 7, 6), + function_file: None, + }; + assert_eq!( + err.to_string(), + "1:5: `external fn scale` and the function `scale` share one name; a name is either a local function or an `external fn`, never both in one program, so rename one\nnote: the function `scale` is defined at 7:1 in the entry file" + ); + } + // Tests for CompoundReturnCallInAssignment and MethodCallChainOnCompoundReturn // migrated to analysis rules A017 and A018. } diff --git a/core/type-checker/src/extern_index.rs b/core/type-checker/src/extern_index.rs new file mode 100644 index 00000000..40aec9be --- /dev/null +++ b/core/type-checker/src/extern_index.rs @@ -0,0 +1,129 @@ +//! Scope-aware resolution of `external fn` declarations. +//! +//! Every phase that lowers, checks or reasons about a call to an `external fn` +//! must first answer the same question: *which* declaration does this bare name +//! mean at this point in the program? [`ExternIndex`] answers it once, for the +//! whole program, so those phases cannot drift apart. + +use inference_ast::arena::AstArena; +use inference_ast::ids::DefId; +use inference_ast::nodes::Def; +use rustc_hash::FxHashMap; + +/// The `external fn` declarations of a program, keyed by the scope that +/// introduces them. +/// +/// Resolution must be by *declaration*, not by name. Two `external fn`s may +/// share a name — a bound top-level `sum` and an unbound spec-inner `sum` — and +/// only the declaration a given call site actually names decides whether that +/// call has a linked body and which module it comes from. A `use … from` clause +/// binds top-level declarations only, so a spec-inner one is never bound; a +/// name-keyed lookup would hand it the top-level declaration's origin and emit +/// an obligation naming a merged body the call does not reach. +/// +/// Every consumer resolves through this one index, so their answers agree by +/// construction rather than by separate walks happening to coincide: `A024` +/// decides whether a call reaches a *bound* extern, and the specification +/// translator decides which declaration an obligation names. Through the full +/// pipeline `A024` rejects an unbound-extern call before translation runs, +/// which makes the agreement defense in depth rather than the sole guard; a +/// pipeline that skips analysis, as the proof-mode test gates do, has nothing +/// else. +/// +/// Two scopes exhaust the language: a file's top level and a `spec` block +/// inside it. Specs do not nest, so an inner scope is keyed by its spec name +/// and a lookup is a two-step walk rather than a stack. +#[derive(Default)] +pub struct ExternIndex { + decls: FxHashMap>, +} + +/// A file, or a `spec` block within it — the two places an `external fn` may be +/// declared. +#[derive(PartialEq, Eq, Hash)] +struct ExternScope { + module_path: Vec, + spec: Option, +} + +impl ExternIndex { + /// Collects every `external fn` declaration in the program, in the scope + /// that introduces it. A scope that declares none records no entry. + #[must_use = "the index is the return value"] + pub fn build(arena: &AstArena) -> Self { + let mut decls: FxHashMap> = FxHashMap::default(); + for file in arena.source_files() { + let spec_scopes = file + .defs + .iter() + .filter_map(|&def_id| match &arena[def_id].kind { + Def::Spec { name, defs, .. } => { + Some((Some(arena[*name].name.clone()), defs.as_slice())) + } + _ => None, + }); + for (spec, defs) in std::iter::once((None, file.defs.as_slice())).chain(spec_scopes) { + let externs = extern_decls(arena, defs); + if !externs.is_empty() { + decls.insert( + ExternScope { + module_path: file.module_path.clone(), + spec, + }, + externs, + ); + } + } + } + Self { decls } + } + + /// The `external fn` a bare `name` resolves to in the file at + /// `module_path`, innermost scope first, or `None` when the name is not an + /// extern there. + /// + /// `spec` names the `spec` block the point of use sits in, and is `None` at + /// the file's top level: a top-level call cannot see a spec-inner + /// declaration, so passing `None` consults the top-level scope alone. + #[must_use = "this is a pure lookup with no side effects"] + pub fn lookup(&self, module_path: &[String], spec: Option<&str>, name: &str) -> Option { + match spec { + Some(spec) => self + .in_scope(module_path, Some(spec), name) + .or_else(|| self.lookup_top_level(module_path, name)), + None => self.lookup_top_level(module_path, name), + } + } + + /// The top-level `external fn` named `name` in the file at `module_path`, + /// ignoring every `spec` block within that file. + /// + /// This is the scope a `use … from` clause reaches: the clause is + /// file-scoped and binds top-level declarations only, so a same-named + /// spec-inner declaration must not be found here. + #[must_use = "this is a pure lookup with no side effects"] + pub fn lookup_top_level(&self, module_path: &[String], name: &str) -> Option { + self.in_scope(module_path, None, name) + } + + fn in_scope(&self, module_path: &[String], spec: Option<&str>, name: &str) -> Option { + let scope = ExternScope { + module_path: module_path.to_vec(), + spec: spec.map(str::to_string), + }; + self.decls.get(&scope)?.get(name).copied() + } +} + +/// The `external fn` declarations `defs` introduces directly, keeping the first +/// of a repeated name — a repeat within one scope is a type error reported +/// earlier, so which one wins cannot matter to a valid program. +fn extern_decls(arena: &AstArena, defs: &[DefId]) -> FxHashMap { + let mut externs = FxHashMap::default(); + for &def_id in defs { + if let Def::ExternFunction { name, .. } = &arena[def_id].kind { + externs.entry(arena[*name].name.clone()).or_insert(def_id); + } + } + externs +} diff --git a/core/type-checker/src/lib.rs b/core/type-checker/src/lib.rs index c6e85ea6..429ef34c 100644 --- a/core/type-checker/src/lib.rs +++ b/core/type-checker/src/lib.rs @@ -105,11 +105,13 @@ use crate::{errors::TypeCheckError, type_checker::TypeChecker, typed_context::Ty mod definition_graph; pub mod errors; +mod extern_index; mod symbol_table; mod type_checker; pub mod type_info; pub mod typed_context; +pub use extern_index::ExternIndex; pub use symbol_table::{EnumInfo, ExternOrigin, StructFieldInfo, StructInfo}; pub use typed_context::MethodMetadata; diff --git a/core/type-checker/src/symbol_table.rs b/core/type-checker/src/symbol_table.rs index 08423f08..4e3e3756 100644 --- a/core/type-checker/src/symbol_table.rs +++ b/core/type-checker/src/symbol_table.rs @@ -33,7 +33,7 @@ //! Functions without an explicit return type default to the unit type, //! represented as `TypeInfo { kind: TypeInfoKind::Unit, type_params: vec![] }`. -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; use std::path::PathBuf; use anyhow::bail; @@ -1917,15 +1917,28 @@ impl SymbolTable { } /// Collects the provenance of every **bound** `external fn` across all - /// scopes, deduplicated by `(logical_module, export_field)`. + /// scopes, deduplicated by `(logical_module, export_field, decl)`. /// /// The driver consumes this to resolve and validate each external `.wasm` /// before linking. Unbound bare externs (declared without a binding `use`) /// carry no origin and are skipped — they never reach the linker. + /// + /// The declaration is part of the key because the driver validates the + /// external library against the *declared* signature it recovers from + /// `decl`. Two files may each declare and bind the same `(module, field)`, + /// and the linker satisfies an import on `(module, field)` alone with no + /// signature comparison of its own — so dropping either declaration here + /// would leave its signature unchecked and let a disagreement ship as a + /// silently mis-linked artifact instead of a rejection. Only a genuinely + /// repeated registration of one declaration (the same extern reachable from + /// more than one scope) collapses. + /// + /// The order is deterministic: a scope's symbols are stored by name in a + /// hash map, so the traversal alone would leave the order — and hence which + /// of several failing externs is reported first — unstable across runs. #[must_use = "this enumeration has no side effects"] pub(crate) fn extern_origins(&self) -> Vec { - let mut seen: BTreeSet<(String, String)> = BTreeSet::new(); - let mut origins = Vec::new(); + let mut origins: BTreeMap<(String, String, DefId), ExternOrigin> = BTreeMap::new(); for scope in &self.scopes { for symbol in scope.symbols.values() { let Some(info) = symbol.as_function() else { @@ -1934,13 +1947,15 @@ impl SymbolTable { let Some(origin) = info.extern_origin() else { continue; }; - let key = (origin.logical_module.clone(), origin.export_field.clone()); - if seen.insert(key) { - origins.push(origin.clone()); - } + let key = ( + origin.logical_module.clone(), + origin.export_field.clone(), + origin.decl, + ); + origins.entry(key).or_insert_with(|| origin.clone()); } } - origins + origins.into_values().collect() } /// Returns the provenance of the **bound** `external fn` declared by @@ -1969,21 +1984,6 @@ impl SymbolTable { None } - /// Looks up a function by name across **all** registered scopes, ignoring - /// file boundaries; the returned [`FuncInfo`] carries the extern provenance, - /// so post-type-check phases can read it scope-agnostically. - #[must_use = "this is a pure lookup with no side effects"] - pub(crate) fn lookup_function_anywhere(&self, name: &str) -> Option { - for scope in &self.scopes { - if let Some(symbol) = scope.lookup_symbol_local(name) - && let Some(info) = symbol.as_function() - { - return Some(info.clone()); - } - } - None - } - pub(crate) fn register_method( &mut self, type_name: &str, @@ -4191,6 +4191,13 @@ mod tests { } } + fn i64_type() -> TypeInfo { + TypeInfo { + kind: TypeInfoKind::Number(NumberType::I64), + type_params: vec![], + } + } + fn origin(module: &str, field: &str) -> ExternOrigin { ExternOrigin { logical_module: module.to_string(), @@ -4214,7 +4221,7 @@ mod tests { .expect("registering a bound extern should succeed"); let info = table - .lookup_function_anywhere("sort") + .lookup_function("sort") .expect("sort should be registered"); assert!(info.is_extern(), "a registered extern must be discriminated"); let found = info.extern_origin().expect("bound extern carries origin"); @@ -4230,7 +4237,7 @@ mod tests { .expect("registering an unbound extern should succeed"); let info = table - .lookup_function_anywhere("add") + .lookup_function("add") .expect("add should be registered"); assert!( info.is_extern(), @@ -4250,7 +4257,7 @@ mod tests { .expect("registering a local function should succeed"); let info = table - .lookup_function_anywhere("helper") + .lookup_function("helper") .expect("helper should be registered"); assert!(!info.is_extern()); assert!(info.extern_origin().is_none()); @@ -4286,9 +4293,9 @@ mod tests { } #[test] - fn extern_origins_dedups_repeated_module_field_pairs() { - // Two distinct externs that name the same module+field collapse to a - // single resolution unit; the driver should resolve that `.wasm` once. + fn extern_origins_dedups_one_declaration_reached_from_two_scopes() { + // One declaration registered in two scopes is one binding; the driver + // should resolve and validate that `.wasm` once. let mut table = SymbolTable::default(); table .register_extern_function( @@ -4314,7 +4321,56 @@ mod tests { assert_eq!( origins.len(), 1, - "identical (module, field) pairs dedup to one origin, got {origins:?}" + "one declaration reached twice is one origin, got {origins:?}" + ); + } + + #[test] + fn extern_origins_keeps_two_declarations_of_one_module_field() { + // Two files may each declare and bind `collections::sort`. The driver + // validates the resolved library against the signature *each* + // declaration states, and the linker satisfies the import on + // `(module, field)` alone with no signature comparison of its own — + // so a dropped declaration is a signature that is never checked and a + // mismatch that ships as a mis-linked artifact. + let mut table = SymbolTable::default(); + let mut first = origin("collections", "sort"); + first.decl = inference_ast::ids::idx_from_u32(1); + let mut second = origin("collections", "sort"); + second.decl = inference_ast::ids::idx_from_u32(2); + table + .register_extern_function( + "sort", + vec![i32_type()], + vec![None], + i32_type(), + Some(first), + ) + .unwrap(); + let _ = table.push_scope_with_name("other_file", Visibility::Public); + table + .register_extern_function( + "sort", + vec![i64_type()], + vec![None], + i64_type(), + Some(second), + ) + .unwrap(); + + let origins = table.extern_origins(); + assert_eq!( + origins.len(), + 2, + "both declarations must reach validation, got {origins:?}" + ); + assert_eq!( + origins.iter().map(|o| o.decl).collect::>(), + vec![ + inference_ast::ids::idx_from_u32(1), + inference_ast::ids::idx_from_u32(2) + ], + "the enumeration order is deterministic, got {origins:?}" ); } } diff --git a/core/type-checker/src/type_checker.rs b/core/type-checker/src/type_checker.rs index c48c314a..832a0c68 100644 --- a/core/type-checker/src/type_checker.rs +++ b/core/type-checker/src/type_checker.rs @@ -171,7 +171,7 @@ pub(crate) struct TypeChecker { /// directives before externs are registered. /// /// Keyed by the *declaration*, not the bare name: a `use { f } from m;` - /// directive is file-global, so it binds only the **top-level** `external fn + /// directive is file-wide, so it binds only the **top-level** `external fn /// f` and never a same-named extern declared inside a `spec` — the file's top /// level and a `spec` within it being the only two places a declaration can /// sit. Keying by [`DefId`] keeps that inner scope's externs unbound (and so @@ -199,6 +199,17 @@ pub(crate) struct TypeChecker { /// callee's parameter names. Guards the same repeated visit as /// [`Self::mixed_argument_calls_reported`]. labelled_argument_calls_checked: FxHashSet, + /// Top-level `external fn` declarations whose name is also a top-level + /// function **in the same file**, reported by + /// [`Self::check_extern_function_name_collisions`]. + /// + /// Registration consults this to leave the extern out of the file's symbol + /// table: the two declarations claim one symbol, and inserting the second + /// raises a generic registration failure that names neither site. Skipping + /// the extern leaves the purpose-built collision diagnostic as the only + /// message — and leaves an unrelated duplicate of either name still able to + /// report its own. + same_file_extern_collisions: FxHashSet, } /// RAII guard that enters a spec scope on construction and pops it on drop. @@ -285,6 +296,10 @@ impl TypeChecker { ) -> (SymbolTable, Vec<(Option, TypeCheckError)>) { self.process_directives(ctx); self.collect_extern_bindings(ctx); + // Runs before any registration so a same-file collision is reported by + // the purpose-built diagnostic rather than by the symbol table refusing + // the second insert. + self.check_extern_function_name_collisions(ctx); self.register_types(ctx); self.collect_function_and_constant_definitions(ctx); // Top-level consts become importable / qualified-resolvable symbols after @@ -1376,6 +1391,13 @@ impl TypeChecker { let return_type = returns .map(|r| TypeInfo::from_type_id(ctx.arena(), r)) .unwrap_or_default(); + // A declaration that collides with a same-file function is left + // out of the symbol table: the function already holds the + // symbol, and inserting over it would raise a second, generic + // diagnostic for a collision already reported precisely. + if self.same_file_extern_collisions.contains(&def_id) { + return; + } let origin = self.extern_module_bindings.get(&def_id).cloned(); if let Err(err) = self.symbol_table.register_extern_function( &func_name, @@ -4093,11 +4115,13 @@ impl TypeChecker { // import (`use lib::arith::{add};`); `definition_scope_id` is the // callee's defining file either way, so codegen file-qualifies the // WASM name correctly even when it differs from the calling file. - // Externs carry no local body, so an extern call resolves by bare - // name against codegen's import map. That bare-identifier shape is - // also what `Compiler::param_escapes_to_extern` keys on when it - // decides whether a compound parameter must keep its private entry - // copy because the callee may write through the pointer. Leaving + // Externs carry no local body, so an extern call is resolved at + // code generation time by looking its bare name up in the scope the + // call is written in — the declaring file, and the `spec` block + // within it. That bare-identifier shape is also what + // `Compiler::param_escapes_to_extern` keys on when it decides + // whether a compound parameter must keep its private entry copy + // because the callee may write through the pointer. Leaving // externs unrecorded keeps "an extern call carries no target" a // contract codegen can rely on, rather than a value it would have // to know to ignore. @@ -4303,34 +4327,43 @@ impl TypeChecker { /// Binds each `external fn` to the source module named by a `use … from` /// clause, populating [`Self::extern_module_bindings`]. /// + /// A `use … from` clause is file-scoped: it names fields of a logical module + /// and binds the *declaring file's own* top-level `external fn`s of those + /// names. Accumulation and resolution are therefore both per file — a + /// directive in one file can neither bind nor conflict with a declaration in + /// another, and the diagnostics below all describe one file's own text. + /// /// For every `use { fields } from module;` directive, each field is paired /// with `module`. The resulting bindings are validated: /// - /// - A field imported from two or more distinct modules is reported as - /// [`TypeCheckError::AmbiguousExternModule`] and left unbound. - /// - A field imported from a module but never declared as an `external fn` - /// is reported as [`TypeCheckError::ExternImportNotDeclared`]. + /// - A field imported from two or more distinct modules *by the same file* + /// is reported as [`TypeCheckError::AmbiguousExternModule`] and left + /// unbound. Two files may bind the same name to different modules; the + /// declarations are distinct and each keeps its own origin. + /// - A field imported from a module but not declared as an `external fn` at + /// the importing file's top level is reported as + /// [`TypeCheckError::ExternImportNotDeclared`]. /// - A field imported from exactly one module and declared as an extern is /// recorded as a bound [`ExternOrigin`]. /// /// An `external fn` with no binding `use` is left unbound (no error): a bare /// extern declaration is valid; analysis rule A024 governs whether *calling* /// an unlinked extern is allowed. + /// + /// Diagnostics are emitted in source order. The per-file map is drained in + /// hash order, so each file's errors are sorted by source position before + /// they are pushed, and the files themselves are visited in arena order — + /// making the reported order the order the user reads. fn collect_extern_bindings(&mut self, ctx: &TypedContext) { let arena = ctx.arena(); + let index = ctx.extern_index(); - let extern_decls = Self::collect_top_level_extern_decls(arena); - - // field name → (distinct modules in first-seen order, first import - // location, label of the file that owns that first `use … from`). The - // owning-file label rides alongside the location because this scan runs - // at the root cursor over every file's directives: a dangling or - // ambiguous import in an imported file must be stamped with that file, - // not the entry, or the location (per-file-local) is misattributed. - let mut imports: FxHashMap, Location, Option)> = - FxHashMap::default(); for sf in arena.source_files() { let owner_label = inference_ast::nodes::file_label(&sf.module_path); + + // field name → (distinct modules in first-seen order, first import + // location), over this file's directives alone. + let mut imports: FxHashMap, Location)> = FxHashMap::default(); for directive in &sf.directives { let Directive::Use(use_dir) = directive; let Some(module_ref) = &use_dir.from else { @@ -4346,74 +4379,134 @@ impl TypeChecker { let field = arena[field_id].name.clone(); let entry = imports .entry(field) - .or_insert_with(|| (Vec::new(), use_dir.location, owner_label.clone())); + .or_insert_with(|| (Vec::new(), use_dir.location)); if !entry.0.contains(&module) { entry.0.push(module.clone()); } } } - } - for (field, (modules, location, owner_label)) in imports { - let Some(&decl) = extern_decls.get(&field) else { - self.push_error_with_label( - owner_label, - TypeCheckError::ExternImportNotDeclared { - name: field, - module: modules.join(", "), + let mut errors: Vec<(Location, TypeCheckError)> = Vec::new(); + for (field, (modules, location)) in imports { + let Some(decl) = index.lookup_top_level(&sf.module_path, &field) else { + errors.push(( location, - }, - ); - continue; - }; - if modules.len() > 1 { - let module_list = modules - .iter() - .map(|m| format!("`{m}`")) - .collect::>() - .join(", "); - self.push_error_with_label( - owner_label, - TypeCheckError::AmbiguousExternModule { - name: field, - modules: module_list, + TypeCheckError::ExternImportNotDeclared { + name: field, + module: modules.join(", "), + location, + }, + )); + continue; + }; + if modules.len() > 1 { + let module_list = modules + .iter() + .map(|m| format!("`{m}`")) + .collect::>() + .join(", "); + errors.push(( location, + TypeCheckError::AmbiguousExternModule { + name: field, + modules: module_list, + location, + }, + )); + continue; + } + let logical_module = modules.into_iter().next().expect("one module"); + self.extern_module_bindings.insert( + decl, + ExternOrigin { + logical_module, + export_field: field, + decl, + resolved_path: None, }, ); - continue; } - let logical_module = modules.into_iter().next().expect("one module"); - self.extern_module_bindings.insert( - decl, - ExternOrigin { - logical_module, - export_field: field, - decl, - resolved_path: None, - }, - ); + + errors.sort_by_key(|(location, _)| (location.offset_start, location.offset_end)); + for (_, error) in errors { + self.push_error_with_label(owner_label.clone(), error); + } } } - /// Collects every **top-level** `external fn` declaration, mapping its name - /// to its declaring [`DefId`]. + /// Rejects every top-level `external fn` whose name is also a top-level + /// function's, anywhere in the program. + /// + /// The two resolve perfectly well: an `external fn` is identified by its + /// declaration and a bare call resolves in the scope it is written in, so + /// such a program has one callee per call site and would run correctly. The + /// pair is rejected as a rule about the name — a local function shadowing a + /// foreign-boundary declaration is hard to read, because a call site does + /// not say whether the callee is compiled here or linked in. That is a + /// property of the spelling, so the rule spans the whole program rather than + /// one file; the same-file half additionally replaces the symbol table's + /// refusal of the second insert, which named neither declaration. + /// + /// Only the two top-level kinds take part. A method is namespaced under its + /// receiver type and a `spec`-inner declaration under its `spec`, so neither + /// is written as the bare name the rule is about. /// - /// A `use … from` clause is file-global and binds only top-level externs, - /// so this deliberately does **not** descend into `spec` or `module` bodies: - /// a same-named extern declared in a spec or module is left out, stays - /// unbound, and remains A024-rejected when called. Descending here (the prior - /// behavior) let a top-level `use` silently bind a spec-inner extern, - /// suppressing A024 and miscompiling proof-mode codegen. - fn collect_top_level_extern_decls(arena: &AstArena) -> FxHashMap { - let mut decls = FxHashMap::default(); + /// One diagnostic per colliding `external fn`, naming the first colliding + /// function in arena order; files are walked in that same order and each + /// file's declarations in source order, so a program with several collisions + /// reports them the way the user reads them. + fn check_extern_function_name_collisions(&mut self, ctx: &TypedContext) { + let arena = ctx.arena(); + let mut functions: FxHashMap<&str, (Location, Option)> = FxHashMap::default(); for sf in arena.source_files() { + let label = inference_ast::nodes::file_label(&sf.module_path); for &def_id in &sf.defs { - if let Def::ExternFunction { name, .. } = &arena[def_id].kind { - decls.insert(arena[*name].name.clone(), def_id); + if let Def::Function { name, .. } = &arena[def_id].kind { + functions + .entry(&arena[*name].name) + .or_insert_with(|| (arena[def_id].location, label.clone())); } } } - decls + if functions.is_empty() { + return; + } + + for sf in arena.source_files() { + let label = inference_ast::nodes::file_label(&sf.module_path); + // The registration skip is a same-file question, so it is decided by + // this file's own function names rather than by the program-wide map + // above, whose entry for a name may belong to another file entirely. + let own: FxHashSet<&str> = sf + .defs + .iter() + .filter_map(|&def_id| match &arena[def_id].kind { + Def::Function { name, .. } => Some(arena[*name].name.as_str()), + _ => None, + }) + .collect(); + for &def_id in &sf.defs { + let Def::ExternFunction { name, .. } = &arena[def_id].kind else { + continue; + }; + let name = &arena[*name].name; + let Some((function_location, function_file)) = functions.get(name.as_str()) else { + continue; + }; + if own.contains(name.as_str()) { + self.same_file_extern_collisions.insert(def_id); + } + self.push_error_with_label( + label.clone(), + TypeCheckError::ExternFunctionNameCollision { + name: name.clone(), + location: arena[def_id].location, + function_location: *function_location, + function_file: function_file.clone(), + }, + ); + } + } } /// Process a use statement (Phase A: registration only). diff --git a/core/type-checker/src/typed_context.rs b/core/type-checker/src/typed_context.rs index 8ffd514f..49dea038 100644 --- a/core/type-checker/src/typed_context.rs +++ b/core/type-checker/src/typed_context.rs @@ -5,6 +5,7 @@ use crate::{ errors::TypeMismatchContext, + extern_index::ExternIndex, symbol_table::{EnumInfo, ExternOrigin, ResolvedNominalType, StructInfo, SymbolTable}, type_info::{NumberType, TypeInfo, TypeInfoKind}, }; @@ -125,6 +126,10 @@ pub struct TypedContext { /// truth for what a literal denotes, and nothing about how a program /// compiles depends on which position supplied that type. literal_type_sources: FxHashMap, + /// Every `external fn` declaration in the program, keyed by the scope that + /// introduces it. A pure function of the arena, so it is built at + /// construction and is already answering queries while type checking runs. + extern_index: ExternIndex, } // Compile-time assertion: TypedContext is Send + Sync. Its symbol table is an @@ -143,6 +148,7 @@ impl TypedContext { Self { symbol_table: SymbolTable::default(), node_types: FxHashMap::default(), + extern_index: ExternIndex::build(&arena), arena, structs_by_key: FxHashMap::default(), enums_by_key: FxHashMap::default(), @@ -593,17 +599,16 @@ impl TypedContext { }) } - /// Returns the provenance of an `external fn`, or `None` for a local - /// function or an unbound extern (one declared without a binding `use`). + /// The program's `external fn` declarations, indexed by the scope that + /// declares them. /// - /// The returned [`ExternOrigin`] gives the logical source module and export - /// field for the named extern. WASM code generation consumes this per call - /// site to emit an import and lower the call to its import index. + /// Resolves the bare callee name at a call site to the declaration that + /// site actually names, which is what an `external fn`'s identity is: two + /// declarations may share a name and differ in everything else. See + /// [`ExternIndex`] for why every consumer must share one index. #[must_use = "this is a pure lookup with no side effects"] - pub fn extern_origin(&self, name: &str) -> Option { - self.symbol_table - .lookup_function_anywhere(name) - .and_then(|info| info.extern_origin().cloned()) + pub fn extern_index(&self) -> &ExternIndex { + &self.extern_index } /// Returns the provenance of the **bound** `external fn` declared by @@ -612,26 +617,25 @@ impl TypedContext { /// Analysis uses this to decide whether a specific call resolves to a bound /// or unbound extern when two same-named externs exist (e.g. a top-level /// and a spec-inner `f`): a name keyed query cannot tell them apart, but the - /// declaring [`DefId`] can. + /// declaring [`DefId`] can. Code generation reserves one import per bound + /// declaration through it, and the specification translator names the body + /// a spec's obligation refers to. #[must_use = "this is a pure lookup with no side effects"] pub fn extern_origin_by_decl(&self, decl: DefId) -> Option { self.symbol_table.extern_origin_by_decl(decl) } - /// Returns true if the named function is an `external fn` (bound or unbound). - #[must_use = "this is a pure check with no side effects"] - pub fn is_extern_function(&self, name: &str) -> bool { - self.symbol_table - .lookup_function_anywhere(name) - .is_some_and(|info| info.is_extern()) - } - /// Returns the provenance of every **bound** `external fn` in the program, - /// deduplicated by `(logical_module, export_field)`. + /// deduplicated by `(logical_module, export_field, decl)`. /// /// The build driver consumes this to resolve and validate each external - /// `.wasm` once before linking. Unbound bare externs carry no origin and do - /// not appear here. + /// `.wasm` before linking. Unbound bare externs carry no origin and do not + /// appear here. + /// + /// Every bound *declaration* appears, including two that name the same + /// module and field: the driver validates the library against the signature + /// each declaration states, and only one entry per `(module, field)` would + /// leave the other's signature unchecked. #[must_use = "this enumeration has no side effects"] pub fn extern_origins(&self) -> Vec { self.symbol_table.extern_origins() diff --git a/core/wasm-codegen/README.md b/core/wasm-codegen/README.md index 62cd393e..237cd125 100644 --- a/core/wasm-codegen/README.md +++ b/core/wasm-codegen/README.md @@ -27,8 +27,11 @@ Typed AST (TypedContext) 2. **Import reservation + function index pre-scan** - Build the complete WASM function index space before any body is compiled, in two stages that each run across all files: (a) `register_imports` assigns indices `0..N` to every `external fn` declaration bound - via `use … from `, populating `extern_name_to_idx` and recording the - `(logical_module, export_field, type_idx)` tuple needed for the import section; + via `use … from `, populating `extern_import_idx` — keyed by the declaring + `DefId` rather than by the extern's name, since two files may declare the same name and + bind it to different modules — and recording the `(logical_module, export_field, + type_idx)` tuple needed for the import section. Declarations that bind the same + `(module, field)` at the same signature share one import; (b) a two-pass local scan first registers all top-level functions from every source file under their mangled `FnKey` names, then registers all struct methods under their mangled names (`"{StructName}.{method_name}"`). Functions from the entry file use unqualified diff --git a/core/wasm-codegen/docs/arrays-and-memory.md b/core/wasm-codegen/docs/arrays-and-memory.md index ee214935..ee293fa9 100644 --- a/core/wasm-codegen/docs/arrays-and-memory.md +++ b/core/wasm-codegen/docs/arrays-and-memory.md @@ -331,7 +331,8 @@ fn compute_frame_layout(input: &FrameLayoutInput<'_>) -> Result register_imports(extern_def_ids) // Stage 0 - | extern_name_to_idx["sum"] = 0, ... + | extern_import_idx[def_id(sum)] = 0, ... | returns N = import_count | +---> set_local_func_base(N) // seeds func_idx = N @@ -247,10 +254,14 @@ local body to compile, but it does have a WASM function index (assigned by Stage 0) and a WASM type signature derived from the declared Inference parameter and return types. -When `lower_function_call` resolves the callee and finds it in -`extern_name_to_idx`, it emits `call ` via the same -`Instruction::Call` path used for local functions. The only difference is which -index table the lookup hits. +Resolving the callee is not a probe of a flat, program-wide name table. It goes +through `ExternCallScope::import_target`, which resolves the bare callee name to +a declaration via `TypedContext::extern_index()` — scoped to the file the call is +written in and the `spec` block enclosing it — and then probes +`extern_import_idx` by that declaration's `DefId`. On a hit `lower_function_call` +emits `call ` via the same `Instruction::Call` path used for local +functions; on a miss the name is not a bound extern *at this point in the +program*, and resolution falls through to the local-function path. ### Example @@ -384,6 +395,7 @@ resolved until multi-file compilation is implemented (currently `todo!()` in `co | `wasm_codegen_param_by_reference` | varies | A compound parameter or receiver is neither assigned through nor forwarded to an `external fn`, so it gets no frame slot and no entry copy | | `wasm_codegen_emit_import_section` | 1+ | Import section emitted (fires whenever at least one `external fn` is present) | | `wasm_codegen_emit_extern_call` | 1+ | Extern call lowered to `call ` (fires in `single_import_test`) | +| `wasm_codegen_extern_out_of_scope` | varies | A callee name that is a bound `external fn` elsewhere in the program is not one in the calling scope, so the call resolves as a local function instead | The `fn_params_test` verifies `wasm_codegen_emit_function_params` fires exactly 7 times (matching `fn_params.inf`: 1+1+1+2+2 params). The `fn_calls_test` verifies diff --git a/core/wasm-codegen/src/compiler.rs b/core/wasm-codegen/src/compiler.rs index 13ca3f6b..79ed764f 100644 --- a/core/wasm-codegen/src/compiler.rs +++ b/core/wasm-codegen/src/compiler.rs @@ -71,7 +71,7 @@ use inference_ast::nodes::{ }; use inference_hassert::HSpecMap; use inference_type_checker::{ - EnumInfo, + EnumInfo, ExternIndex, type_info::{NumberType, TypeInfo, TypeInfoKind}, typed_context::TypedContext, }; @@ -214,7 +214,7 @@ struct LoopContext { /// indexes the shared [`Compiler::types`] table; identical signatures dedup onto /// the same entry. Imports occupy WASM function indices `0..N`, ahead of every /// locally defined function (see [`Compiler::register_imports`]). -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] struct ImportEntry { module: String, field: String, @@ -332,10 +332,21 @@ pub(crate) struct Compiler { /// registration order. Each occupies WASM function index `i` for its /// position `i` in this vector (imports come before all local functions). imports: Vec, - /// Maps an `external fn` name to its WASM import function index (`0..N`). - /// Calls to an extern lower to `call ` rather than a local - /// function index. Populated alongside [`Self::imports`] during Stage 1. - extern_name_to_idx: FxHashMap, + /// Maps an `external fn` *declaration* to its WASM import function index + /// (`0..N`). Calls to a bound extern lower to `call ` rather + /// than a local function index. Populated alongside [`Self::imports`] + /// during Stage 1. + /// + /// Keyed by the declaring [`DefId`], never by the extern's name, because a + /// name is not an identity: two files may each declare an `external fn` of + /// that name bound to different modules, and a name-keyed map holds exactly + /// one of them program-wide. It would then route the other file's calls — + /// and any same-named *local* call anywhere in the program — into an import + /// the calling file cannot see. A call site resolves its bare name to a + /// declaration through [`TypedContext::extern_index`] first, so what it + /// looks up here is already a declaration that call can name. Declarations + /// binding the same `(module, field)` share one import index. + extern_import_idx: FxHashMap, /// Sticky flag: set to `true` when any function requires linear memory. has_memory: bool, /// Maps function keys to their array return type metadata. @@ -514,9 +525,51 @@ struct FrameLayoutInput<'a> { /// Module path of the file the function is defined in, used to resolve type /// names against their defining file. module_path: &'a [String], - /// Registered import names, as `register_imports` left them. Only the keys - /// are read. - extern_names: &'a FxHashMap, + /// Which `external fn` a bare callee name in this body denotes, and whether + /// that declaration registered an import. + extern_scope: ExternCallScope<'a>, +} + +/// The point in the program a bare callee name is being resolved from, paired +/// with the import registration [`Compiler::register_imports`] produced. +/// +/// An `external fn`'s identity is its declaration, and the only thing that +/// picks a declaration out of a bare name is where the name is written: the +/// declaring file, and the `spec` block within it. Two passes ask the same +/// question of that scope — [`Compiler::lower_function_call`], which emits the +/// call, and [`Compiler::param_escapes_to_extern`], which decides whether a +/// compound parameter may be passed by reference. They must answer identically: +/// a gate narrower than emission drops an entry copy and lets a foreign store +/// land in the caller's memory. Routing both through one scope and one +/// [`ExternCallScope::import_target`] makes that agreement structural rather +/// than a coincidence between two separate walks. +#[derive(Clone, Copy)] +struct ExternCallScope<'a> { + index: &'a ExternIndex, + /// Module path of the file the point of use sits in. + module_path: &'a [String], + /// The `spec` block the point of use sits in, `None` at a file's top level. + spec: Option<&'a str>, + imports: &'a FxHashMap, +} + +impl ExternCallScope<'_> { + /// The WASM import index a call to `callee_name` lowers to here, or `None` + /// when the name is not a bound `external fn` at this point — either + /// because no declaration of that name is in scope, or because the one that + /// is has no binding `use … from` clause and so registered no import. + fn import_target(&self, callee_name: &str) -> Option { + self.index + .lookup(self.module_path, self.spec, callee_name) + .and_then(|decl| self.imports.get(&decl).copied()) + } + + /// Whether the program registered any import at all. A program with none + /// has nothing for a call to reach, so scans that only look for extern + /// calls can skip their body walk outright. + fn has_imports(&self) -> bool { + !self.imports.is_empty() + } } impl Compiler { @@ -534,7 +587,7 @@ impl Compiler { module_name: module_name.to_string(), func_name_to_idx: FxHashMap::default(), imports: Vec::new(), - extern_name_to_idx: FxHashMap::default(), + extern_import_idx: FxHashMap::default(), has_memory: false, func_array_returns: FxHashMap::default(), func_struct_returns: FxHashMap::default(), @@ -753,10 +806,12 @@ impl Compiler { /// For each extern this: /// 1. lowers its declared signature to a WASM `(params, results)` type and /// dedups it into [`Self::types`] (identical signatures share one entry); - /// 2. records an [`ImportEntry`] carrying the logical module / export field - /// from the Phase 1 provenance ([`TypedContext::extern_origin`]); - /// 3. maps the extern's name to its import function index so call lowering can - /// emit `call `. + /// 2. interns an [`ImportEntry`] carrying the logical module / export field + /// from the declaration's provenance + /// ([`TypedContext::extern_origin_by_decl`]), sharing one import with any + /// declaration that already named the same foreign function; + /// 3. maps the *declaration* to its import function index, so call lowering + /// can emit `call ` for the calls that can name it. /// /// Externs without provenance (a bare `external fn` with no binding `use`) are /// skipped: they cannot be emitted as a well-formed two-level import, and @@ -773,16 +828,9 @@ impl Compiler { ctx: &TypedContext, ) -> Result { for &def_id in extern_def_ids { - let Def::ExternFunction { - name, - args, - returns, - .. - } = &arena[def_id].kind - else { + let Def::ExternFunction { args, returns, .. } = &arena[def_id].kind else { continue; }; - let extern_name = arena[*name].name.clone(); // Resolve provenance by the declaring `DefId`, not by name. Two // same-named externs can coexist (a bound top-level `f` and an // unbound spec-inner `f`); a name-keyed lookup cannot tell them apart @@ -802,18 +850,34 @@ impl Compiler { }; let type_idx = self.intern_type(params, results); - let import_idx = self.imports.len() as u32; - self.imports.push(ImportEntry { + let import_idx = self.intern_import(ImportEntry { module: origin.logical_module, field: origin.export_field, type_idx, }); - self.extern_name_to_idx.insert(extern_name, import_idx); + self.extern_import_idx.insert(def_id, import_idx); } Ok(self.imports.len() as u32) } + /// Interns `entry` into [`Self::imports`], returning the WASM function index + /// it occupies. + /// + /// Declarations that bind the same `(module, field)` at the same signature + /// name one foreign function, so they share one import. Emitting a second + /// import of an identical pair would give that one function two function + /// indices, obliging a host or a linker to satisfy it twice. + #[allow(clippy::cast_possible_truncation)] + fn intern_import(&mut self, entry: ImportEntry) -> u32 { + if let Some(existing) = self.imports.iter().position(|e| *e == entry) { + return existing as u32; + } + let idx = self.imports.len() as u32; + self.imports.push(entry); + idx + } + /// Lowers an extern's declared parameter types to WASM value types. An /// ignored parameter (`external fn f(_: i32)`) still occupies an ABI slot: /// the call site pushes the argument and the real `.wasm` export declares @@ -1405,7 +1469,10 @@ impl Compiler { reach_plan, ); - self.frame_layout = Self::compute_frame_layout( + // The layout gate must read the very scope call lowering will read, so + // the input borrows `self`; the result lands in `self.frame_layout` + // only once that borrow has ended. + let frame_layout = Self::compute_frame_layout( &FrameLayoutInput { arena, block_id: body_id, @@ -1414,10 +1481,11 @@ impl Compiler { args: &args, method_struct_name, module_path, - extern_names: &self.extern_name_to_idx, + extern_scope: self.extern_call_scope(ctx), }, self.memory_layout.stack_size(), )?; + self.frame_layout = frame_layout; // Record the real frame size (0 for frameless functions) keyed by the // structured `FnKey` itself, not its lossy `Display` rendering: two @@ -2035,25 +2103,26 @@ impl Compiler { /// fails in the unsafe direction — a missing copy. A false positive only /// costs one extra copy. /// - /// Callee resolution mirrors emission: `extern_name_to_idx` is read only by - /// [`Self::lower_function_call`], which is reachable only for a bare - /// `Expr::Identifier` callee, and `register_imports` fills the map before any - /// body is compiled, so it is complete at layout time. Complete in both - /// senses: the map is whole-program, built from every file's externs at once, - /// so an extern declared *and* called inside a non-entry file disqualifies - /// there too. + /// Callee resolution mirrors emission because it *is* emission's: this scan + /// and [`Self::lower_function_call`] ask [`ExternCallScope::import_target`] + /// about the same name in the same scope, so neither can recognize a call + /// the other does not. `register_imports` fills that scope's import map + /// before any body is compiled, so the answer is already final when a frame + /// is laid out. /// /// That a bare identifier is the only callee shape worth checking is not an - /// approximation but a consequence of the grammar: an `external fn` cannot be - /// `pub`, so no extern is ever nameable from another file, and the qualified - /// and cross-file callee shapes that preempt the bare-identifier arm can - /// therefore never resolve to one. Should externs ever become exportable, - /// this gate silently starts missing them and must be widened with the arm - /// that lowers them. + /// approximation but a consequence of the grammar and of how an extern call + /// is lowered. An `external fn` may not be `pub` — the parser reports that + /// as an error — and the type checker deliberately records no call target + /// for a call to one, so the qualified and item-imported callee arms that + /// preempt the bare-identifier arm in [`Self::lower_function_call`] can + /// never resolve to an extern. Should externs ever become exportable, this + /// gate silently starts missing them and must be widened with the arm that + /// lowers them. fn param_escapes_to_extern( arena: &AstArena, block_id: BlockId, - extern_names: &FxHashMap, + extern_scope: ExternCallScope<'_>, param_name: &str, ) -> bool { Self::body_has_expr(arena, block_id, &mut |arena, expr_id| { @@ -2062,7 +2131,8 @@ impl Compiler { }; let callee_is_extern = matches!( &arena[*function].kind, - Expr::Identifier(ident_id) if extern_names.contains_key(&arena[*ident_id].name) + Expr::Identifier(ident_id) + if extern_scope.import_target(&arena[*ident_id].name).is_some() ); callee_is_extern && args @@ -2239,11 +2309,11 @@ impl Compiler { // The gate costs a full body walk, so a program that registered no // imports skips it outright: there is nothing for the parameter to escape // to, and the common case pays nothing. - if !input.extern_names.is_empty() + if input.extern_scope.has_imports() && Self::param_escapes_to_extern( input.arena, input.block_id, - input.extern_names, + input.extern_scope, param_name, ) { @@ -3558,10 +3628,16 @@ impl Compiler { // An `external fn` call targets its import index (0..N) rather than a // local function index. Imports never participate in spec-mangled // lookup, so this probe precedes the free-callee resolution. - if let Some(&import_idx) = self.extern_name_to_idx.get(callee_name) { - cov_mark::hit!(wasm_codegen_emit_extern_call); - self.func().instruction(&Instruction::Call(import_idx)); - return Ok(()); + match self.extern_call_scope(ctx).import_target(callee_name) { + Some(import_idx) => { + cov_mark::hit!(wasm_codegen_emit_extern_call); + self.func().instruction(&Instruction::Call(import_idx)); + return Ok(()); + } + None if self.names_a_registered_import(callee_name) => { + cov_mark::hit!(wasm_codegen_extern_out_of_scope); + } + None => {} } let func_idx = self @@ -3603,6 +3679,33 @@ impl Compiler { Ok(()) } + /// The scope a bare callee name in the function currently being compiled + /// resolves against, together with the imports `register_imports` left. + fn extern_call_scope<'a>(&'a self, ctx: &'a TypedContext) -> ExternCallScope<'a> { + ExternCallScope { + index: ctx.extern_index(), + module_path: &self.current_module_path, + spec: self.current_spec.as_deref(), + imports: &self.extern_import_idx, + } + } + + /// Whether a probe by bare name over the registered imports would have + /// matched `callee_name`. + /// + /// An import's field is the name of the `external fn` that bound it — a + /// `use { f } from m;` clause matches declarations by that very name — so + /// this is exactly the answer a whole-program name map would give. It is + /// asked only once [`ExternCallScope::import_target`] has already decided, + /// to record that the two disagreed: the name is a bound extern somewhere + /// in the program, and is not one here. Nothing depends on the answer; + /// without the mark this case is indistinguishable at compile time from an + /// ordinary local call, and a regression that widened the scope back out + /// would surface only as a wrong value at run time. + fn names_a_registered_import(&self, callee_name: &str) -> bool { + self.imports.iter().any(|entry| entry.field == callee_name) + } + /// Resolves a free-function bare name to its WASM function index, /// preferring the spec-mangled key when inside a spec scope, then the /// current file's qualified key. This is the fallback for a same-file call @@ -6531,6 +6634,60 @@ fn try_const_index_byte_offset( mod tests { use super::*; + /// An `external fn` registration as [`Compiler::register_imports`] leaves + /// it: the program's own declaration index, plus an import index for each + /// declaration a `use … from` clause bound. + /// + /// The scans driven from here resolve a callee name to a *declaration* and + /// only then to an import index, so a list of names no longer describes a + /// registration — the declarations have to be the subject program's. + struct RegisteredExterns { + index: ExternIndex, + imports: FxHashMap, + } + + impl RegisteredExterns { + /// The registration `register_imports` leaves for a program whose + /// `external fn`s are all bound: every top-level declaration, in + /// declaration order, at import indices `0..N`. + fn all(arena: &AstArena) -> Self { + let mut imports = FxHashMap::default(); + for file in arena.source_files() { + for &def_id in &file.defs { + if matches!(arena[def_id].kind, Def::ExternFunction { .. }) { + let idx = + u32::try_from(imports.len()).expect("test import index fits in u32"); + imports.insert(def_id, idx); + } + } + } + Self { + index: ExternIndex::build(arena), + imports, + } + } + + /// The same program with nothing registered, as an unbound declaration + /// leaves it: there is no import for a call to reach. + fn none(arena: &AstArena) -> Self { + Self { + index: ExternIndex::build(arena), + imports: FxHashMap::default(), + } + } + + /// The entry file's top level, which is where every subject program + /// below writes its calls. + fn scope(&self) -> ExternCallScope<'_> { + ExternCallScope { + index: &self.index, + module_path: &[], + spec: None, + imports: &self.imports, + } + } + } + /// Prepares `compiler` to emit a function body whose eager declarations are /// `declarations`, mirroring what `visit_function_definition` sets up before /// lowering, and returns the compiler ready for `take_completed_function`. @@ -7014,12 +7171,213 @@ mod tests { } } + /// Tests for [`ExternCallScope`], the one resolution that both extern-call + /// emission and the frame-layout escape gate go through. + /// + /// These pin **resolution only**: which declaration a bare name denotes at a + /// given point, and which import index that declaration holds. They say + /// nothing about the bytes that come out. That an emitted `call` carries the + /// resolved index is asserted by the executed coverage in + /// `tests/src/codegen/wasm/multi_file_extern.rs`, which runs the module. + /// + /// They build their arenas with the parser alone. The scope an `external fn` + /// is resolved in is a property of where declarations and calls are + /// *written*, which the parser already establishes, so driving the + /// resolution directly keeps these cases reachable regardless of which + /// programs the binding pass accepts. + mod extern_call_scope { + use super::*; + + /// Folds `(module_path, source)` pairs into one arena, the shape a + /// multi-file program reaches code generation in. + fn multi_file_arena(files: &[(&[&str], &str)]) -> AstArena { + let mut arena = AstArena::default(); + for (module_path, source) in files { + let module_path = module_path.iter().map(|s| (*s).to_string()).collect(); + let parsed = inference_parser::parse_into(arena, source, module_path); + assert!( + parsed.errors.is_empty(), + "parse errors: {:?}\nsource:\n{source}", + parsed.errors + ); + arena = parsed.arena; + } + arena + } + + fn path(segments: &[&str]) -> Vec { + segments.iter().map(|s| (*s).to_string()).collect() + } + + /// Two files each declaring `external fn f`, each binding it to a + /// different foreign module, resolve to their own declaration and so to + /// their own import. + /// + /// This is the shape a name-keyed table cannot express: it holds one + /// `f` for the whole program, so one of the two files calls an import + /// satisfied by the other file's library. Each file's call must reach + /// the import its own declaration registered. + #[test] + fn same_named_externs_in_two_files_resolve_per_file() { + let arena = multi_file_arena(&[ + ( + &["a"], + "external fn f(x: i32) -> i32;\nuse { f } from libA;\npub fn ua(x: i32) -> \ + i32 { return f(x); }\n", + ), + ( + &["b"], + "external fn f(x: i32) -> i32;\nuse { f } from libB;\npub fn ub(x: i32) -> \ + i32 { return f(x); }\n", + ), + ]); + let index = ExternIndex::build(&arena); + let (path_a, path_b) = (path(&["a"]), path(&["b"])); + let decl_a = index + .lookup_top_level(&path_a, "f") + .expect("file `a` declares `f`"); + let decl_b = index + .lookup_top_level(&path_b, "f") + .expect("file `b` declares `f`"); + assert_ne!( + decl_a, decl_b, + "the two files declare two different `f`s, which a key that dropped the \ + file would collapse into one" + ); + + let mut imports = FxHashMap::default(); + imports.insert(decl_a, 0); + imports.insert(decl_b, 1); + let scope_a = ExternCallScope { + index: &index, + module_path: &path_a, + spec: None, + imports: &imports, + }; + let scope_b = ExternCallScope { + module_path: &path_b, + ..scope_a + }; + + assert_eq!( + scope_a.import_target("f"), + Some(0), + "file `a`'s call must reach the import its own declaration registered" + ); + assert_eq!( + scope_b.import_target("f"), + Some(1), + "file `b`'s call must reach the import its own declaration registered" + ); + } + + /// A file that declares no `external fn f` sees none, even though + /// another file in the program declares and binds one. + /// + /// This is the miscompile the scope key closes: with a whole-program + /// name table the entry file's call to its *own* `f` matched the + /// sibling's import and was emitted as a call into the foreign module, + /// leaving the entry's own function compiled and unreachable. + #[test] + fn a_siblings_extern_is_not_visible_to_the_entry_file() { + let arena = multi_file_arena(&[ + ( + &[], + "use side;\nfn f(x: i32) -> i32 { return x * 10; }\npub fn run(x: i32) -> \ + i32 { return f(x); }\n", + ), + ( + &["side"], + "external fn f(x: i32) -> i32;\nuse { f } from libA;\npub fn boost(x: i32) \ + -> i32 { return f(x); }\n", + ), + ]); + let index = ExternIndex::build(&arena); + let side = path(&["side"]); + let decl = index + .lookup_top_level(&side, "f") + .expect("file `side` declares `f`"); + let mut imports = FxHashMap::default(); + imports.insert(decl, 0); + + let entry_scope = ExternCallScope { + index: &index, + module_path: &[], + spec: None, + imports: &imports, + }; + assert_eq!( + entry_scope.import_target("f"), + None, + "the entry file names its own `f`, not the sibling's external one" + ); + + let side_scope = ExternCallScope { + index: &index, + module_path: &side, + spec: None, + imports: &imports, + }; + assert_eq!( + side_scope.import_target("f"), + Some(0), + "the declaring file still reaches its own import" + ); + } + + /// Within a `spec` block, a spec-inner declaration shadows a top-level + /// one of the same name, and an unbound declaration registers no import + /// — so a call there reaches no import at all. + /// + /// A key without the spec component would hand the spec's call the + /// top-level declaration's import, emitting a call to a foreign body + /// the program never bound it to. + #[test] + fn a_spec_inner_declaration_shadows_the_top_level_one() { + let arena = multi_file_arena(&[( + &[], + "external fn probe(x: i32) -> i32;\nuse { probe } from sensors;\nspec S {\n \ + external fn probe(x: i32) -> i32;\n}\n", + )]); + let index = ExternIndex::build(&arena); + let top_level = index + .lookup_top_level(&[], "probe") + .expect("the file declares a top-level `probe`"); + let mut imports = FxHashMap::default(); + imports.insert(top_level, 0); + + let file_scope = ExternCallScope { + index: &index, + module_path: &[], + spec: None, + imports: &imports, + }; + assert_eq!( + file_scope.import_target("probe"), + Some(0), + "at the file's top level `probe` is the bound declaration" + ); + + let spec_scope = ExternCallScope { + index: &index, + module_path: &[], + spec: Some("S"), + imports: &imports, + }; + assert_eq!( + spec_scope.import_target("probe"), + None, + "inside `spec S` the name is the spec's own unbound declaration" + ); + } + } + /// Tests for [`Compiler::param_escapes_to_extern`], the gate that decides /// whether a compound parameter — here the `self` receiver — needs its own /// frame slot. /// - /// The scan reads only the AST and the registered import names, so these - /// drive it directly on parsed source rather than through code generation: + /// The scan reads only the AST and the extern scope the body sits in, so + /// these drive it directly on parsed source rather than through codegen: /// the emitted bytes are pinned by the codegen fixtures, while these pin the /// *shapes* the gate must recognize. A missed shape is a silently missing /// copy, so the negative cases are as load-bearing as the positive ones. @@ -7070,22 +7428,10 @@ struct Pair {{ (arena, block_id) } - /// The import map as `register_imports` leaves it: import names to - /// indices. Only the keys matter to the scan. - fn imports() -> FxHashMap { - let mut map = FxHashMap::default(); - for (idx, name) in ["sort_pair", "probe"].iter().enumerate() { - map.insert( - (*name).to_string(), - u32::try_from(idx).expect("test import index fits in u32"), - ); - } - map - } - fn escapes(body: &str) -> bool { let (arena, block_id) = touch_body(body); - Compiler::param_escapes_to_extern(&arena, block_id, &imports(), "self") + let registered = RegisteredExterns::all(&arena); + Compiler::param_escapes_to_extern(&arena, block_id, registered.scope(), "self") } /// Every argument shape whose root peels to `self` hands the external an @@ -7223,18 +7569,6 @@ fn subject(mut g: Holder, mut arr: [i32; 4], mut n: i32) -> i32 {{ (arena, block_id) } - /// The import map as `register_imports` leaves it. Only the keys matter. - fn imports() -> FxHashMap { - let mut map = FxHashMap::default(); - for (idx, name) in ["scramble", "probe"].iter().enumerate() { - map.insert( - (*name).to_string(), - u32::try_from(idx).expect("test import index fits in u32"), - ); - } - map - } - fn written(body: &str, param: &str) -> bool { let (arena, block_id) = subject_body(body); Compiler::param_is_written(&arena, block_id, param) @@ -7242,7 +7576,8 @@ fn subject(mut g: Holder, mut arr: [i32; 4], mut n: i32) -> i32 {{ fn escapes(body: &str, param: &str) -> bool { let (arena, block_id) = subject_body(body); - Compiler::param_escapes_to_extern(&arena, block_id, &imports(), param) + let registered = RegisteredExterns::all(&arena); + Compiler::param_escapes_to_extern(&arena, block_id, registered.scope(), param) } /// The `left` expression of the first assignment in the subject body. @@ -7486,10 +7821,10 @@ fn subject(mut g: Holder, mut arr: [i32; 4], mut n: i32) -> i32 {{ /// Whether the subject program's `external fn` declarations are handed to /// the decision as registered imports. /// - /// The escape gate is keyed on the import map, so the same source answers - /// differently under the two, and which one a case means is the whole - /// point of that case — a bare `true`/`false` at the call site would hide - /// it. + /// The escape gate fires only on a declaration that registered an + /// import, so the same source answers differently under the two, and + /// which one a case means is the whole point of that case — a bare + /// `true`/`false` at the call site would hide it. #[derive(Clone, Copy)] enum Imports { Registered, @@ -7532,9 +7867,9 @@ fn subject(mut g: Holder, mut arr: [i32; 4], mut n: i32) -> i32 {{ _ => None, }) .unwrap_or_else(|| panic!("function `{fn_name}` must be present")); - let extern_names = match imports_state { - Imports::Registered => imports(), - Imports::Unregistered => FxHashMap::default(), + let registered = match imports_state { + Imports::Registered => RegisteredExterns::all(arena), + Imports::Unregistered => RegisteredExterns::none(arena), }; let input = FrameLayoutInput { arena, @@ -7544,7 +7879,7 @@ fn subject(mut g: Holder, mut arr: [i32; 4], mut n: i32) -> i32 {{ args: &args, method_struct_name: None, module_path: &[], - extern_names: &extern_names, + extern_scope: registered.scope(), }; Compiler::compound_param_is_by_reference(&input, &args[param_index]) .expect("the subject programs lay out") @@ -7663,10 +7998,10 @@ fn writes_in_a_nondet_block(mut p: Pair) -> i32 {{ /// A parameter forwarded to a registered import keeps its slot; the same /// program with no imports registered does not. /// - /// The second half is what makes the first non-vacuous: the gate is - /// keyed on the import map, so a fixture whose `external fn` was never - /// bound would exercise the *ungated* path while looking like it - /// exercised the gate. + /// The second half is what makes the first non-vacuous: the gate fires + /// only on a declaration that registered an import, so a fixture + /// whose `external fn` was never bound would exercise the *ungated* + /// path while looking like it exercised the gate. #[test] fn parameters_forwarded_to_an_extern_are_not_by_reference() { let source = "\ diff --git a/core/wasm-codegen/src/hassert/mod.rs b/core/wasm-codegen/src/hassert/mod.rs index 36033d50..fab0c522 100644 --- a/core/wasm-codegen/src/hassert/mod.rs +++ b/core/wasm-codegen/src/hassert/mod.rs @@ -115,119 +115,12 @@ use diag::PCode; /// function compiled from Inference source a specification term may call. /// /// `external fn`s are deliberately absent: they have no structured key and no -/// body here, and are resolved through [`ExternIndex`] instead. +/// body here, and are resolved through +/// [`ExternIndex`](inference_type_checker::ExternIndex) instead. pub(crate) struct CalleeIndex { defs: FxHashMap, } -/// The `external fn` declarations visible from a specification function body, -/// keyed by the scope that introduces them. -/// -/// Resolution must be by *declaration*, not by name. Two `external fn`s may -/// share a name — a bound top-level `sum` and an unbound spec-inner `sum` — and -/// only the declaration a given call site actually names decides whether that -/// call has a linked body and which module it comes from. A `use … from` clause -/// binds top-level declarations only, so a spec-inner one is never bound; a -/// name-keyed lookup would hand it the top-level declaration's origin and emit -/// an obligation naming a merged body the call does not reach. -/// -/// This mirrors `A024`, which resolves unbound-extern calls through the same -/// two-layer walk, so those two passes agree on which declaration a call names. -/// Through the full pipeline `A024` rejects such a call before this pass runs, -/// which makes the agreement defense in depth rather than the sole guard; a -/// pipeline that skips analysis, as the proof-mode test gates do, has nothing -/// else. -/// -/// Code generation does **not** make the same distinction. It reads an -/// extern's *provenance* off the declaration, keyed by `DefId`, but the table -/// its call sites probe (`Compiler::extern_name_to_idx`) is keyed by the bare -/// extern name, whole program: neither file-scoped nor spec-scoped, and unable -/// to hold two same-named declarations at once. A bound `external fn scale` in -/// one file therefore claims the name `scale` for every call site in the -/// program, a sibling file's call to its own local `fn scale` included. This -/// index is strictly the more precise of the two, so where they disagree the -/// obligation names one function and the emitted `call` reaches another. That -/// divergence is pre-existing and tracked by -/// [#423](https://github.com/Inferara/inference/issues/423); the precision here -/// neither causes it nor depends on it. -/// -/// Two scopes exhaust the language: a file's top level and a `spec` block -/// inside it. Specs do not nest, so an inner scope is keyed by its spec name -/// and the lookup is a two-step walk rather than a stack. -pub(crate) struct ExternIndex { - decls: FxHashMap>, -} - -/// A file, or a `spec` block within it — the two places an `external fn` may be -/// declared. -#[derive(PartialEq, Eq, Hash)] -struct ExternScope { - module_path: Vec, - spec: Option, -} - -impl ExternIndex { - /// Collects every `external fn` declaration in the program, in the scope - /// that introduces it. A scope that declares none records no entry. - fn build(arena: &AstArena) -> Self { - let mut decls: FxHashMap> = FxHashMap::default(); - for file in arena.source_files() { - let spec_scopes = file - .defs - .iter() - .filter_map(|&def_id| match &arena[def_id].kind { - Def::Spec { name, defs, .. } => { - Some((Some(arena[*name].name.clone()), defs.as_slice())) - } - _ => None, - }); - for (spec, defs) in std::iter::once((None, file.defs.as_slice())).chain(spec_scopes) { - let externs = extern_decls(arena, defs); - if !externs.is_empty() { - decls.insert( - ExternScope { - module_path: file.module_path.clone(), - spec, - }, - externs, - ); - } - } - } - Self { decls } - } - - /// The `external fn` a bare `name` resolves to from inside `spec` in the - /// file at `module_path`, innermost scope first, or `None` when the name is - /// not an extern there. - fn lookup(&self, module_path: &[String], spec: &str, name: &str) -> Option { - let inner = ExternScope { - module_path: module_path.to_vec(), - spec: Some(spec.to_string()), - }; - let outer = ExternScope { - module_path: module_path.to_vec(), - spec: None, - }; - [inner, outer] - .iter() - .find_map(|scope| self.decls.get(scope)?.get(name).copied()) - } -} - -/// The `external fn` declarations `defs` introduces directly, keeping the first -/// of a repeated name — a repeat within one scope is a type error reported -/// earlier, so which one wins cannot matter to a valid program. -fn extern_decls(arena: &AstArena, defs: &[DefId]) -> FxHashMap { - let mut externs = FxHashMap::default(); - for &def_id in defs { - if let Def::ExternFunction { name, .. } = &arena[def_id].kind { - externs.entry(arena[*name].name.clone()).or_insert(def_id); - } - } - externs -} - impl CalleeIndex { /// Builds the index from the same buckets code generation collected, keying /// each function by the identity code generation registered it under. @@ -302,7 +195,7 @@ pub(crate) fn translate_spec_fns( ) -> (HSpecMap, Vec) { let arena = ctx.arena(); let callee = CalleeIndex::build(arena, buckets); - let externs = ExternIndex::build(arena); + let externs = ctx.extern_index(); let mut map = HSpecMap::default(); let mut diagnostics = Vec::new(); @@ -313,7 +206,7 @@ pub(crate) fn translate_spec_fns( &entry.module_path, &entry.spec_name, &callee, - &externs, + externs, ); let hassert = translator.translate_fn(entry.def_id, plan); let fn_diagnostics = translator.take_diagnostics(); diff --git a/core/wasm-codegen/src/hassert/tests.rs b/core/wasm-codegen/src/hassert/tests.rs index 9ee5b00d..4c135464 100644 --- a/core/wasm-codegen/src/hassert/tests.rs +++ b/core/wasm-codegen/src/hassert/tests.rs @@ -2059,36 +2059,38 @@ spec Outer { fn f() forall { let a: i32 = @; assert(probe(a) == a); } } ); } -/// An `external fn` is visible only in the file that declares it. A sibling -/// file's spec that calls a same-named *local* function resolves to its own -/// function, not to the other file's extern. +/// An `external fn` is visible only in the file that declares it, so a bare +/// name written in two files means two declarations and each file's obligation +/// names its own. /// /// The scope key is `(defining file, enclosing spec)`, and this pins the file -/// half of it: drop that component and the extern in `side` captures `main`'s -/// call, emitting an obligation about a body the call never reaches — which -/// resolves post-link and elaborates under `coqc`, so nothing downstream would -/// notice. +/// half of it: without that component the index holds one `scale` for the whole +/// program, and one of the two specs states an obligation about the *other* +/// library — a body its call never reaches, which still resolves post-link and +/// elaborates under `coqc`, so nothing downstream would notice. /// -/// Code generation is *not* this precise on the very program below. Its -/// call-site extern table (`Compiler::extern_name_to_idx`) is keyed by bare -/// name across the whole program, so `side`'s bound `scale` claims the name and -/// the entry file's own `scale(a)` lowers to `call $libA.scale`. The obligation -/// this test pins is the correct reading; the emitted call is not, and the two -/// describe different functions. That is pre-existing and tracked by -/// [#423](https://github.com/Inferara/inference/issues/423) — recorded here so -/// the assertion below is not mistaken for agreement between the two passes. +/// The two declarations are bound to different libraries because that is what +/// makes the mix-up visible at all: bound to the same one, the wrong answer and +/// the right answer are the same string. A local function of the same name +/// cannot serve as the witness — a top-level function sharing a bare name with a +/// top-level `external fn` anywhere in the program is rejected outright. +/// +/// Code generation resolves this very program through this very index, so the +/// obligation and the emitted call name one function. That agreement is what +/// makes the assertions below worth having: were the two keyed differently, this +/// test would pin the correct reading of a call compiled as something else. #[test] fn an_extern_in_one_file_does_not_capture_a_sibling_files_call() { let ctx = type_check_multi(&[ ( vec![], - "use side;\nfn scale(x: i32) -> i32 { return x * 10; }\nspec MainSpec { fn f() forall \ - { let a: i32 = @; assert(scale(a) == a * 10); } }\n", + "use side;\nexternal fn scale(x: i32) -> i32;\nuse { scale } from libA;\nspec \ + MainSpec { fn f() forall { let a: i32 = @; assert(scale(a) == a); } }\n", ), ( vec!["side"], - "external fn scale(x: i32) -> i32;\nuse { scale } from libA;\npub fn use_side(x: i32) \ - -> i32 { return scale(x); }\n", + "external fn scale(x: i32) -> i32;\nuse { scale } from libB;\nspec SideSpec { fn f() \ + forall { let a: i32 = @; assert(scale(a) == a); } }\n", ), ]); let (map, diagnostics) = translate(&ctx); @@ -2098,8 +2100,13 @@ fn an_extern_in_one_file_does_not_capture_a_sibling_files_call() { ); assert_eq!( applied_symbols(&sole_obligation(&map, "MainSpec")), - vec!["scale".to_string()], - "the entry file's own `scale` — `libA.scale` would be the sibling file's extern" + vec!["libA.scale".to_string()], + "the entry file's spec names the declaration its own file bound" + ); + assert_eq!( + applied_symbols(&sole_obligation(&map, "side_SideSpec")), + vec!["libB.scale".to_string()], + "the sibling's spec names the declaration its own file bound" ); } @@ -2136,16 +2143,19 @@ fn a_spec_inner_function_cannot_shadow_a_top_level_name() { } } -/// The second invariant the extern symbol rests on: one `external fn` name may -/// not be bound to two modules. +/// The second invariant the extern symbol rests on: an `external fn` +/// *declaration* is bound to at most one module. /// -/// Code generation keys its import table on the bare extern *name*, program -/// wide, while an obligation names the module its declaration was bound to. Two -/// bindings for one name would leave every call site invoking a single import -/// while the obligations named two different merged bodies — the executable and -/// the claim would then describe different functions. -#[test] -fn one_extern_name_cannot_be_bound_to_two_modules() { +/// An obligation names the module its declaration was bound to, and a call is +/// emitted against the import that same declaration registered. Both are +/// answers about a declaration, so both stay well defined exactly as long as a +/// *declaration* has one module — not as long as a *name* does. Two files may +/// each declare `scale` and bind it to a different library; the declarations +/// are distinct, so the program is legal and each obligation names its own +/// module. The rule is per file: one file naming one field from two modules is +/// still the conflict, and is covered by the binding pass's own tests. +#[test] +fn one_extern_declaration_is_bound_to_one_module() { let mut arena = AstArena::default(); for (module_path, source) in [ ( @@ -2167,12 +2177,27 @@ fn one_extern_name_cannot_be_bound_to_two_modules() { ); arena = parsed.arena; } - let error = TypeCheckerBuilder::build_typed_context(arena) - .err() - .expect("one extern name bound to two modules must be rejected"); - assert!( - error.to_string().contains("bound to multiple modules"), - "the rejection must be the multiple-binding rule, got: {error}" + let ctx = TypeCheckerBuilder::build_typed_context(arena) + .expect("two files may each bind their own declaration to their own module") + .typed_context(); + let index = ctx.extern_index(); + let in_a = index + .lookup_top_level(&["a".to_string()], "scale") + .expect("file `a` declares `scale`"); + let in_b = index + .lookup_top_level(&["b".to_string()], "scale") + .expect("file `b` declares `scale`"); + assert_ne!( + in_a, in_b, + "the fixture must declare two distinct `scale`s, or this test proves nothing" + ); + assert_eq!( + ctx.extern_origin_by_decl(in_a).map(|o| o.logical_module), + Some("libA".to_string()) + ); + assert_eq!( + ctx.extern_origin_by_decl(in_b).map(|o| o.logical_module), + Some("libB".to_string()) ); } diff --git a/core/wasm-codegen/src/hassert/translate.rs b/core/wasm-codegen/src/hassert/translate.rs index 057f5bd6..f45ae6db 100644 --- a/core/wasm-codegen/src/hassert/translate.rs +++ b/core/wasm-codegen/src/hassert/translate.rs @@ -202,13 +202,14 @@ use inference_ast::nodes::{ }; use inference_fn_key::{FnKey, merged_name}; use inference_hassert::{HAssert, HBinop, HConst, HFnRef, HNumType, HRelop, HTerm}; +use inference_type_checker::ExternIndex; use inference_type_checker::type_info::{NumberType, TypeInfo, TypeInfoKind}; use inference_type_checker::typed_context::TypedContext; use rustc_hash::FxHashMap; +use super::CalleeIndex; use super::diag::{HassertDiagnostic, PCode}; use super::reach::ChoicePlan; -use super::{CalleeIndex, ExternIndex}; /// Polarity of the surrounding quantification. /// @@ -2062,7 +2063,10 @@ impl<'a> SpecFnTranslator<'a> { // rule is ever relaxed, this must become a real innermost-first // walk over both kinds: a file-scope extern would otherwise // hide a spec-sibling function that shadows it. - if let Some(decl) = self.externs.lookup(self.module_path, self.spec_name, &name) { + if let Some(decl) = + self.externs + .lookup(self.module_path, Some(self.spec_name), &name) + { return self.resolve_external(decl); } // A cross-file item import (`use lib::arith::{add}; add()`) @@ -3946,8 +3950,8 @@ mod tests { let buckets = EmittableFunctions::default(); let callee = CalleeIndex::build(ctx.arena(), &buckets); - let externs = ExternIndex::build(ctx.arena()); - let mut translator = SpecFnTranslator::new(&ctx, &[], "S", &callee, &externs); + let externs = ctx.extern_index(); + let mut translator = SpecFnTranslator::new(&ctx, &[], "S", &callee, externs); let unwound = catch_unwind(AssertUnwindSafe(|| translator.number_literal(orphan, "7"))); let payload = unwound.expect_err("an untyped literal must abort translation"); let text = panic_text(payload.as_ref()); @@ -3968,8 +3972,8 @@ mod tests { let buckets = EmittableFunctions::default(); let callee = CalleeIndex::build(ctx.arena(), &buckets); - let externs = ExternIndex::build(ctx.arena()); - let mut translator = SpecFnTranslator::new(&ctx, &[], "S", &callee, &externs); + let externs = ctx.extern_index(); + let mut translator = SpecFnTranslator::new(&ctx, &[], "S", &callee, externs); assert_eq!( translator.number_literal(orphan, "7"), zero_sentinel(), diff --git a/core/wasm-linker/README.md b/core/wasm-linker/README.md index 3d31c70b..76eba6d9 100644 --- a/core/wasm-linker/README.md +++ b/core/wasm-linker/README.md @@ -22,7 +22,8 @@ C-library half of issue #9. For each import in the main module the linker performs these steps: ```text -1. Find which external module exports a function of that name +1. Find which external module exports a function of that name under the + matching logical module 2. Compute the transitive closure of that export inside its source module (the functions it calls, recursively, plus any helpers) 3. Classify the closure's feasibility tier (A, B, or C — see below) diff --git a/core/wasm-linker/src/lib.rs b/core/wasm-linker/src/lib.rs index 0798499d..f73a44d9 100644 --- a/core/wasm-linker/src/lib.rs +++ b/core/wasm-linker/src/lib.rs @@ -12,7 +12,8 @@ //! //! For each import the main module declares, the linker: //! -//! 1. finds the external module that exports a function of that name, +//! 1. finds the external module that exports a function of that name under the +//! matching logical module, //! 2. computes the **transitive closure** of that export inside its module (the //! functions it calls, the types they reference), //! 3. classifies the closure into a **feasibility tier** (see [`tier`]), diff --git a/core/wasm-linker/src/merge.rs b/core/wasm-linker/src/merge.rs index b1d508da..cb52e5b2 100644 --- a/core/wasm-linker/src/merge.rs +++ b/core/wasm-linker/src/merge.rs @@ -279,10 +279,11 @@ impl Plan { } // 2. Resolve each satisfied import to an external export and close over - // it. An import is satisfiable when some external module exports a - // function of the import's field name; the module name is the - // logical module the front-end bound, but the merge keys on the - // field, matching the codegen import contract. + // it. An import is satisfiable when the external bound under its + // logical module exports a function of its field name: the merge + // keys on the full `(module, field)` pair codegen records for every + // import, so two libraries exporting the same field under different + // logical modules are never conflated. let main_import_count = main.imported_funcs.len() as u32; let mut import_target = BTreeMap::new(); let mut merged: Vec = Vec::new(); diff --git a/tests/src/analysis/rules_a024.rs b/tests/src/analysis/rules_a024.rs index a301f67f..80e395d2 100644 --- a/tests/src/analysis/rules_a024.rs +++ b/tests/src/analysis/rules_a024.rs @@ -218,7 +218,7 @@ mod analysis_rules_tests { #[test] fn a024_top_level_use_does_not_bind_spec_inner_extern() { - // H8: a top-level `use { sort } from sorting;` is file-global but binds + // H8: a top-level `use { sort } from sorting;` is file-wide but binds // only top-level externs. With no top-level `sort` declared, the `use` // names an undeclared top-level extern, so the type checker reports // ExternImportNotDeclared rather than silently binding the spec-inner diff --git a/tests/src/codegen/wasm/extern_link.rs b/tests/src/codegen/wasm/extern_link.rs index 6bfc7b25..25038a2f 100644 --- a/tests/src/codegen/wasm/extern_link.rs +++ b/tests/src/codegen/wasm/extern_link.rs @@ -91,6 +91,21 @@ mod extern_link_tests { imports } + /// The number of functions **defined** in `wasm`, excluding imports. + /// + /// After a merge every function is defined, so this counts the program's own + /// functions plus one body per merged external — the number that tells two + /// declarations sharing one import apart from two separate imports. + fn defined_function_count(wasm: &[u8]) -> usize { + let mut count = 0; + for payload in Parser::new(0).parse_all(wasm) { + if let Payload::FunctionSection(reader) = payload.expect("valid payload") { + count += reader.count() as usize; + } + } + count + } + /// Runs the pipeline an `infc -L main.inf -v` invocation runs and /// returns the unified `.wasm` together with its Rocq translation. fn compile_and_link(main_source: &str, lib_dir: &Path, module_name: &str) -> (Vec, String) { @@ -1041,4 +1056,217 @@ pub fn probe_named_array() -> i32 { "the claim the extern is measured against must survive; .v was:\n{rocq}" ); } + + /// Compiles a multi-file program and links its externals, the pipeline an + /// `infs build` of a project runs. + /// + /// `files` is `(module_path, source)` pairs with the entry file first, folded + /// into one arena exactly as the project front end folds a source tree. + fn compile_and_link_multi_file( + files: &[(Vec<&str>, &str)], + lib_dir: &Path, + module_name: &str, + ) -> Vec { + let mut arena = inference_ast::arena::AstArena::default(); + for (module_path, source) in files { + let module_path: Vec = module_path.iter().map(|s| (*s).to_string()).collect(); + let parsed = inference_parser::parse_into(arena, source, module_path); + assert!( + parsed.errors.is_empty(), + "multi-file source has syntax errors: {:?}", + parsed.errors + ); + arena = parsed.arena; + } + let typed = type_check(arena).expect("multi-file source type-checks"); + inference_analysis::analyze(&typed).expect("multi-file analysis succeeds"); + + let mut search_path = SearchPath::new(); + search_path.push_lib_dir(lib_dir.to_path_buf()); + let externals = resolve_external_modules(&typed, &search_path, None) + .expect("external modules resolve and validate"); + let external_bytes: Vec<(&str, &[u8])> = externals + .iter() + .map(|m| (m.logical_module.as_str(), m.bytes.as_slice())) + .collect(); + + let codegen_output = codegen(&typed, module_name).expect("multi-file codegen succeeds"); + link(codegen_output.wasm(), &external_bytes).expect("link succeeds") + } + + /// Instantiates a self-contained merged module and calls `name` with one + /// `i32` argument. + fn call_i32(unified: &[u8], name: &str, argument: i32) -> i32 { + inf_wasmparser::validate(unified).expect("unified module is valid wasm"); + assert!( + function_imports(unified).is_empty(), + "a merged module must be self-contained, found {:?}", + function_imports(unified) + ); + let engine = Engine::default(); + let module = + Module::new(&engine, unified).unwrap_or_else(|e| panic!("merged module rejected: {e}")); + let mut store = Store::new(&engine, ()); + let instance = Instance::new(&mut store, &module, &[]) + .unwrap_or_else(|e| panic!("merged module failed to instantiate: {e}")); + let func: TypedFunc = instance + .get_typed_func(&mut store, name) + .unwrap_or_else(|e| panic!("merged module must export `{name}`: {e}")); + func.call(&mut store, argument) + .unwrap_or_else(|e| panic!("`{name}({argument})` failed: {e}")) + } + + /// Two files each declare `external fn scale`, bind it to a **different** + /// library, and call it. Executed end to end: each file's call must reach + /// its own library. + /// + /// Nothing before execution can tell the two apart. The declarations agree + /// on name and signature, so the module type-checks, links and validates + /// whichever import each call is wired to; only the returned value says + /// which library ran. `libA` multiplies by 999 and `libB` by 7, values + /// nothing else in the program can produce. + #[test] + fn two_files_binding_one_name_to_two_libraries_each_call_their_own() { + let lib_a = compile_wasm("pub fn scale(a: i32) -> i32 { return a * 999; }", "libA"); + let lib_b = compile_wasm("pub fn scale(a: i32) -> i32 { return a * 7; }", "libB"); + let lib_dir = TempLibDir::new("two-modules"); + lib_dir.write_module(Path::new("libA.wasm"), &lib_a); + lib_dir.write_module(Path::new("libB.wasm"), &lib_b); + + let unified = compile_and_link_multi_file( + &[ + ( + vec![], + "use sib;\n\ + external fn scale(a: i32) -> i32;\n\ + use { scale } from libA;\n\ + pub fn from_a(x: i32) -> i32 { return scale(x); }\n\ + pub fn from_b(x: i32) -> i32 { return sib::via_b(x); }\n", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\n\ + use { scale } from libB;\n\ + pub fn via_b(x: i32) -> i32 { return scale(x); }\n", + ), + ], + lib_dir.path(), + "two_modules", + ); + + assert_eq!( + call_i32(&unified, "from_a", 2), + 1998, + "the entry file's `scale` is bound to `libA`, so `from_a(2)` is 2 * 999" + ); + assert_eq!( + call_i32(&unified, "from_b", 2), + 14, + "the sibling's `scale` is bound to `libB`, so `from_b(2)` is 2 * 7; \ + 1998 would mean both files' calls funnelled into one import" + ); + } + + /// Two files declaring and binding the same `(module, field)` share one + /// import, and both calls reach it. + /// + /// The two declarations are distinct, but the import they need is the same + /// one; the merge folds a single `libA.scale` body in, and neither call is + /// left pointing at a second copy or at nothing. + #[test] + fn two_files_binding_one_library_share_a_single_merged_body() { + let lib_a = compile_wasm("pub fn scale(a: i32) -> i32 { return a * 999; }", "libA"); + let lib_dir = TempLibDir::new("one-module"); + lib_dir.write_module(Path::new("libA.wasm"), &lib_a); + + let unified = compile_and_link_multi_file( + &[ + ( + vec![], + "use sib;\n\ + external fn scale(a: i32) -> i32;\n\ + use { scale } from libA;\n\ + pub fn direct(x: i32) -> i32 { return scale(x); }\n\ + pub fn through_sibling(x: i32) -> i32 { return sib::via(x); }\n", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\n\ + use { scale } from libA;\n\ + pub fn via(x: i32) -> i32 { return scale(x); }\n", + ), + ], + lib_dir.path(), + "one_module", + ); + + assert_eq!(call_i32(&unified, "direct", 2), 1998); + assert_eq!(call_i32(&unified, "through_sibling", 2), 1998); + assert_eq!( + defined_function_count(&unified), + 4, + "`direct`, `through_sibling`, `via` and one merged `libA.scale`: the \ + two declarations must share a single import, not merge two copies" + ); + } + + /// Two files bind the same `(module, field)` and **disagree** on its arity. + /// The program must be rejected. + /// + /// The linker satisfies an import on `(module, field)` alone and compares no + /// signatures, so a declaration whose signature never reaches validation is + /// linked to the library regardless. Arity is the shape that makes the + /// consequence an artifact rather than a late error: re-validating the + /// merged module catches a disagreement in value *types*, but a surplus + /// operand pushed into a narrower callee leaves the module well typed, + /// because `return` is stack-polymorphic. The merged program then runs, and + /// the extra argument is silently discarded. + /// + /// So validation has to reach *every* bound declaration, not one per + /// `(module, field)`. + #[test] + fn two_files_disagreeing_on_one_externs_signature_are_rejected() { + let lib_a = compile_wasm("pub fn scale(a: i32) -> i32 { return a * 999; }", "libA"); + let lib_dir = TempLibDir::new("signature-clash"); + lib_dir.write_module(Path::new("libA.wasm"), &lib_a); + + let mut arena = inference_ast::arena::AstArena::default(); + for (module_path, source) in [ + ( + Vec::new(), + "use sib;\n\ + external fn scale(a: i32) -> i32;\n\ + use { scale } from libA;\n\ + pub fn direct(x: i32) -> i32 { return scale(x); }\n\ + pub fn through_sibling(a: i32, b: i32) -> i32 { return sib::via(a, b); }\n", + ), + ( + vec!["sib".to_string()], + "external fn scale(a: i32, b: i32) -> i32;\n\ + use { scale } from libA;\n\ + pub fn via(a: i32, b: i32) -> i32 { return scale(a, b); }\n", + ), + ] { + let parsed = inference_parser::parse_into(arena, source, module_path); + assert!( + parsed.errors.is_empty(), + "multi-file source has syntax errors: {:?}", + parsed.errors + ); + arena = parsed.arena; + } + let typed = type_check(arena).expect("both declarations are well formed on their own"); + + let mut search_path = SearchPath::new(); + search_path.push_lib_dir(lib_dir.path().to_path_buf()); + let error = resolve_external_modules(&typed, &search_path, None) + .expect_err("a declaration the library does not match must be rejected"); + let rendered = error.to_string(); + assert!( + rendered.contains("signature mismatch for external function `scale`") + && rendered.contains("declared (i32, i32) -> (i32)"), + "the rejection must name the mismatching extern and the signature that \ + disagrees; got: {rendered}" + ); + } } diff --git a/tests/src/codegen/wasm/mod.rs b/tests/src/codegen/wasm/mod.rs index 428090fe..fec58c58 100644 --- a/tests/src/codegen/wasm/mod.rs +++ b/tests/src/codegen/wasm/mod.rs @@ -33,6 +33,7 @@ mod literal_ctx_i64; mod literal_ctx_nested_array; mod loops; mod multi_file; +mod multi_file_extern; mod multi_file_golden; mod negative; mod param_by_ref; diff --git a/tests/src/codegen/wasm/multi_file_extern.rs b/tests/src/codegen/wasm/multi_file_extern.rs new file mode 100644 index 00000000..4ea3be12 --- /dev/null +++ b/tests/src/codegen/wasm/multi_file_extern.rs @@ -0,0 +1,259 @@ +//! Multi-file `external fn` scoping, executed through Wasmtime. +//! +//! An `external fn` is declared in one file and can be named only there. The +//! code generation seam that has to respect that is the call-site probe: a bare +//! callee name is resolved to the declaration the *calling* file can see before +//! it becomes an import index. A whole-program name table gets that wrong in a +//! way nothing downstream reports — the module validates either way, and a call +//! to an import type-checks against the same signature, so the only observable +//! difference is the value that comes back. +//! +//! Two rules meet here, and both are pinned below. A bare name shared by an +//! `external fn` and a function is rejected outright, so the program that +//! motivated this file never reaches code generation. A bare name shared by two +//! `external fn`s in two files stays legal, and *that* is the program the probe +//! still has to resolve per file. +//! +//! The foreign modules are supplied as host functions rather than statically +//! linked, so each import is backed by a body whose result is unmistakable: +//! `libA` multiplies by 999 and `libB` by 7, values nothing in the program +//! produces. + +use crate::utils::{ + proof_wasm_codegen_multi_file, try_type_check_multi_file, wasm_codegen_multi_file, +}; + +use wasmtime::{Engine, Instance, Linker, Module, Store, TypedFunc}; + +/// The entry file defines its own `scale` and calls it. The sibling declares an +/// `external fn scale` and binds it to `libA`. Both calls are written +/// `scale(..)`, and only the file each is written in says which declaration it +/// means — which is what makes the shared name unreadable, and why the program +/// is rejected even though each call resolves correctly. +const ENTRY: &str = "\ +use side; + +fn scale(x: i32) -> i32 { + return x * 10; +} + +pub fn run(x: i32) -> i32 { + return scale(x); +} +"; + +const SIDE: &str = "\ +external fn scale(a: i32) -> i32; +use { scale } from libA; + +pub fn boost(a: i32) -> i32 { + return scale(a); +} +"; + +/// Instantiates `wasm_bytes` with `libA.scale` and `libB.scale` supplied by the +/// host. A module that imports only one of them still instantiates: the linker +/// is asked for the names the module names. +fn instantiate_with_host_libraries(wasm_bytes: &[u8]) -> (Store<()>, Instance) { + inf_wasmparser::validate(wasm_bytes) + .unwrap_or_else(|e| panic!("generated multi-file Wasm module is invalid: {e}")); + let engine = Engine::default(); + let module = Module::new(&engine, wasm_bytes) + .unwrap_or_else(|e| panic!("failed to create Wasm module: {e}")); + let mut linker = Linker::new(&engine); + linker + .func_wrap("libA", "scale", |a: i32| a * 999) + .expect("libA.scale is supplied by the host"); + linker + .func_wrap("libB", "scale", |a: i32| a * 7) + .expect("libB.scale is supplied by the host"); + let mut store = Store::new(&engine, ()); + let instance = linker + .instantiate(&mut store, &module) + .unwrap_or_else(|e| panic!("failed to instantiate Wasm module: {e}")); + (store, instance) +} + +/// The entry file's `fn scale` and the sibling's `external fn scale` share one +/// bare name, and the program is rejected before code generation. +/// +/// The rejection is about the name, not about resolution: once the call-site +/// probe was keyed on the declaring file this program compiled and `run(2)` came +/// back 20, the entry's own `scale`. It is rejected because a reader cannot see +/// from `scale(x)` whether the callee is compiled here or linked in — so the +/// message states that rule and names both declarations, rather than reporting +/// a resolution the compiler in fact performs. +#[test] +fn a_siblings_extern_and_the_entry_files_function_collide() { + let Err(err) = try_type_check_multi_file(&[(vec![], ENTRY), (vec!["side"], SIDE)]) else { + panic!("a function and an `external fn` of one name must be rejected"); + }; + let message = err.to_string(); + assert!( + message.contains("side:1:1: `external fn scale` and the function `scale` share one name") + && message.contains("note: the function `scale` is defined at 3:1 in the entry file"), + "both declarations must be named with their locations: {message}" + ); +} + +/// Two files each declare `external fn scale` and bind it to a **different** +/// library. Each file's call must reach the import its own declaration +/// registered. +/// +/// This is the program a whole-program name table cannot express: it holds one +/// `scale` for the whole program, so one of the two files ends up calling the +/// other's library. Nothing before execution tells the two apart — the +/// declarations agree on name and signature, so the module type-checks and +/// validates whichever import each call is wired to. +#[test] +fn two_files_binding_one_name_call_their_own_library() { + const ENTRY_TWO_LIBRARIES: &str = "\ +use side; + +external fn scale(a: i32) -> i32; +use { scale } from libA; + +pub fn from_a(x: i32) -> i32 { + return scale(x); +} + +pub fn from_b(x: i32) -> i32 { + return side::via_b(x); +} +"; + + const SIDE_TWO_LIBRARIES: &str = "\ +external fn scale(a: i32) -> i32; +use { scale } from libB; + +pub fn via_b(x: i32) -> i32 { + return scale(x); +} +"; + + let wasm = wasm_codegen_multi_file(&[ + (vec![], ENTRY_TWO_LIBRARIES), + (vec!["side"], SIDE_TWO_LIBRARIES), + ]); + let (mut store, instance) = instantiate_with_host_libraries(&wasm); + let from_a: TypedFunc = instance + .get_typed_func(&mut store, "from_a") + .expect("`from_a` is exported from the entry file"); + let from_b: TypedFunc = instance + .get_typed_func(&mut store, "from_b") + .expect("`from_b` is exported from the entry file"); + assert_eq!( + from_a.call(&mut store, 2).expect("from_a(2) executes"), + 1998, + "the entry file's `scale` is bound to `libA`, so `from_a(2)` is 2 * 999" + ); + assert_eq!( + from_b.call(&mut store, 2).expect("from_b(2) executes"), + 14, + "the sibling's `scale` is bound to `libB`, so `from_b(2)` is 2 * 7" + ); +} + +/// The entry file declares, binds and calls its own `external fn scale`; the +/// sibling declares one too and never binds or calls it. +/// +/// The sibling's declaration is inert — nothing in the program refers to it — +/// and the entry file's program is byte-identical to one where the sibling does +/// not exist. A whole-program name table holds one declaration per name, so the +/// inert one can displace the bound one and leave the entry's working call +/// unbound: rejected by A024 if analysis runs, and reaching nothing if it does +/// not. +#[test] +fn an_inert_sibling_declaration_does_not_unbind_the_entrys_call() { + const ENTRY_BOUND: &str = "\ +use side; + +external fn scale(a: i32) -> i32; +use { scale } from libA; + +pub fn run(x: i32) -> i32 { + return scale(x) + side::helper(); +} +"; + + const SIDE_INERT: &str = "\ +external fn scale(a: i32) -> i32; + +pub fn helper() -> i32 { + return 0; +} +"; + + let wasm = wasm_codegen_multi_file(&[(vec![], ENTRY_BOUND), (vec!["side"], SIDE_INERT)]); + let (mut store, instance) = instantiate_with_host_libraries(&wasm); + let run: TypedFunc = instance + .get_typed_func(&mut store, "run") + .expect("`run` is exported from the entry file"); + assert_eq!( + run.call(&mut store, 2).expect("run(2) executes"), + 1998, + "the entry file's own binding must survive an inert sibling declaration" + ); +} + +/// A `spec`-inner function shares its bare name with an `external fn` another +/// file declares and binds. The call written beside it inside the `spec` must +/// reach that sibling function, not the import the name registered elsewhere. +/// +/// This is the shape where the two answers still differ and neither is an error. +/// A *top-level* function sharing a name with an `external fn` is rejected +/// outright, so the collision can no longer hide a wrong call — but a +/// `spec`-inner declaration is namespaced under its `spec` and takes no part in +/// that rule, so this program is legal and the call has a local target to reach. +/// A probe that widened back out to a program-wide name table would find the +/// import, emit `call `, and produce a module that validates and +/// links: the obligation would then be about the foreign body rather than the +/// function the specification is written about. `wasm_codegen_extern_out_of_scope` +/// is what records that the probe saw the name as an import elsewhere and still +/// declined it here. +/// +/// Proof mode, because compile mode emits no specification function at all. +#[test] +fn a_spec_inner_function_is_not_displaced_by_a_siblings_import() { + cov_mark::check_count!(wasm_codegen_extern_out_of_scope, 1); + + const ENTRY_BINDS_SCALE: &str = "\ +use side; + +external fn scale(a: i32) -> i32; +use { scale } from libA; + +pub fn from_a(x: i32) -> i32 { + return scale(x); +} + +pub fn touch(x: i32) -> i32 { + return side::helper(x); +} +"; + + const SIDE_SPEC_SCALE: &str = "\ +spec S { + fn scale(a: i32) -> i32 { + assert(a == a); + return a * 10; + } + + fn probe() forall { + let x: i32 = @; + assert(scale(x) == x * 10); + } +} + +pub fn helper(x: i32) -> i32 { + return x; +} +"; + + let wasm = proof_wasm_codegen_multi_file(&[ + (vec![], ENTRY_BINDS_SCALE), + (vec!["side"], SIDE_SPEC_SCALE), + ]); + inf_wasmparser::validate(&wasm) + .unwrap_or_else(|e| panic!("proof-mode multi-file module is invalid: {e}")); +} diff --git a/tests/src/type_checker/extern_binding.rs b/tests/src/type_checker/extern_binding.rs new file mode 100644 index 00000000..35e26eca --- /dev/null +++ b/tests/src/type_checker/extern_binding.rs @@ -0,0 +1,249 @@ +//! Tests for the `use … from` binding pass across files. +//! +//! A `use { f } from m;` clause names fields of a logical module and binds the +//! `external fn` declarations of **its own file**. That scope is what these +//! pin: a clause must not reach a sibling's declaration, a sibling's +//! declaration must not intercept a clause meant for a local one, and the +//! per-file consistency rule (one name, one module) must stay a per-file rule +//! rather than a program-wide one. +//! +//! The bindings are read back by declaration [`DefId`], because that is what a +//! binding attaches to. Two files may declare `external fn scale` and bind them +//! to different modules; asked by name the program has two answers, and only +//! the declaration says which is which. +#[cfg(test)] +mod tests { + use crate::utils::{build_ast, try_type_check_multi_file}; + use inference_ast::ids::DefId; + use inference_ast::nodes::Def; + use inference_type_checker::check_with_diagnostics; + use inference_type_checker::errors::TypeCheckError; + use inference_type_checker::typed_context::TypedContext; + + /// The entry file's module path. + const ENTRY: &[String] = &[]; + + fn module(segments: &[&str]) -> Vec { + segments.iter().map(|s| (*s).to_string()).collect() + } + + /// The [`DefId`] of the top-level `external fn name` declared by the file at + /// `module_path`. + /// + /// Walks the arena rather than asking the extern index, so an assertion + /// about a binding never leans on the resolver that produced it. + fn declaration(ctx: &TypedContext, module_path: &[String], name: &str) -> DefId { + let arena = ctx.arena(); + let file = ctx + .source_files() + .find(|sf| sf.module_path == module_path) + .unwrap_or_else(|| panic!("the fixture has no file at {module_path:?}")); + file.defs + .iter() + .copied() + .find(|&def_id| match &arena[def_id].kind { + Def::ExternFunction { + name: decl_name, .. + } => arena[*decl_name].name == name, + _ => false, + }) + .unwrap_or_else(|| { + panic!("the file at {module_path:?} declares no `external fn {name}`") + }) + } + + /// The logical module the declaration of `name` in `module_path` is bound + /// to, or `None` when that declaration is unbound. + fn bound_module(ctx: &TypedContext, module_path: &[String], name: &str) -> Option { + let decl = declaration(ctx, module_path, name); + ctx.extern_origin_by_decl(decl).map(|o| o.logical_module) + } + + fn errors(files: &[(Vec<&str>, &str)]) -> String { + match try_type_check_multi_file(files) { + Ok(_) => panic!("the fixture should be rejected"), + Err(e) => e.to_string(), + } + } + + /// A `use … from` clause binds a declaration in its own file only. + /// + /// The entry file names `scale` with nothing of that name declared in it; + /// that the *sibling* declares one is irrelevant, and treating it as a + /// candidate silently attaches the entry's clause to a declaration the entry + /// file cannot even name. + #[test] + fn a_use_clause_does_not_reach_a_siblings_declaration() { + let rendered = errors(&[ + ( + vec![], + "use sib;\nuse { scale } from libA;\npub fn go(x: i32) -> i32 { return \ + sib::via(x); }", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\npub fn via(x: i32) -> i32 { return scale(x); }", + ), + ]); + assert!( + rendered.contains("imports `scale` from module `libA`") + && rendered.contains("no `external fn scale` is declared"), + "binding across a file boundary must be a dangling import, got: {rendered}" + ); + } + + /// A sibling's unrelated declaration does not intercept a binding. + /// + /// The sibling declares `scale` and never binds or calls it. The entry file + /// declares and binds its own. A program-wide name table holds one entry, so + /// whichever file it happens to keep decides which declaration the entry's + /// clause attaches to — leaving the entry's own declaration unbound and its + /// working call rejected. + #[test] + fn an_unrelated_sibling_declaration_does_not_intercept_a_binding() { + let ctx = try_type_check_multi_file(&[ + ( + vec![], + "use sib;\nexternal fn scale(a: i32) -> i32;\nuse { scale } from libA;\npub fn \ + go(x: i32) -> i32 { return scale(x) + sib::helper(); }", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\npub fn helper() -> i32 { return 7; }", + ), + ]) + .expect("an unbound sibling declaration must not reject the entry's binding"); + assert_eq!(bound_module(&ctx, ENTRY, "scale").as_deref(), Some("libA")); + assert_eq!( + bound_module(&ctx, &module(&["sib"]), "scale"), + None, + "the sibling declares `scale` without binding it, so it stays unbound" + ); + } + + /// Two files may each declare `external fn scale` and bind it to a + /// *different* module. The declarations are distinct, so the two bindings + /// are not a conflict — the linker names the merged roots per module. + #[test] + fn two_files_may_bind_one_name_to_different_modules() { + let ctx = try_type_check_multi_file(&[ + ( + vec![], + "use sib;\nexternal fn scale(a: i32) -> i32;\nuse { scale } from libA;\npub fn \ + from_a(x: i32) -> i32 { return scale(x); }\npub fn from_b(x: i32) -> i32 { \ + return sib::via_b(x); }", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\nuse { scale } from libB;\npub fn via_b(x: \ + i32) -> i32 { return scale(x); }", + ), + ]) + .expect("two files binding one name to two modules is legal"); + assert_eq!(bound_module(&ctx, ENTRY, "scale").as_deref(), Some("libA")); + assert_eq!( + bound_module(&ctx, &module(&["sib"]), "scale").as_deref(), + Some("libB") + ); + } + + /// The commonest shape of all: two files both use the same library + /// function. Each declares its own extern and binds it to the same module. + #[test] + fn two_files_may_bind_one_name_to_the_same_module() { + let ctx = try_type_check_multi_file(&[ + ( + vec![], + "use sib;\nexternal fn scale(a: i32) -> i32;\nuse { scale } from libA;\npub fn \ + a(x: i32) -> i32 { return scale(x); }\npub fn b(x: i32) -> i32 { return \ + sib::via(x); }", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\nuse { scale } from libA;\npub fn via(x: i32) \ + -> i32 { return scale(x); }", + ), + ]) + .expect("two files binding one name to the same module is legal"); + assert_eq!(bound_module(&ctx, ENTRY, "scale").as_deref(), Some("libA")); + assert_eq!( + bound_module(&ctx, &module(&["sib"]), "scale").as_deref(), + Some("libA") + ); + } + + /// The per-file consistency rule survives the narrowing: one file naming one + /// field from two modules is still ambiguous, and its declaration is left + /// unbound rather than resolved to an arbitrary one of the two. + #[test] + fn one_file_binding_one_name_to_two_modules_is_ambiguous() { + let rendered = errors(&[ + ( + vec![], + "use sib;\nexternal fn scale(a: i32) -> i32;\nuse { scale } from libA;\nuse { \ + scale } from libB;\npub fn go(x: i32) -> i32 { return scale(x) + sib::helper(); }", + ), + (vec!["sib"], "pub fn helper() -> i32 { return 7; }"), + ]); + assert!( + rendered.contains("external function `scale` is bound to multiple modules") + && rendered.contains("`libA`") + && rendered.contains("`libB`"), + "a within-file conflict must still be rejected, got: {rendered}" + ); + } + + /// An ambiguity inside an imported file names that file. The clause, the + /// declaration and the diagnostic all belong to one file now, so the label + /// cannot drift to whichever file the scan reached first. + #[test] + fn an_ambiguity_in_an_imported_file_names_that_file() { + let rendered = errors(&[ + ( + vec![], + "use sib;\npub fn go(x: i32) -> i32 { return sib::via(x); }", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\nuse { scale } from libA;\nuse { scale } from \ + libB;\npub fn via(x: i32) -> i32 { return scale(x); }", + ), + ]); + assert!( + rendered.contains("sib:") && rendered.contains("bound to multiple modules"), + "the offending file must be named, got: {rendered}" + ); + } + + /// Binding diagnostics are reported in source order. + /// + /// They are produced by draining a name-keyed map, so without an explicit + /// ordering step four dangling imports written on consecutive lines report + /// in hash order — a list the reader cannot follow back into the file. + #[test] + fn dangling_import_diagnostics_are_reported_in_source_order() { + let source = "use { alpha } from libA;\nuse { beta } from libA;\nuse { gamma } from \ + libA;\nuse { delta } from libA;\nfn main() -> i32 { return 0; }"; + let outcome = check_with_diagnostics(build_ast(source.to_string())); + let reported: Vec<(u32, String)> = outcome + .errors + .iter() + .filter_map(|d| match &d.error { + TypeCheckError::ExternImportNotDeclared { name, location, .. } => { + Some((location.start_line, name.clone())) + } + _ => None, + }) + .collect(); + assert_eq!( + reported, + vec![ + (1, "alpha".to_string()), + (2, "beta".to_string()), + (3, "gamma".to_string()), + (4, "delta".to_string()), + ], + "dangling imports must report in the order they are written" + ); + } +} diff --git a/tests/src/type_checker/extern_index.rs b/tests/src/type_checker/extern_index.rs new file mode 100644 index 00000000..ea12e788 --- /dev/null +++ b/tests/src/type_checker/extern_index.rs @@ -0,0 +1,211 @@ +//! Tests for the shared `external fn` index: which declaration a bare name +//! means at a given point in the program. +//! +//! An `external fn`'s identity is the declaration a use site names, not its +//! bare name — two declarations may share a name and agree on nothing else. +//! Every consumer (analysis rule A024, the specification translator) resolves +//! through this one index, so these tests pin the resolution all of them get. +#[cfg(test)] +mod tests { + use crate::utils::{build_ast, try_type_check_multi_file}; + use inference_ast::ids::DefId; + use inference_ast::nodes::Def; + use inference_type_checker::TypeCheckerBuilder; + use inference_type_checker::typed_context::TypedContext; + + /// The entry file's module path. + const ENTRY: &[String] = &[]; + + fn type_check(source: &str) -> TypedContext { + TypeCheckerBuilder::build_typed_context(build_ast(source.to_string())) + .expect("test source should type-check") + .typed_context() + } + + fn module(segments: &[&str]) -> Vec { + segments.iter().map(|s| (*s).to_string()).collect() + } + + /// The [`DefId`] of the `external fn` named `name` declared directly in the + /// given scope — the file at `module_path`, or the `spec` block of that name + /// within it. + /// + /// Found by walking the arena, so an assertion comparing against it never + /// leans on the index it is checking. + fn declaration( + ctx: &TypedContext, + module_path: &[String], + spec: Option<&str>, + name: &str, + ) -> DefId { + let arena = ctx.arena(); + let file = ctx + .source_files() + .find(|sf| sf.module_path == module_path) + .unwrap_or_else(|| panic!("the fixture has no file at {module_path:?}")); + let defs: &[DefId] = match spec { + None => &file.defs, + Some(spec) => file + .defs + .iter() + .find_map(|&def_id| match &arena[def_id].kind { + Def::Spec { + name: spec_name, + defs, + .. + } if arena[*spec_name].name == spec => Some(defs.as_slice()), + _ => None, + }) + .unwrap_or_else(|| panic!("the fixture has no `spec {spec}`")), + }; + defs.iter() + .copied() + .find(|&def_id| match &arena[def_id].kind { + Def::ExternFunction { + name: decl_name, .. + } => arena[*decl_name].name == name, + _ => false, + }) + .unwrap_or_else(|| panic!("the fixture declares no `external fn {name}` in that scope")) + } + + /// A top-level declaration resolves from the file's top level, and both + /// entry points agree there: `lookup` with no enclosing spec is + /// `lookup_top_level`. + #[test] + fn a_top_level_declaration_resolves_at_the_top_level() { + let ctx = type_check("external fn print(val: i32) -> (); fn main() { }"); + let decl = declaration(&ctx, ENTRY, None, "print"); + let index = ctx.extern_index(); + assert_eq!(index.lookup(ENTRY, None, "print"), Some(decl)); + assert_eq!(index.lookup_top_level(ENTRY, "print"), Some(decl)); + } + + /// A name that is a plain function, not an extern, resolves to nothing — + /// the index answers "which extern", never "which function". + #[test] + fn a_plain_function_is_not_an_extern() { + let ctx = type_check("fn helper(x: i32) -> i32 { return x; }"); + assert_eq!(ctx.extern_index().lookup(ENTRY, None, "helper"), None); + } + + /// Two same-named declarations, one per scope: a call inside the spec means + /// the spec's, a call at the top level means the file's. This is the whole + /// reason resolution cannot be name-keyed — the two differ in arity, type + /// and binding. + #[test] + fn a_spec_declaration_shadows_a_same_named_top_level_one() { + let ctx = type_check( + "external fn sort(a: i32) -> i32; spec Ms { external fn sort(a: i64, b: i64) -> i64; }", + ); + let top_level = declaration(&ctx, ENTRY, None, "sort"); + let spec_inner = declaration(&ctx, ENTRY, Some("Ms"), "sort"); + assert_ne!( + top_level, spec_inner, + "the fixture must declare two distinct `sort`s, or this test proves nothing" + ); + let index = ctx.extern_index(); + assert_eq!(index.lookup(ENTRY, Some("Ms"), "sort"), Some(spec_inner)); + assert_eq!(index.lookup(ENTRY, None, "sort"), Some(top_level)); + } + + /// `lookup_top_level` never descends into a `spec`. A `use … from` clause is + /// file-scoped and binds top-level declarations only, so a spec-inner + /// declaration must stay invisible to it — otherwise a top-level `use` would + /// silently bind an extern its own scope never declared. + #[test] + fn lookup_top_level_never_reaches_into_a_spec() { + let ctx = type_check("spec Ms { external fn sort(a: i32) -> i32; }"); + let spec_inner = declaration(&ctx, ENTRY, Some("Ms"), "sort"); + let index = ctx.extern_index(); + assert_eq!( + index.lookup(ENTRY, Some("Ms"), "sort"), + Some(spec_inner), + "the declaration must be reachable from inside its own spec" + ); + assert_eq!(index.lookup_top_level(ENTRY, "sort"), None); + assert_eq!(index.lookup(ENTRY, None, "sort"), None); + } + + /// A spec that declares no extern of its own falls back to the file's top + /// level: the walk is innermost-first, not innermost-only. + #[test] + fn a_spec_falls_back_to_the_file_top_level() { + let ctx = type_check( + "external fn sort(a: i32) -> i32; spec Ms { fn run(x: i32) -> i32 { return x; } }", + ); + let top_level = declaration(&ctx, ENTRY, None, "sort"); + assert_eq!( + ctx.extern_index().lookup(ENTRY, Some("Ms"), "sort"), + Some(top_level) + ); + } + + /// Sibling specs are isolated: one spec's declaration is not in scope in + /// another. + #[test] + fn sibling_specs_do_not_see_each_others_declarations() { + let ctx = type_check( + "spec A { external fn f(x: i32) -> i32; } spec B { fn g(x: i32) -> i32 { return x; } }", + ); + let in_a = declaration(&ctx, ENTRY, Some("A"), "f"); + let index = ctx.extern_index(); + assert_eq!(index.lookup(ENTRY, Some("A"), "f"), Some(in_a)); + assert_eq!(index.lookup(ENTRY, Some("B"), "f"), None); + } + + /// Two files of one program may each declare an `external fn` of the same + /// name. They are distinct declarations and each file resolves the name to + /// its own — a program-wide map keyed by the bare name could hold only one + /// of the two. + #[test] + fn same_named_declarations_in_two_files_stay_distinct() { + let files = [ + ( + vec![], + "use lib; external fn scale(x: i32) -> i32; pub fn main() -> i32 { return lib::helper(1); }", + ), + ( + vec!["lib"], + "external fn scale(x: i32) -> i32; pub fn helper(x: i32) -> i32 { return x; }", + ), + ]; + let ctx = + try_type_check_multi_file(&files).expect("the two-file fixture should type-check"); + let lib = module(&["lib"]); + let in_entry = declaration(&ctx, ENTRY, None, "scale"); + let in_lib = declaration(&ctx, &lib, None, "scale"); + assert_ne!( + in_entry, in_lib, + "the fixture must declare two distinct `scale`s, or this test proves nothing" + ); + let index = ctx.extern_index(); + assert_eq!(index.lookup_top_level(ENTRY, "scale"), Some(in_entry)); + assert_eq!(index.lookup_top_level(&lib, "scale"), Some(in_lib)); + } + + /// A declaration is visible only in the file that makes it: an extern + /// declared in the entry file does not resolve from an imported one, and + /// vice versa. + #[test] + fn a_declaration_is_invisible_from_another_file() { + let files = [ + ( + vec![], + "use lib; external fn scale(x: i32) -> i32; pub fn main() -> i32 { return lib::helper(1); }", + ), + ( + vec!["lib"], + "external fn shift(x: i32) -> i32; pub fn helper(x: i32) -> i32 { return x; }", + ), + ]; + let ctx = + try_type_check_multi_file(&files).expect("the two-file fixture should type-check"); + let index = ctx.extern_index(); + let lib = module(&["lib"]); + assert!(index.lookup_top_level(ENTRY, "scale").is_some()); + assert!(index.lookup_top_level(&lib, "shift").is_some()); + assert_eq!(index.lookup_top_level(&lib, "scale"), None); + assert_eq!(index.lookup_top_level(ENTRY, "shift"), None); + } +} diff --git a/tests/src/type_checker/extern_name_collision.rs b/tests/src/type_checker/extern_name_collision.rs new file mode 100644 index 00000000..1e7bc8a7 --- /dev/null +++ b/tests/src/type_checker/extern_name_collision.rs @@ -0,0 +1,288 @@ +//! A top-level `external fn` and a top-level function may not share a name. +//! +//! The rejection is not a resolution failure. A bare call resolves in the scope +//! it is written in, so such a program has one callee per call site and would +//! run correctly; the pair is rejected because a local function shadowing a +//! foreign-boundary declaration is hard to read — a call site does not say +//! whether the callee is compiled here or linked in. That is a property of the +//! spelling, so the rule spans the whole program. Within one file it also +//! replaces the symbol table's refusal of the second insert, whose message named +//! neither declaration. +//! +//! Both declarations and both locations are named, because neither is at fault +//! on its own — renaming either resolves it. The negative controls below matter +//! as much as the positive ones: the rule is about *these two* top-level kinds, +//! and every neighbouring shape (two externs, two functions, a method, a +//! `spec`-inner function) stays outside it. + +use crate::utils::{build_ast, build_multi_file_ast, try_type_check_multi_file}; +use inference_type_checker::check_with_diagnostics; +use inference_type_checker::errors::TypeCheckError; + +/// Every collision reported for `files`, as +/// `(file the diagnostic belongs to, extern line, function line, file named in the note)`. +fn collisions(files: &[(Vec<&str>, &str)]) -> Vec<(Option, u32, u32, Option)> { + check_with_diagnostics(build_multi_file_ast(files)) + .errors + .into_iter() + .filter_map(|d| match d.error { + TypeCheckError::ExternFunctionNameCollision { + location, + function_location, + function_file, + .. + } => Some(( + d.file_label, + location.start_line, + function_location.start_line, + function_file, + )), + _ => None, + }) + .collect() +} + +fn single_file_errors(source: &str) -> Vec { + check_with_diagnostics(build_ast(source.to_string())) + .errors + .into_iter() + .map(|d| d.error) + .collect() +} + +/// The declaration order must not decide which site is reported: the extern is +/// always the error and the function always the note, so the message reads the +/// same whichever the user wrote first. +#[test] +fn one_file_declaring_both_is_rejected_in_either_order() { + const EXTERN_FIRST: &str = "external fn scale(a: i32) -> i32;\nfn scale(x: i32) -> i32 { \ + return x * 10; }\npub fn run(x: i32) -> i32 { return scale(x); }"; + const FUNCTION_FIRST: &str = "fn scale(x: i32) -> i32 { return x * 10; }\nexternal fn \ + scale(a: i32) -> i32;\npub fn run(x: i32) -> i32 { return \ + scale(x); }"; + + assert_eq!( + collisions(&[(vec![], EXTERN_FIRST)]), + vec![(None, 1, 2, None)], + "the extern on line 1 collides with the function on line 2" + ); + assert_eq!( + collisions(&[(vec![], FUNCTION_FIRST)]), + vec![(None, 2, 1, None)], + "the extern on line 2 collides with the function on line 1" + ); +} + +/// The collision replaces the symbol table's generic refusal, which named a +/// symbol and a scope but neither declaration's location. +#[test] +fn the_collision_is_the_whole_report() { + let errors = single_file_errors( + "external fn scale(a: i32) -> i32;\nfn scale(x: i32) -> i32 { return x * 10; }\npub fn \ + run(x: i32) -> i32 { return scale(x); }", + ); + assert!( + matches!( + errors.as_slice(), + [TypeCheckError::ExternFunctionNameCollision { .. }] + ), + "one purpose-built diagnostic and nothing else: {errors:?}" + ); +} + +/// A duplicate that has nothing to do with the extern still reports. The +/// colliding extern is kept out of the symbol table so it cannot raise a second +/// message of its own, and that removal must not swallow an unrelated one. +#[test] +fn an_unrelated_duplicate_function_still_reports() { + let errors = single_file_errors( + "external fn scale(a: i32) -> i32;\nfn scale(x: i32) -> i32 { return x; }\nfn scale(x: \ + i32) -> i32 { return x + 1; }\npub fn run() -> i32 { return 0; }", + ); + assert!( + errors + .iter() + .any(|e| matches!(e, TypeCheckError::ExternFunctionNameCollision { .. })), + "the collision is reported: {errors:?}" + ); + assert!( + errors + .iter() + .any(|e| matches!(e, TypeCheckError::RegistrationFailed { .. })), + "the second `fn scale` is still a duplicate in its own right: {errors:?}" + ); +} + +/// The cross-file half of the rule: the entry file defines `scale` and calls it, +/// an imported file declares and binds an `external fn scale`. Each call +/// resolves in its own file, so nothing rejected this shape — the rule is what +/// rejects it, and it must reach across the file boundary or the two spellings +/// still meet in one program. +#[test] +fn a_function_and_a_siblings_extern_are_rejected() { + assert_eq!( + collisions(&[ + ( + vec![], + "use sib;\nfn scale(x: i32) -> i32 { return x * 10; }\npub fn run(x: i32) -> i32 \ + { return scale(x); }\npub fn via(v: i32) -> i32 { return sib::doubled(v); }", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\nuse { scale } from libA;\npub fn doubled(v: \ + i32) -> i32 { return scale(v); }", + ), + ]), + vec![(Some("sib".to_string()), 1, 2, None)], + "the diagnostic belongs to the declaring file and names the entry file's function" + ); +} + +/// Both sites are readable in the rendered message, which is all a command-line +/// user sees. A second location renders as a bare `line:col`, so the file it +/// belongs to has to be spelled out. +#[test] +fn the_rendered_message_names_both_declarations() { + let Err(err) = try_type_check_multi_file(&[ + ( + vec![], + "use sib;\npub fn via(v: i32) -> i32 { return sib::doubled(v); }", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\nfn scale(x: i32) -> i32 { return x; }\npub fn \ + doubled(v: i32) -> i32 { return scale(v); }", + ), + ]) else { + panic!("a function and an extern of one name must be rejected"); + }; + let message = err.to_string(); + assert!( + message.contains("sib:1:1: `external fn scale` and the function `scale` share one name") + && message.contains("note: the function `scale` is defined at 2:1 in file `sib`"), + "both declarations are named with their locations: {message}" + ); +} + +/// Visibility is irrelevant: a private function is written as the same bare name +/// inside its own file, which is where a reader meets the two spellings. +#[test] +fn a_private_function_collides_as_well() { + assert_eq!( + collisions(&[ + ( + vec![], + "use sib;\npub fn via(v: i32) -> i32 { return sib::doubled(v); }", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\nfn scale(x: i32) -> i32 { return x; }\npub \ + fn doubled(v: i32) -> i32 { return scale(v); }", + ), + ]), + vec![(Some("sib".to_string()), 1, 2, Some("sib".to_string()))], + "a private function in an imported file collides just as a `pub` one does" + ); +} + +/// Two files each declaring `external fn scale` stay legal — the declarations +/// are distinct, each file's calls reach its own, and the linker resolves them +/// per module. Narrowing this to a name-level rule would reject the commonest +/// multi-file shape there is. +#[test] +fn two_files_may_each_declare_an_extern_of_one_name() { + try_type_check_multi_file(&[ + ( + vec![], + "use sib;\nexternal fn scale(a: i32) -> i32;\nuse { scale } from libA;\npub fn \ + from_a(x: i32) -> i32 { return scale(x); }\npub fn from_b(x: i32) -> i32 { return \ + sib::via_b(x); }", + ), + ( + vec!["sib"], + "external fn scale(a: i32) -> i32;\nuse { scale } from libB;\npub fn via_b(x: i32) \ + -> i32 { return scale(x); }", + ), + ]) + .expect("two files each declaring and binding their own `scale` is legal"); +} + +/// Two files each defining `fn scale` stay legal: same-named items in different +/// files are different entities, reached by their own file's namespace. +#[test] +fn two_files_may_each_define_a_function_of_one_name() { + try_type_check_multi_file(&[ + ( + vec![], + "use sib;\npub fn scale(x: i32) -> i32 { return x * 10; }\npub fn both(x: i32) -> \ + i32 { return scale(x) + sib::scale(x); }", + ), + (vec!["sib"], "pub fn scale(x: i32) -> i32 { return x * 2; }"), + ]) + .expect("two files each defining their own `scale` is legal"); +} + +/// A method is written with a receiver, so `p.scale()` and `scale(1)` are two +/// visibly different call forms and the rule must not reach it. +#[test] +fn a_struct_method_may_share_a_name_with_an_extern() { + try_type_check_multi_file(&[( + vec![], + "external fn scale(a: i32) -> i32;\nuse { scale } from libA;\nstruct P { v: i32; fn \ + scale(self) -> i32 { return self.v; } }\npub fn run() -> i32 { let p: P = P { v: 3 }; \ + return p.scale() + scale(1); }", + )]) + .expect("a method named `scale` beside an `external fn scale` is legal"); +} + +/// The rule is about two *top-level* declarations. A `spec`-inner function is +/// governed by the separate spec/top-level shadowing rule, which rejects this +/// program on its own terms; the collision check must stay out of it rather than +/// stack a second message on top. +#[test] +fn a_spec_inner_function_is_not_a_collision() { + let errors = single_file_errors( + "external fn scale(a: i32) -> i32;\nuse { scale } from libA;\nspec S { fn scale(x: i32) \ + -> i32 { return x; } }\npub fn run() -> i32 { return scale(1); }", + ); + assert!( + errors + .iter() + .any(|e| matches!(e, TypeCheckError::SpecFunctionShadowsTopLevel { .. })), + "the spec/top-level shadowing rule owns this program: {errors:?}" + ); + assert!( + !errors + .iter() + .any(|e| matches!(e, TypeCheckError::ExternFunctionNameCollision { .. })), + "a `spec`-inner function is not a top-level one: {errors:?}" + ); +} + +/// Several collisions report in the order the user reads them — by file in +/// arena order, then by declaration within a file. Reporting them from a +/// name-keyed map would order them by hash, which is a list no reader can follow +/// back into the source. +#[test] +fn collisions_are_reported_in_file_then_source_order() { + assert_eq!( + collisions(&[ + ( + vec![], + "use sib;\nfn alpha(x: i32) -> i32 { return x; }\nfn beta(x: i32) -> i32 { \ + return x; }\npub fn run(x: i32) -> i32 { return alpha(x) + beta(x) + \ + sib::go(x); }", + ), + ( + vec!["sib"], + "external fn alpha(a: i32) -> i32;\nexternal fn beta(a: i32) -> i32;\npub fn \ + go(x: i32) -> i32 { return x; }", + ), + ]), + vec![ + (Some("sib".to_string()), 1, 2, None), + (Some("sib".to_string()), 2, 3, None), + ], + "the sibling's two externs report in the order they are declared" + ); +} diff --git a/tests/src/type_checker/mod.rs b/tests/src/type_checker/mod.rs index bd229d78..37a7ed3c 100644 --- a/tests/src/type_checker/mod.rs +++ b/tests/src/type_checker/mod.rs @@ -6,6 +6,9 @@ mod associated_functions; mod coverage; mod duplicate_parameter_name; mod error_recovery; +mod extern_binding; +mod extern_index; +mod extern_name_collision; mod features; mod literal_typing; mod multi_file; diff --git a/tests/src/type_checker/type_checker.rs b/tests/src/type_checker/type_checker.rs index edc16fe8..cab2fd62 100644 --- a/tests/src/type_checker/type_checker.rs +++ b/tests/src/type_checker/type_checker.rs @@ -3315,13 +3315,18 @@ mod external_function_tests { /// Phase 1 of issue #9: extern provenance binding. /// /// An `external fn` is bound to the source module named by a `use … from` -/// clause. The binding is exposed on [`TypedContext`] via `extern_origin` and -/// `is_extern_function`. A name imported from two distinct modules is an -/// ambiguity error; a `use … from` naming an undeclared extern is a dangling -/// import error; a bare extern (no binding `use`) stays valid but unbound. +/// clause. The binding is read back by *declaration*: `ExternIndex` resolves a +/// name to the declaration it denotes in a given scope, and +/// `TypedContext::extern_origin_by_decl` gives that declaration's provenance. A +/// name imported from two distinct modules is an ambiguity error; a `use … from` +/// naming an undeclared extern is a dangling import error; a bare extern (no +/// binding `use`) stays valid but unbound. #[cfg(test)] mod extern_provenance_tests { use super::*; + use inference_ast::ids::DefId; + use inference_type_checker::ExternOrigin; + use inference_type_checker::typed_context::TypedContext; fn err_string(source: &str) -> String { match try_type_check(source) { @@ -3330,6 +3335,19 @@ mod extern_provenance_tests { } } + /// The top-level `external fn name` this fixture declares, or `None` when it + /// declares none. Every fixture here is a single file, whose module path is + /// empty. + fn declaration(ctx: &TypedContext, name: &str) -> Option { + ctx.extern_index().lookup_top_level(&[], name) + } + + /// The provenance of the top-level `external fn name`, or `None` when the + /// fixture declares no such extern or declares one no `use … from` binds. + fn origin_of(ctx: &TypedContext, name: &str) -> Option { + declaration(ctx, name).and_then(|decl| ctx.extern_origin_by_decl(decl)) + } + // Binding succeeds --- #[test] @@ -3340,16 +3358,14 @@ mod extern_provenance_tests { fn main() -> i32 { return 0; } "#; let ctx = try_type_check(source).expect("binding a single module should type-check"); - let origin = ctx - .extern_origin("sort") - .expect("sort should carry a bound origin"); + let origin = origin_of(&ctx, "sort").expect("sort should carry a bound origin"); assert_eq!(origin.logical_module, "collections"); assert_eq!(origin.export_field, "sort"); assert!( origin.resolved_path.is_none(), "Phase 1 leaves resolved_path unset; the driver fills it" ); - assert!(ctx.is_extern_function("sort")); + assert!(declaration(&ctx, "sort").is_some()); } #[test] @@ -3360,7 +3376,7 @@ mod extern_provenance_tests { fn main() -> i32 { return 0; } "#; let ctx = try_type_check(source).expect("nested module path should type-check"); - let origin = ctx.extern_origin("hash").expect("hash should be bound"); + let origin = origin_of(&ctx, "hash").expect("hash should be bound"); assert_eq!( origin.logical_module, "crypto::sha256", "nested path joins with `::`, never an OS separator" @@ -3377,11 +3393,11 @@ mod extern_provenance_tests { "#; let ctx = try_type_check(source).expect("multi-field use should type-check"); assert_eq!( - ctx.extern_origin("sort").expect("sort bound").logical_module, + origin_of(&ctx, "sort").expect("sort bound").logical_module, "collections" ); assert_eq!( - ctx.extern_origin("search") + origin_of(&ctx, "search") .expect("search bound") .logical_module, "collections" @@ -3400,7 +3416,7 @@ mod extern_provenance_tests { "#; let ctx = try_type_check(source).expect("repeated identical import should bind"); assert_eq!( - ctx.extern_origin("sort").expect("sort bound").logical_module, + origin_of(&ctx, "sort").expect("sort bound").logical_module, "collections" ); } @@ -3415,11 +3431,11 @@ mod extern_provenance_tests { "#; let ctx = try_type_check(source).expect("a bare extern declaration is valid"); assert!( - ctx.extern_origin("add").is_none(), + origin_of(&ctx, "add").is_none(), "an extern with no binding `use` has no provenance" ); assert!( - ctx.is_extern_function("add"), + declaration(&ctx, "add").is_some(), "an unbound extern is still discriminated as extern, not local" ); } @@ -3428,8 +3444,8 @@ mod extern_provenance_tests { fn local_function_is_not_extern() { let source = r#"fn helper() -> i32 { return 1; } fn main() -> i32 { return helper(); }"#; let ctx = try_type_check(source).expect("local functions type-check"); - assert!(!ctx.is_extern_function("helper")); - assert!(ctx.extern_origin("helper").is_none()); + assert!(declaration(&ctx, "helper").is_none()); + assert!(origin_of(&ctx, "helper").is_none()); } // Ambiguity errors --- @@ -3505,7 +3521,7 @@ mod extern_provenance_tests { #[test] fn top_level_use_does_not_bind_a_spec_inner_extern() { - // H8: a `use … from` clause is file-global but binds only TOP-LEVEL + // H8: a `use … from` clause is file-wide but binds only TOP-LEVEL // externs. A spec-inner `external fn mix` is a different scope; naming it // from a top-level `use` with no matching top-level extern is a dangling // import (`ExternImportNotDeclared`), not a silent bind. The prior @@ -3541,9 +3557,19 @@ mod extern_provenance_tests { "#; let ctx = try_type_check(source).expect("top-level mix binds; spec mix stays unbound"); assert_eq!( - ctx.extern_origin("mix").expect("top-level mix is bound").logical_module, + origin_of(&ctx, "mix") + .expect("top-level mix is bound") + .logical_module, "crypto" ); + let spec_mix = ctx + .extern_index() + .lookup(&[], Some("s"), "mix") + .expect("the spec declares its own `mix`"); + assert!( + ctx.extern_origin_by_decl(spec_mix).is_none(), + "the spec-inner declaration is a different extern and stays unbound" + ); } #[test] diff --git a/tests/src/utils.rs b/tests/src/utils.rs index 907ec26f..b2ea7ca9 100644 --- a/tests/src/utils.rs +++ b/tests/src/utils.rs @@ -52,7 +52,24 @@ pub(crate) fn try_build_ast(source_code: String) -> anyhow::Result { pub(crate) fn try_type_check_multi_file( files: &[(Vec<&str>, &str)], ) -> anyhow::Result { - let mut arena = inference_ast::arena::AstArena::default(); + Ok( + inference_type_checker::TypeCheckerBuilder::build_typed_context(build_multi_file_ast( + files, + ))? + .typed_context(), + ) +} + +/// Folds `(module_path, source)` pairs into one arena, the shape a multi-file +/// program reaches the type checker in. The multi-file analogue of +/// [`build_ast`], for tests that need the structured diagnostics +/// [`inference_type_checker::check_with_diagnostics`] returns rather than the +/// aggregated message [`try_type_check_multi_file`] renders. +/// +/// # Panics +/// Panics if any file has a syntax error (tests should pass valid sources). +pub(crate) fn build_multi_file_ast(files: &[(Vec<&str>, &str)]) -> AstArena { + let mut arena = AstArena::default(); for (module_path, source) in files { let module_path: Vec = module_path.iter().map(|s| (*s).to_string()).collect(); let parsed = inference_parser::parse_into(arena, source, module_path); @@ -63,10 +80,7 @@ pub(crate) fn try_type_check_multi_file( ); arena = parsed.arena; } - Ok( - inference_type_checker::TypeCheckerBuilder::build_typed_context(arena)? - .typed_context(), - ) + arena } /// Controls whether the analysis pass runs during codegen. @@ -539,6 +553,34 @@ pub(crate) fn proof_wasm_codegen_project(module_path: &str, test_name: &str) -> .to_vec() } +/// Type-checks, analyzes and generates a multi-file program from in-memory +/// sources in Proof mode, returning the merged WASM bytes. +/// +/// The proof-mode analogue of [`wasm_codegen_multi_file`], for the constructs +/// that exist only inside a `spec` — compile mode emits no specification +/// function at all, so a seam reached only from a spec body is unreachable +/// through the compile-mode helper. +/// +/// # Panics +/// Panics if any file has a syntax error, or if type checking, analysis or +/// code generation fails. +pub(crate) fn proof_wasm_codegen_multi_file(files: &[(Vec<&str>, &str)]) -> Vec { + let typed_context = try_type_check_multi_file(files) + .expect("multi-file proof-mode test source should type-check"); + inference_analysis::analyze(&typed_context).expect("multi-file proof analysis should succeed"); + inference_wasm_codegen::codegen( + &typed_context, + "output", + inference_wasm_codegen::CodegenOptions { + mode: inference_wasm_codegen::CompilationMode::Proof, + ..Default::default() + }, + ) + .expect("multi-file proof codegen should succeed") + .wasm() + .to_vec() +} + /// Like [`wasm_codegen_project`] but returns the error instead of panicking, for /// negative tests that assert a multi-file program is rejected. pub(crate) fn try_codegen_project(