From b714cddcb2c439680a054d5fe634545b73012b8f Mon Sep 17 00:00:00 2001 From: simontreanor Date: Sat, 29 Aug 2026 10:17:40 +0100 Subject: [PATCH 1/3] types: a module's interface carries the records and opaque types it references ModuleExports held only what a module declares, so anything its exports referred to fell off at the next boundary: an extern type could not be named across a module at all (#72, there was no slot for it), and a record reached only through another module's exports had invisible fields (#75, the field registry crossed one hop while the type crossed two). Both get the same treatment the scheme side got in #26: the interface now closes over what it references. extern type declarations export as name + arity and register in a consumer like any imported type, clash check included. Records and opaque types mentioned by exported schemes, constructors, or record fields are pulled from the imports' interfaces, tagged with their declaring module, and re-exported; the walk follows newly pulled records and terminates on cycles. A carried type registers under its bare identity name only, so it can be named, unified, and field-accessed, while construction and patterns still require importing the declaring module. A taken bare name skips the carried entry silently (local and direct declarations win; the same record along two paths is admitted once), and a shadowed record's fields feed the unknown-field diagnostic, which now names the record and the module that declares it. Record updates in a consumer that never imports the declaring module reconstruct via that module's class, with its import hoisted. Closes #75 Closes #72 --- DESIGN.md | 22 ++- INTERNALS.md | 21 +- src/lowering/mod.rs | 20 +- src/project/mod.rs | 26 ++- src/types/mod.rs | 471 +++++++++++++++++++++++++++++++++++--------- tests/project.rs | 392 ++++++++++++++++++++++++++++++++++++ 6 files changed, 853 insertions(+), 99 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 0c0f8ed..9d4a815 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -893,7 +893,8 @@ checker already uses for built-in/in-file modules, so the existing `Field`-node cross-module reference with no new lookup logic. A module's interface is its top-level **`let` values** plus its **sum types** (since the cross-module-ADT follow-on; `ModuleExports` carries each public sum type's name, arity, and constructors) **and its records** (since the cross-module-record follow-on; -`ModuleExports` also carries each public record's name + fields). A consumer can construct (`Geometry.Circle +`ModuleExports` also carries each public record's name + fields) **and its opaque handle types** (name + +arity, so `extern type Rng` can be named across the boundary). A consumer can construct (`Geometry.Circle 2.0`) and pattern-match (`| Geometry.Circle r ->`, a qualified constructor pattern) the imported type's values, with **exhaustiveness checked across the boundary** (a missing arm reports the qualified witness `Geometry.Rect _ _`). **Records cross too** (`DESIGN.md` §8.3): construct `Geometry.Point { x = 1, y = 2 }`, @@ -904,10 +905,21 @@ an ADT variant payload or an `extern` signature. Both spellings are accepted, ba `Shapes.Placed`, and they denote the same type: an imported type registers under its bare identity name (unique across everything visible, as the import clash check enforces) plus a qualified key, and a written qualifier is validated and then folded back to that identity, so the two unify freely. Prefer the qualified -spelling where the reader benefits from knowing which module a type came from. The one thing this does not -extend to is *transitive* naming: a third module importing `Holder` without importing `Shapes` can hold and -pass its `item` around, but cannot access that value's fields or construct one, since only `Shapes` brings -`Placed`'s field registry into scope. **Externs and +spelling where the reader benefits from knowing which module a type came from. **Opaque handle types +(`extern type Rng`, §6) cross like any other type name:** the interface carries the name + arity, so a +consumer can write `Gen.Rng` (or bare `Rng`) in a record field, an ADT payload, or an `extern` signature; +values of the type always crossed through the exported schemes. **Interfaces close transitively over what +they reference:** a record or opaque type that an exported scheme, constructor, or record field mentions is +carried in the exporter's interface, tagged with the module that declares it, and the pull repeats through +each hop (a worklist walk with a seen set, so mutually referencing records terminate). A third module +importing `Holder` without importing `Shapes` can therefore hold its `item`, read that value's fields, and +name its type bare (`Placed`). A carried type crosses as an *identity*, not as a member: constructing it, +pattern-matching it, or writing the qualified spelling still requires importing the declaring module +directly. A carried name already taken here (by a local type or a direct import) is skipped silently rather +than reported, since the consumer never wrote that name: local and direct declarations win the bare name, +the same record arriving along two import paths is recognized by its declaring module and admitted once, +and a genuinely different type shadows the carried record, whose fields then feed the "unknown record +field" diagnostic (the message names the hidden record and the module that declares it). **Externs and measures cross too:** an imported `extern` (`Mathx.cbrt`) is exported like a value (its scheme joins the interface) and — in the project lowering path — also **bound at top level in its own module** (`cbrt = math.cbrt`, `import math` hoisted) so a dependent module references it as `mathx.cbrt`; single-file lowering diff --git a/INTERNALS.md b/INTERNALS.md index 5743f3a..6a6d5c8 100644 --- a/INTERNALS.md +++ b/INTERNALS.md @@ -430,8 +430,9 @@ returned topological order (dependencies first, entry last). `project::build_fro in the entry's directory). Cross-module *checking* and *emit* consume this `Project`. **Cross-module checking** (`types::check_module` + `project::check`). *Implementation:* the single-file `run` was generalized to take the imports map and return the module's -exported value schemes (which now include `extern` names), its exported sum types, **its exported records**, -**and its measures + measure-aliases**; `check_module(module, +`ModuleExports` interface: its exported value schemes (which include `extern` names), its exported sum +types, **its exported records**, **its opaque handle types** (`collect_exported_opaques` / +`ExportedOpaque`: name + arity, issue #72), **and its measures + measure-aliases**; `check_module(module, imports)` seeds imported values under qualified keys and imported sum types into the decls under **qualified constructor keys**. The type merge happens in **two halves**, because a local `type` declaration may name an imported type (`type Holder = { item: Shapes.Placed }`, DESIGN §6.1): `merge_imported_type_names` runs @@ -456,6 +457,22 @@ and leaked local variables into the "free in the env" set — blocking generaliz bindings, which then exported un-quantified and cascaded onward (#26). Every imported scheme is therefore alpha-renamed into the consumer's id space as it is seeded (`Infer::refresh_scheme`, values and constructors alike, in sorted order so the ids do not depend on `HashMap` iteration). +*Transitive interface closure* (issues #72/#75): before `run` hands back its `ModuleExports`, +`close_over_references` walks every exported scheme, constructor scheme, and record field for `Ty::Con` +names (`collect_con_names`) and pulls any name satisfied by an import's record/opaque tables into this +module's own exports, tagged `origin: Some(declaring module)` (own declarations carry `origin: None`); +newly pulled records are walked in turn, and a seen set terminates the walk on mutually referencing +records. Each import's interface is itself closed, so one pull per hop reaches any depth. On the consumer +side `merge_imported_type_names` registers carried entries **last** and under the **bare identity name +only** (no qualified key, no `record_aliases` entry, so constructing or pattern-matching a carried record +still requires the direct import); a taken bare name skips the carried entry silently (`record_home` +recognizes the same record arriving along two paths), and a genuinely shadowed record's fields land in +`Decls::field_hints`, which `record_of_field` / `record_of_field_on` render as "the record `Config` in +module `Inner` declares this field" instead of a bare unknown. The lowering side: +`ModuleExports::carried_records` feeds `project::compile`'s per-module `ImportContext::record_class_modules` +plus field data keyed by the *declaring* module's tag, so a record **update** in a consumer that never +imports the declaring module still reconstructs via the right class (`inner.Config(...)`, +`Lowerer::record_class_name` accepts those modules and hoists their imports). `project::check` threads the `ModuleExports` map through the topological order, seeding each module from only the modules it actually imports (so an unimported module's members/constructors stay invisible), and returns errors grouped by module. *Lowering* routes a qualified constructor — in expression or pattern diff --git a/src/lowering/mod.rs b/src/lowering/mod.rs index 315b6d0..2a2eac4 100644 --- a/src/lowering/mod.rs +++ b/src/lowering/mod.rs @@ -122,6 +122,12 @@ pub struct ImportContext { /// (`x` → `Geometry.Point`), so a cross-module update `{ p with x = 3 }` /// (which carries no tag) routes to the imported class. pub record_field_owners: HashMap, + /// Modules that declare a **transitively** visible record (`DESIGN.md` §6.1) + /// without being imported by this module directly. A record update on such a + /// record reconstructs via the declaring module's class (`inner.Config(...)`), + /// so its module must be recognized as a class source (and its Python import + /// hoisted) even though no `import` names it here. + pub record_class_modules: HashSet, } /// A module lowered as part of a multi-file project. @@ -149,6 +155,7 @@ pub fn lower_in_project( let mut lowerer = Lowerer::new(module); lowerer.float_literals = float_literals.clone(); lowerer.imported_modules = ctx.modules.clone(); + lowerer.record_class_modules = ctx.record_class_modules.clone(); lowerer.imported_nullary_ctors = ctx.nullary_ctors.clone(); lowerer .newtype_ctors @@ -318,6 +325,11 @@ struct Lowerer { /// A `Geometry.member` reference routes to Python `geometry.member` (vs the /// `Geometry_member` mangling used for in-file `module` declarations). imported_modules: HashSet, + /// Modules that declare a transitively visible record without being imported + /// here directly ([`ImportContext::record_class_modules`]). Consulted only by + /// [`Lowerer::record_class_name`], so nothing but a record reconstruction can + /// route through such a module. + record_class_modules: HashSet, /// Qualified names of imported nullary constructors (`Palette.Red`), referenced /// as values, which must lower to a call (`palette.Red()`) not the bare class. imported_nullary_ctors: HashSet, @@ -610,6 +622,7 @@ impl Lowerer { float_literals: HashSet::new(), cur_module: None, imported_modules: HashSet::new(), + record_class_modules: HashSet::new(), imported_nullary_ctors: HashSet::new(), use_runtime: false, project_mode: false, @@ -2850,10 +2863,13 @@ impl Lowerer { /// file module (`Geometry.Point`) becomes dotted attribute access on that module /// (`geometry.Point`, with `import geometry` hoisted) so it references the *same* /// class the module defines (the consumer never redefines it); a bare tag is the - /// record class name (mangled for the reserved `Exception`). + /// record class name (mangled for the reserved `Exception`). A tag rooted in a + /// module visible only through a transitively carried record + /// (`record_class_modules`) routes the same way — an update on such a record + /// reconstructs via the declaring module's class. fn record_class_name(&mut self, tag: &str) -> String { if let Some((base, rec)) = tag.split_once('.') - && self.imported_modules.contains(base) + && (self.imported_modules.contains(base) || self.record_class_modules.contains(base)) { let module = self.py_module_ref(base); format!("{module}.{}", py_record_class(rec)) diff --git a/src/project/mod.rs b/src/project/mod.rs index aa2c920..a988057 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -242,9 +242,11 @@ pub fn compile_targeting( // Per-module spans of integer literals that inference resolved to `float`, so // lowering emits them as `7.0`. Computed in topological order (like `check`) so // each module's imports are seeded before it — a literal's float-ness can depend - // on a cross-module call. Exports are threaded forward in the same pass. + // on a cross-module call. Exports are threaded forward in the same pass and kept: + // an import's interface also carries the records it references transitively + // (`DESIGN.md` §6.1), which the lowering context below needs. + let mut exports: HashMap = HashMap::new(); let float_spans: HashMap> = { - let mut exports: HashMap = HashMap::new(); let mut spans = HashMap::new(); for module in &project.modules { let imports: HashMap = module @@ -294,6 +296,26 @@ pub fn compile_targeting( } } } + // Records an import's interface carries transitively (declared in a + // module this one does not import directly): keyed by the *declaring* + // module's tag, so an update on such a record reconstructs via the right + // class (`inner.Config(...)`, with `import inner` hoisted). Registered + // after the direct entries, which keep precedence for shared field names. + for import in &module.imports { + let Some(exp) = exports.get(import) else { + continue; + }; + for (origin, rec, fields) in exp.carried_records() { + let tag = format!("{origin}.{rec}"); + for field in &fields { + ctx.record_field_owners + .entry(field.clone()) + .or_insert_with(|| tag.clone()); + } + ctx.record_fields.entry(tag).or_insert(fields); + ctx.record_class_modules.insert(origin); + } + } let floats = float_spans.get(&module.name).unwrap_or(&no_floats); let lowered = lowering::lower_in_project(&module.ast, &ctx, floats)?; needs_runtime |= lowered.uses_runtime; diff --git a/src/types/mod.rs b/src/types/mod.rs index c9d4705..9eef480 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -1805,6 +1805,13 @@ struct Decls { /// name (`Point`), so a qualified construction/pattern resolves to the same /// `RecordInfo` (and `Ty::Con`) the exporting module uses. record_aliases: HashMap, + /// Field name → `(record, declaring module)` for each record an import's + /// interface carries **transitively** whose bare name is already taken here + /// (by a local type or another import), so the record itself stays out of + /// scope. Consulted only to improve the "unknown record field" / + /// "has no field" diagnostics: the field's owner is knowable even though the + /// record is not visible. + field_hints: HashMap>, /// User-declared in-file module names (`module Foo = …`), for the "X is a /// module" diagnostic on a bare reference. modules: HashSet, @@ -1817,8 +1824,7 @@ struct Decls { /// Type-check a whole module, returning every independent error found. pub fn check(module: &Module) -> Result<(), Vec> { - let (errors, _types, _schemes, _exports, _records, _measures, _holes, _ordered) = - run(module, false, &HashMap::new()); + let (errors, _types, _exports, _holes, _ordered) = run(module, false, &HashMap::new()); if errors.is_empty() { Ok(()) } else { @@ -1829,7 +1835,8 @@ pub fn check(module: &Module) -> Result<(), Vec> { /// One sum type a module exports (`DESIGN.md` §6.1): its name, type-parameter /// arity, and constructors (bare name + signature/arity). Carried in /// [`ModuleExports`] so an importing module can construct and pattern-match the -/// type's values. Records / measures / externs are not yet cross-module exported. +/// type's values. Records, measures, externs, and opaque handle types cross the +/// boundary through the other [`ModuleExports`] slots. #[derive(Clone)] struct ExportedType { name: String, @@ -1837,18 +1844,55 @@ struct ExportedType { ctors: Vec<(String, CtorInfo)>, } +/// One record a module's interface carries (`DESIGN.md` §8.3): its bare identity +/// name and its [`RecordInfo`] (params + fields). +#[derive(Clone)] +struct ExportedRecord { + name: String, + info: RecordInfo, + /// The module that **declares** the record: `None` when this module declares + /// it itself, `Some(module)` when the record is carried transitively because + /// an exported scheme or record field references it (`DESIGN.md` §6.1). + origin: Option, +} + +/// One opaque handle type (`extern type Rng`) a module's interface carries: its +/// name and type-parameter arity. There is nothing more to a handle type — no +/// constructors, no fields — so exporting the name is exporting the type. +#[derive(Clone)] +struct ExportedOpaque { + name: String, + arity: usize, + /// As on [`ExportedRecord`]: `None` for this module's own declaration, + /// `Some(module)` for one carried transitively from that module. + origin: Option, +} + /// A module's exported interface (`DESIGN.md` §6.1): its public top-level `let` /// values' schemes (keyed by **bare** name) and its public sum types. Opaque — /// the scheme/ctor representation is internal — produced by [`check_module`] and /// fed back in as a dependent module's imports. +/// +/// The record and opaque-type slots are **transitively closed**: they hold this +/// module's own declarations plus every record / opaque type its exported +/// schemes, constructors, and record fields reference out of its own imports' +/// interfaces (each tagged with the module that declares it). `Wrap` cannot +/// honestly be exported without the `Config` its field is typed with, or a +/// consumer two hops from `Config`'s module could hold a value whose fields it +/// cannot access and whose type it cannot name. #[derive(Clone, Default)] pub struct ModuleExports { schemes: Env, types: Vec, - /// Public **record** types (`DESIGN.md` §8.3): each bare name + its - /// [`RecordInfo`] (params + fields), so an importing module can construct, - /// pattern-match, update, and field-access the record via qualified tags. - records: Vec<(String, RecordInfo)>, + /// **Record** types (`DESIGN.md` §8.3) — own + transitively referenced — so + /// an importing module can construct, pattern-match, update, and + /// field-access a record via qualified tags (own records), and field-access + /// / name transitively carried ones. + records: Vec, + /// **Opaque handle types** (`extern type Rng`) — own + transitively + /// referenced — so an importing module can write the type's name in a + /// record field, an `extern` signature, or an ADT payload. + opaques: Vec, /// Public **base measure** names (`measure m`). Merged **unqualified** into a /// consumer's decls — there is no qualified unit syntax (`` is bare), so /// measures cross by name and erase at lowering (`DESIGN.md` §6.1). @@ -1858,6 +1902,27 @@ pub struct ModuleExports { measure_aliases: HashMap, } +impl ModuleExports { + /// The records this interface carries from **other** modules' declarations + /// (the transitively pulled entries): `(declaring module, record name, + /// declared field order)`. The project lowerer uses this to route a record + /// update in a consumer that never imports the declaring module directly — + /// the update must reconstruct via the declaring module's class. + pub(crate) fn carried_records(&self) -> Vec<(String, String, Vec)> { + self.records + .iter() + .filter_map(|rec| { + let origin = rec.origin.clone()?; + Some(( + origin, + rec.name.clone(), + rec.info.fields.iter().map(|(f, _)| f.clone()).collect(), + )) + }) + .collect() + } +} + /// Type-check `module` as one node of a multi-file project (`DESIGN.md` §6.1). /// /// `imports` maps each imported module's **name** to its [`ModuleExports`]; its @@ -1872,18 +1937,8 @@ pub fn check_module( module: &Module, imports: &HashMap, ) -> (Vec, ModuleExports) { - let (errors, _types, schemes, types, records, (measures, measure_aliases), _holes, _ordered) = - run(module, false, imports); - ( - errors, - ModuleExports { - schemes, - types, - records, - measures, - measure_aliases, - }, - ) + let (errors, _types, exports, _holes, _ordered) = run(module, false, imports); + (errors, exports) } /// Like [`check_module`] but also returns the span→type table (as @@ -1894,19 +1949,8 @@ pub fn check_module_collecting( module: &Module, imports: &HashMap, ) -> (Vec, Vec, ModuleExports) { - let (errors, types, schemes, tys, records, (measures, measure_aliases), _holes, _ordered) = - run(module, true, imports); - ( - errors, - types, - ModuleExports { - schemes, - types: tys, - records, - measures, - measure_aliases, - }, - ) + let (errors, types, exports, _holes, _ordered) = run(module, true, imports); + (errors, types, exports) } /// Like [`check_collecting`] but with imported modules' exports seeded @@ -1916,8 +1960,7 @@ pub fn check_collecting_with_imports( module: &Module, imports: &HashMap, ) -> (Vec, Vec, Vec) { - let (errors, types, _schemes, _exports, _records, _measures, holes, _ordered) = - run(module, true, imports); + let (errors, types, _exports, holes, _ordered) = run(module, true, imports); (errors, types, holes) } @@ -1929,8 +1972,7 @@ pub fn check_collecting_with_imports( pub fn check_collecting( module: &Module, ) -> (Vec, Vec, Vec, HashSet) { - let (errors, types, _schemes, _exports, _records, _measures, holes, ordered) = - run(module, true, &HashMap::new()); + let (errors, types, _exports, holes, ordered) = run(module, true, &HashMap::new()); (errors, types, holes, ordered) } @@ -1939,40 +1981,35 @@ pub fn check_collecting( /// node, which we resolve and render once inference is complete. `imports` pre-binds /// imported modules' interfaces (members under qualified keys, sum types under /// qualified constructor keys, for the multi-file driver); it is empty for a -/// single-file check. Returns the errors, the hover table, the module's exported -/// value schemes (top-level `let` bindings under their bare names), and its -/// exported sum types, and its exported records. +/// single-file check. Returns the errors, the hover table, and the module's +/// [`ModuleExports`] interface (value schemes, sum types, records, opaque handle +/// types, measures — transitively closed over what the interface references). type RunResult = ( Vec, Vec, - Env, - Vec, - Vec<(String, RecordInfo)>, - ExportedMeasures, + ModuleExports, Vec, // User type names the program compares (need ordering methods emitted). HashSet, ); -/// This module's own measures, for its export interface: base names and derived -/// aliases (`DESIGN.md` §6.1). -type ExportedMeasures = (HashSet, HashMap); - fn run(module: &Module, record: bool, imports: &HashMap) -> RunResult { let mut errors = Vec::new(); // `build_decls` registers imported type *names* mid-way through, so a local // `type` declaration can be written in terms of an imported type; it hands back // the names it accepted for the body merge below. let (mut decls, ctor_env, imported) = build_decls(module, imports, &mut errors); - // This module's own public sum types + records: `collect_exported_*` walk this - // module's own items, so only what it declares is exported. + // This module's own public sum types + records + opaque handle types: + // `collect_exported_*` walk this module's own items, so only what it declares + // starts here (the transitive closure below may add carried records/opaques). let exported_types = collect_exported_types(module, &decls); let exported_records = collect_exported_records(module, &decls); + let exported_opaques = collect_exported_opaques(module, &decls); // Captured before imports are merged, so we export only this module's own // measures. `decls.measures` holds base + alias names; `decls.measure_aliases` // holds the aliases' expansions. - let exported_measures: ExportedMeasures = - (decls.measures.clone(), decls.measure_aliases.clone()); + let exported_measures = decls.measures.clone(); + let exported_measure_aliases = decls.measure_aliases.clone(); // Merge imported modules' type *bodies* into the decls (qualified constructor // keys), so `Geometry.Circle` construction, qualified ctor patterns, and // exhaustiveness all resolve against the imported type. This half runs after @@ -2245,21 +2282,133 @@ fn run(module: &Module, record: bool, imports: &HashMap) .collect(); let ordered = std::mem::take(&mut inf.ordered); + let (exported_records, exported_opaques) = close_over_references( + &exports, + &exported_types, + exported_records, + exported_opaques, + imports, + ); ( errors, types, - exports, - exported_types, - exported_records, - exported_measures, + ModuleExports { + schemes: exports, + types: exported_types, + records: exported_records, + opaques: exported_opaques, + measures: exported_measures, + measure_aliases: exported_measure_aliases, + }, holes, ordered, ) } +/// Close a module's export interface over the records and opaque handle types it +/// references (`DESIGN.md` §6.1, issues #72/#75): any `Ty::Con` name mentioned by +/// an exported scheme, an exported constructor's scheme, or an exported record's +/// field type that is satisfied by an import's record/opaque tables joins this +/// module's own exports, tagged with the module that declares it. Newly pulled +/// records are walked in turn, so a chain of any depth crosses; the `seen` set +/// makes the walk terminate on cycles (mutually-referencing records are legal). +/// Each import's interface is itself closed, so one level of pulling per hop +/// reaches everything. Names not found in any import (builtins, type parameters, +/// imported sum types) are simply skipped. +fn close_over_references( + schemes: &Env, + types: &[ExportedType], + mut records: Vec, + mut opaques: Vec, + imports: &HashMap, +) -> (Vec, Vec) { + if imports.is_empty() { + return (records, opaques); + } + // Names this module's interface already provides need no pull. + let mut seen: HashSet = HashSet::new(); + seen.extend(types.iter().map(|t| t.name.clone())); + seen.extend(records.iter().map(|r| r.name.clone())); + seen.extend(opaques.iter().map(|o| o.name.clone())); + + let mut work: Vec = Vec::new(); + for scheme in schemes.values() { + collect_con_names(&scheme.ty, &mut work); + } + for ty in types { + for (_, info) in &ty.ctors { + collect_con_names(&info.scheme.ty, &mut work); + } + } + for rec in &records { + for (_, field_ty) in &rec.info.fields { + collect_con_names(field_ty, &mut work); + } + } + + // Sorted so the pull is deterministic when two imports carry the same name + // (they then carry the same record, so the choice only fixes iteration order). + let mut import_names: Vec<&String> = imports.keys().collect(); + import_names.sort(); + while let Some(name) = work.pop() { + if !seen.insert(name.clone()) { + continue; + } + for module_name in &import_names { + let exp = &imports[*module_name]; + if let Some(rec) = exp.records.iter().find(|r| r.name == name) { + let origin = rec.origin.clone().unwrap_or_else(|| (*module_name).clone()); + for (_, field_ty) in &rec.info.fields { + collect_con_names(field_ty, &mut work); + } + records.push(ExportedRecord { + name: name.clone(), + info: rec.info.clone(), + origin: Some(origin), + }); + break; + } + if let Some(op) = exp.opaques.iter().find(|o| o.name == name) { + let origin = op.origin.clone().unwrap_or_else(|| (*module_name).clone()); + opaques.push(ExportedOpaque { + name: name.clone(), + arity: op.arity, + origin: Some(origin), + }); + break; + } + } + } + (records, opaques) +} + +/// Collect the name of every applied type constructor in `ty`, at any depth +/// (through arrows, tuples, and constructor arguments). +fn collect_con_names(ty: &Ty, out: &mut Vec) { + match ty { + Ty::Con(name, args) => { + out.push(name.clone()); + for a in args { + collect_con_names(a, out); + } + } + Ty::Fun(a, b, _) => { + collect_con_names(a, out); + collect_con_names(b, out); + } + Ty::Tuple(elems) => { + for e in elems { + collect_con_names(e, out); + } + } + Ty::Int(_) | Ty::Float(_) | Ty::Bool | Ty::Str | Ty::Unit | Ty::Num(..) | Ty::Var(_) => {} + } +} + /// Capture a module's own public **sum** types from the freshly-built decls, for -/// its export interface (`DESIGN.md` §6.1). Records / measures / externs are not -/// yet cross-module exported. +/// its export interface (`DESIGN.md` §6.1). Records, opaque handle types, and +/// measures are captured by their own collectors below; externs export like +/// values (their schemes join the env). fn collect_exported_types(module: &Module, decls: &Decls) -> Vec { let mut out = Vec::new(); for item in &module.items { @@ -2293,7 +2442,7 @@ fn collect_exported_types(module: &Module, decls: &Decls) -> Vec { /// (`DESIGN.md` §8.3), for its export interface: each bare name + its [`RecordInfo`] /// (params + fields). The reserved `Exception` record is seeded, not user-declared, /// so it is never exported. -fn collect_exported_records(module: &Module, decls: &Decls) -> Vec<(String, RecordInfo)> { +fn collect_exported_records(module: &Module, decls: &Decls) -> Vec { let mut out = Vec::new(); for item in &module.items { let Item::Type(decl) = item else { continue }; @@ -2301,20 +2450,50 @@ fn collect_exported_records(module: &Module, decls: &Decls) -> Vec<(String, Reco continue; } if let Some(info) = decls.records.get(&decl.name) { - out.push((decl.name.clone(), info.clone())); + out.push(ExportedRecord { + name: decl.name.clone(), + info: info.clone(), + origin: None, + }); + } + } + out +} + +/// Capture a module's own **opaque handle types** (`extern type Rng`) from the +/// freshly-built decls, for its export interface (`DESIGN.md` §6.1 / issue #72): +/// each name + arity, so an importing module can write `Gen.Rng` (or bare `Rng`) +/// in a type position. Values of the type already cross through the exported +/// schemes; this makes the *name* cross with them. +fn collect_exported_opaques(module: &Module, decls: &Decls) -> Vec { + let mut out = Vec::new(); + for item in &module.items { + let Item::Type(decl) = item else { continue }; + if !matches!(decl.kind, TypeDeclKind::Opaque) { + continue; + } + if let Some(&arity) = decls.type_arity.get(&decl.name) { + out.push(ExportedOpaque { + name: decl.name.clone(), + arity, + origin: None, + }); } } out } -/// Register imported modules' sum-type and record **names** in `decls` -/// (`DESIGN.md` §6.1 + §8.3): each under its **bare identity name** (`Point`, so an -/// imported type is the same `Ty::Con` everywhere) *and* under a qualified key -/// (`Geometry.Point`) at the same arity, which [`resolve`] accepts in a type -/// position and folds back to the identity name. A name clashing with one already -/// present is reported (the bare-name uniqueness sum types already rely on) and -/// then skipped, so the returned set of accepted `(module, name)` pairs is what -/// [`merge_imported_types`] may go on to fill in. +/// Register imported modules' sum-type, record, and opaque-handle-type **names** +/// in `decls` (`DESIGN.md` §6.1 + §8.3): each under its **bare identity name** +/// (`Point`, so an imported type is the same `Ty::Con` everywhere) *and* under a +/// qualified key (`Geometry.Point`) at the same arity, which [`resolve`] accepts +/// in a type position and folds back to the identity name. A directly imported +/// name clashing with one already present is reported (the bare-name uniqueness +/// sum types already rely on) and then skipped, so the returned set of accepted +/// `(module, name)` pairs is what [`merge_imported_types`] may go on to fill in. +/// Records and opaque types an import carries **transitively** register last, +/// under the bare identity name only, and a taken name skips them silently — see +/// the pass comments below. /// /// This runs from [`build_decls`] **between** its two passes: after local type /// names are registered (so a local declaration still wins a clash) and before @@ -2351,8 +2530,18 @@ fn merge_imported_type_names( } } // Records after types (both share the `type_arity` namespace and clash check). + // Only an import's **own** records here (`origin: None`); the transitively + // carried ones follow below, after every directly named declaration, so a + // direct import always wins the bare name. `record_home` tracks each accepted + // record's declaring module so the same record arriving along two paths is + // recognized as one. + let mut record_home: HashMap = HashMap::new(); for module_name in &module_names { - for (name, info) in &imports[*module_name].records { + for rec in &imports[*module_name].records { + if rec.origin.is_some() { + continue; + } + let name = &rec.name; if decls.type_arity.contains_key(name) { errors.push(TypeError { message: format!( @@ -2362,11 +2551,78 @@ fn merge_imported_type_names( }); continue; } - decls.type_arity.insert(name.clone(), info.params_count); + decls.type_arity.insert(name.clone(), rec.info.params_count); decls .type_arity - .insert(format!("{module_name}.{name}"), info.params_count); + .insert(format!("{module_name}.{name}"), rec.info.params_count); accepted.insert(((*module_name).clone(), name.clone())); + record_home.insert(name.clone(), (*module_name).clone()); + } + } + // An import's own opaque handle types (`extern type Rng`): name + arity under + // the bare identity and a qualified key, exactly like a sum type — there is + // nothing more to a handle type, so this is the whole export (issue #72). + for module_name in &module_names { + for op in &imports[*module_name].opaques { + if op.origin.is_some() { + continue; + } + if decls.type_arity.contains_key(&op.name) { + errors.push(TypeError { + message: format!( + "imported type `{}` (from `{module_name}`) clashes with an existing type", + op.name + ), + span: Span::new(0, 0), + }); + continue; + } + decls.type_arity.insert(op.name.clone(), op.arity); + decls + .type_arity + .insert(format!("{module_name}.{}", op.name), op.arity); + } + } + // Transitively carried records (`origin: Some`, `DESIGN.md` §6.1): declared in + // a module this one does not import directly, referenced by an import's + // interface. They register under the **bare identity name only** — no + // qualified key, no construction alias — so the type can be named, unified, + // and field-accessed, while constructing or pattern-matching it still asks + // for a direct import of the declaring module. A bare name already taken is + // **not** an error (the consumer never asked for this name): the same record + // arriving along another path is already in, and a genuinely different type + // keeps the name — local and direct declarations win — while the hidden + // record's fields feed the unknown-field hints. + for module_name in &module_names { + for rec in &imports[*module_name].records { + let Some(origin) = &rec.origin else { continue }; + let name = &rec.name; + if decls.type_arity.contains_key(name) { + if record_home.get(name) == Some(origin) { + continue; // the same record, already visible along another path + } + for (field, _) in &rec.info.fields { + let hint = (name.clone(), origin.clone()); + let hints = decls.field_hints.entry(field.clone()).or_default(); + if !hints.contains(&hint) { + hints.push(hint); + } + } + continue; + } + decls.type_arity.insert(name.clone(), rec.info.params_count); + accepted.insert(((*module_name).clone(), name.clone())); + record_home.insert(name.clone(), origin.clone()); + } + } + // Transitively carried opaque handle types: bare identity name only, and a + // taken name is skipped silently (there are no fields to hint about). + for module_name in &module_names { + for op in &imports[*module_name].opaques { + if op.origin.is_none() || decls.type_arity.contains_key(&op.name) { + continue; + } + decls.type_arity.insert(op.name.clone(), op.arity); } } accepted @@ -2400,23 +2656,36 @@ fn merge_imported_types( decls.type_ctors.insert(ty.name.clone(), ctor_names); } } - for module_name in &module_names { - for (name, info) in &imports[*module_name].records { - if !accepted.contains(&((*module_name).clone(), name.clone())) { - continue; - } - decls.records.insert(name.clone(), info.clone()); - decls - .record_aliases - .insert(format!("{module_name}.{name}"), name.clone()); - // Fields join the multimap under the record's bare identity name. Local - // records were registered first (in `build_decls`), so they lead. - for (field, _) in &info.fields { - decls - .field_owner - .entry(field.clone()) - .or_default() - .push(name.clone()); + // Directly imported records first, then transitively carried ones, so the + // field multimap keeps a stable order: local records (from `build_decls`), + // then direct imports by module name, then carried records. A carried record + // joins `records` and `field_owner` like any other — it participates in + // field resolution and access-site ambiguity identically — but gets **no** + // qualified construction alias: constructing or pattern-matching it still + // requires importing its declaring module directly. + for transitive in [false, true] { + for module_name in &module_names { + for rec in &imports[*module_name].records { + if rec.origin.is_some() != transitive + || !accepted.contains(&((*module_name).clone(), rec.name.clone())) + { + continue; + } + let name = &rec.name; + decls.records.insert(name.clone(), rec.info.clone()); + if rec.origin.is_none() { + decls + .record_aliases + .insert(format!("{module_name}.{name}"), name.clone()); + } + // Fields join the multimap under the record's bare identity name. + for (field, _) in &rec.info.fields { + decls + .field_owner + .entry(field.clone()) + .or_default() + .push(name.clone()); + } } } } @@ -7139,8 +7408,9 @@ impl Infer { .filter(|&(d, n)| d <= (field.chars().count().max(n.chars().count()) / 3).max(2)) .map(|(_, n)| format!(" (did you mean `{n}`?)")) .unwrap_or_default(); + let hidden = self.hidden_field_hint(field); return Err(TypeError { - message: format!("record `{rec}` has no field `{field}`{near}"), + message: format!("record `{rec}` has no field `{field}`{near}{hidden}"), span, }); } @@ -7157,6 +7427,16 @@ impl Infer { Some([only]) => Ok(only.clone()), Some(owners) if owners.len() >= 2 => Err(self.ambiguous_field(field, span)), _ => { + // A record carried transitively but shadowed by a same-named type + // is out of scope, yet its interface data still says where the + // field lives — say so instead of a dead-end "unknown". + let hidden = self.hidden_field_hint(field); + if !hidden.is_empty() { + return Err(TypeError { + message: format!("unknown record field `{field}`{hidden}"), + span, + }); + } // Empty `decls.records` means records aren't in use at all. let hint = if self.decls.records.is_empty() { " (no record types are declared)" @@ -7171,6 +7451,21 @@ impl Infer { } } + /// A rendered note when `field` is declared by a record the consumer's + /// interface data knows of but that is **not in scope here** (a transitively + /// carried record whose bare name a local or directly imported type already + /// holds — `Decls::field_hints`). Empty when there is nothing to say, so the + /// plain messages stay as they are. + fn hidden_field_hint(&self, field: &str) -> String { + match self.decls.field_hints.get(field).and_then(|h| h.first()) { + Some((rec, module)) => format!( + "; the record `{rec}` in module `{module}` declares this field, but another \ + type named `{rec}` is already in scope here, so that record is hidden" + ), + None => String::new(), + } + } + /// Resolve a surface record tag (`Point` or `Geometry.Point`) at a construction /// or pattern site to its **bare identity name** (`DESIGN.md` §8.3). A qualified /// tag resolves via the imported-record alias table; a bare tag resolves only to diff --git a/tests/project.rs b/tests/project.rs index 1a52447..f761051 100644 --- a/tests/project.rs +++ b/tests/project.rs @@ -879,6 +879,398 @@ fn a_cross_module_newtype_stays_distinct_from_its_underlying() { ); } +// ---------- transitive interface closure (issues #72 / #75) ---------- + +/// The three-module shape of issue #75: `Inner` declares a record, `Middle` +/// wraps it, and the consumer imports only `Middle`. +const INNER: &str = "type Config = { cSize: int }\nlet make n = Config { cSize = n }"; +const MIDDLE: &str = "import Inner\n\ + type Wrap = { wConf: Inner.Config }\n\ + let mk n = Wrap { wConf = Inner.make n }\n\ + let size w = w.wConf.cSize"; + +#[test] +fn e2e_an_extern_type_can_be_named_across_the_module_boundary() { + // issue #72: the *name* of an `extern type` crosses with its module's + // interface, so a consumer can type a record field `Gen.Rng` (values of the + // type always crossed through the imported schemes). + let files = compile( + "Main", + &[ + ( + "Gen", + "extern type Rng\n\ + extern newRng : int -> Rng = random.Random\n\ + let make seed = newRng seed", + ), + ( + "Main", + "import Gen\n\ + type Holder = { h: Gen.Rng }\n\ + let x = Holder { h = Gen.make 1 }\n\ + print \"ok\"", + ), + ], + ); + let dir = Scratch::new("e2e_extern_type_name"); + if let Some(out) = run_project(&dir, &files, "main.py") { + assert_eq!(out.trim(), "ok"); + } +} + +#[test] +fn an_unknown_type_qualified_to_an_opaque_exporting_module_reports_once() { + // A genuinely unknown qualified type still reports cleanly — one error, no + // cascade — now that the module also exports opaque type names. + let project = build_mem( + "Main", + &[ + ( + "Gen", + "extern type Rng\nextern newRng : int -> Rng = random.Random", + ), + ("Main", "import Gen\ntype Holder = { h: Gen.Nope }"), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].errors.len(), 1, "got: {:?}", errors[0].errors); + assert!( + errors[0].errors[0] + .message + .contains("unknown type `Gen.Nope`"), + "got: {}", + errors[0].errors[0].message + ); +} + +#[test] +fn an_opaque_type_carried_transitively_can_be_named_bare() { + // `Middle.fresh : int -> Rng` mentions Gen's handle type, so `Rng` crosses + // with Middle's interface and the consumer can name it (bare — constructing + // or qualifying still asks for the declaring module). + let project = build_mem( + "Main", + &[ + ( + "Gen", + "extern type Rng\n\ + extern newRng : int -> Rng = random.Random\n\ + let make seed = newRng seed", + ), + ("Middle", "import Gen\nlet fresh n = Gen.make n"), + ( + "Main", + "import Middle\n\ + type H = { h: Rng }\n\ + let x = H { h = Middle.fresh 1 }", + ), + ], + ); + let errors = project::check(&project); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); +} + +#[test] +fn a_local_type_clashing_with_an_imported_opaque_type_is_reported() { + // A handle type now registers like any imported type, so a same-named local + // declaration is the ordinary import clash (it would otherwise silently + // conflate two distinct nominal types). + let project = build_mem( + "Main", + &[ + ( + "Gen", + "extern type Rng\nextern newRng : int -> Rng = random.Random", + ), + ("Main", "import Gen\ntype Rng = A | B\nlet x = A"), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert!( + errors[0].errors[0] + .message + .contains("imported type `Rng` (from `Gen`) clashes"), + "got: {}", + errors[0].errors[0].message + ); +} + +#[test] +fn e2e_a_transitive_record_field_is_accessible() { + // issue #75: `Wrap`'s field is typed `Inner.Config`, so `Config` crosses + // with Middle's interface and the consumer can read `x.wConf.cSize` without + // importing `Inner` directly. + let files = compile( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\nlet x = Middle.mk 5\nprint x.wConf.cSize", + ), + ], + ); + let dir = Scratch::new("e2e_transitive_field"); + if let Some(out) = run_project(&dir, &files, "main.py") { + assert_eq!(out.trim(), "5"); + } +} + +#[test] +fn e2e_a_record_update_through_a_transitive_boundary_routes_to_the_declaring_class() { + // An update reconstructs the record, so the consumer's emitted Python must + // reference the *declaring* module's class (`inner.Config`), with its import + // hoisted, even though the consumer never writes `import Inner`. + let files = compile( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\n\ + let x = Middle.mk 5\n\ + let y = { x with wConf.cSize = 9 }\n\ + let z = { x.wConf with cSize = 7 }\n\ + print y.wConf.cSize\n\ + print z.cSize", + ), + ], + ); + let main = file(&files, "main.py"); + assert!(main.contains("import inner"), "{main}"); + assert!(main.contains("inner.Config("), "{main}"); + let dir = Scratch::new("e2e_transitive_update"); + if let Some(out) = run_project(&dir, &files, "main.py") { + assert_eq!(out.replace("\r\n", "\n").trim(), "9\n7"); + } +} + +#[test] +fn e2e_a_transitive_record_crosses_any_depth() { + // The record is declared three imports away; each hop's interface carries it + // onward, so the chain works at any depth. + let files = compile( + "Main", + &[ + ( + "Deep", + "type Core = { cv: int }\nlet mkCore n = Core { cv = n }", + ), + ( + "Mid", + "import Deep\n\ + type Two = { w1: Deep.Core }\n\ + let mk2 n = Two { w1 = Deep.mkCore n }", + ), + ( + "Top", + "import Mid\n\ + type Three = { w2: Mid.Two }\n\ + let mk3 n = Three { w2 = Mid.mk2 n }", + ), + ("Main", "import Top\nlet x = Top.mk3 7\nprint x.w2.w1.cv"), + ], + ); + let dir = Scratch::new("e2e_transitive_depth"); + if let Some(out) = run_project(&dir, &files, "main.py") { + assert_eq!(out.trim(), "7"); + } +} + +#[test] +fn mutually_referencing_records_cross_transitively() { + // Two records that reference each other (via Option) are legal; the export + // closure must follow the cycle and terminate rather than recurse forever. + let project = build_mem( + "Main", + &[ + ( + "Inner", + "type A = { av: int, ab: Option B }\n\ + type B = { bv: int, ba: Option A }\n\ + let mkA n = A { av = n, ab = None }\n\ + let mkB n = B { bv = n, ba = Some (mkA n) }", + ), + ("Middle", "import Inner\nlet make n = Inner.mkB n"), + ( + "Main", + "import Middle\n\ + let b = Middle.make 3\n\ + let v = b.bv\n\ + let w = Option.map (fun a -> a.av) b.ba", + ), + ], + ); + let errors = project::check(&project); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); +} + +#[test] +fn a_transitive_record_participates_in_field_ambiguity() { + // A local record and a transitively carried one both declare `cSize`; a bare + // access on an untyped base is ambiguous at the access site, exactly as with + // a directly imported record. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\n\ + type Local = { cSize: int, other: int }\n\ + let get w = w.cSize", + ), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert!( + errors[0].errors[0].message.contains("ambiguous"), + "got: {}", + errors[0].errors[0].message + ); +} + +#[test] +fn a_transitive_record_shadowed_by_a_local_name_hints_at_its_home() { + // A local type already holds the name `Config`, so the carried record stays + // out of scope (local and direct declarations win the bare name, and the + // consumer never asked for the carried one — no clash error). Its fields + // still feed the diagnostics: both the known-base and the untyped-base + // failure name the record and the module that declares it. + let with_known_base = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\n\ + type Config = { other: int }\n\ + let x = Middle.mk 5\n\ + let bad = x.wConf.cSize", + ), + ], + ); + let errors = project::check(&with_known_base); + assert_eq!(errors.len(), 1); + let message = &errors[0].errors[0].message; + assert!( + message.contains("has no field `cSize`") + && message.contains("module `Inner`") + && message.contains("hidden"), + "got: {message}" + ); + + let with_untyped_base = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\n\ + type Config = { other: int }\n\ + let get w = w.cSize", + ), + ], + ); + let errors = project::check(&with_untyped_base); + assert_eq!(errors.len(), 1); + let message = &errors[0].errors[0].message; + assert!( + message.contains("unknown record field `cSize`") && message.contains("module `Inner`"), + "got: {message}" + ); +} + +#[test] +fn a_diamond_of_carried_records_does_not_clash() { + // Two imports both carry `Inner.Config` transitively; the consumer must see + // one record, not a spurious clash for a name it never asked for. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ( + "Left", + "import Inner\n\ + type LWrap = { lc: Inner.Config }\n\ + let mkL n = LWrap { lc = Inner.make n }", + ), + ( + "Right", + "import Inner\n\ + type RWrap = { rc: Inner.Config }\n\ + let mkR n = RWrap { rc = Inner.make n }", + ), + ( + "Main", + "import Left\n\ + import Right\n\ + let a = (Left.mkL 1).lc.cSize\n\ + let b = (Right.mkR 2).rc.cSize", + ), + ], + ); + let errors = project::check(&project); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); +} + +#[test] +fn importing_the_declaring_module_alongside_the_carrier_is_fine() { + // Direct import and transitive carry of the same record coexist: one record, + // full access, and construction through the direct import. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Inner\n\ + import Middle\n\ + let x = Middle.mk 5\n\ + let a = x.wConf.cSize\n\ + let mine = Inner.Config { cSize = 1 }", + ), + ], + ); + let errors = project::check(&project); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); +} + +#[test] +fn a_carried_record_cannot_be_constructed_without_a_direct_import() { + // The carried record crosses as an identity (naming, unification, field + // access), not as a member: constructing it still asks for `import Inner`. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\nlet bad = Inner.Config { cSize = 1 }", + ), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert!( + errors[0].errors[0] + .message + .contains("not a member of `Inner`"), + "got: {}", + errors[0].errors[0].message + ); +} + #[test] fn a_binding_colliding_with_a_module_alias_gets_a_mangled_import() { // `import Ids` + `let ids = …` used to emit `ids = …` after `import ids`, From a1f503ddcf3a3fcccf37e48ba7f8f369657d4e6c Mon Sep 17 00:00:00 2001 From: simontreanor Date: Sat, 29 Aug 2026 10:24:52 +0100 Subject: [PATCH 2/3] types: refusing a carried record names the module to import Constructing or pattern-matching a transitively carried record bare said "not a record type", which the surrounding program visibly contradicts: the same name reads fields and updates fine. The two rejection sites now use the carried entry's declaring module (Decls::carried_record_home) to state the real situation and the fix: "the record `Config` is declared in module `Inner`; import `Inner` to construct its values here" (the pattern site says "to match"). A name that is no record at all keeps the old message, and qualified tags behave as before. --- INTERNALS.md | 5 ++++- src/types/mod.rs | 29 +++++++++++++++++++++++++---- tests/project.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/INTERNALS.md b/INTERNALS.md index 6a6d5c8..a55dfe5 100644 --- a/INTERNALS.md +++ b/INTERNALS.md @@ -468,7 +468,10 @@ only** (no qualified key, no `record_aliases` entry, so constructing or pattern- still requires the direct import); a taken bare name skips the carried entry silently (`record_home` recognizes the same record arriving along two paths), and a genuinely shadowed record's fields land in `Decls::field_hints`, which `record_of_field` / `record_of_field_on` render as "the record `Config` in -module `Inner` declares this field" instead of a bare unknown. The lowering side: +module `Inner` declares this field" instead of a bare unknown. Constructing or matching a carried record +bare is refused with the fix spelled out (`resolve_record_tag` via `Decls::carried_record_home`: "the +record `Config` is declared in module `Inner`; import `Inner` to construct its values here", the pattern +site says "to match"), while a name that is no record at all keeps "not a record type". The lowering side: `ModuleExports::carried_records` feeds `project::compile`'s per-module `ImportContext::record_class_modules` plus field data keyed by the *declaring* module's tag, so a record **update** in a consumer that never imports the declaring module still reconstructs via the right class (`inner.Config(...)`, diff --git a/src/types/mod.rs b/src/types/mod.rs index 9eef480..84284c7 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -1805,6 +1805,13 @@ struct Decls { /// name (`Point`), so a qualified construction/pattern resolves to the same /// `RecordInfo` (and `Ty::Con`) the exporting module uses. record_aliases: HashMap, + /// Bare identity name → declaring module for each record an import's + /// interface carries **transitively** and that registered here. Such a + /// record deliberately has no construction alias (constructing or matching + /// it requires importing its declaring module directly), so the tag + /// resolver uses this to say exactly that instead of the false + /// "not a record type". + carried_record_home: HashMap, /// Field name → `(record, declaring module)` for each record an import's /// interface carries **transitively** whose bare name is already taken here /// (by a local type or another import), so the record itself stays out of @@ -2613,6 +2620,9 @@ fn merge_imported_type_names( decls.type_arity.insert(name.clone(), rec.info.params_count); accepted.insert(((*module_name).clone(), name.clone())); record_home.insert(name.clone(), origin.clone()); + decls + .carried_record_home + .insert(name.clone(), origin.clone()); } } // Transitively carried opaque handle types: bare identity name only, and a @@ -7470,9 +7480,12 @@ impl Infer { /// or pattern site to its **bare identity name** (`DESIGN.md` §8.3). A qualified /// tag resolves via the imported-record alias table; a bare tag resolves only to /// a **local** record (an imported record must be tagged qualified, exactly as an - /// imported sum-type constructor must be). Anything else is "not a record type" + /// imported sum-type constructor must be). A bare tag naming a **transitively + /// carried** record is a record type, just not one this module may construct or + /// match, so the error says so and names the module to import (`verb` is the + /// site's word: "construct" or "match"). Anything else is "not a record type" /// (or "not a member of `M`" for an unknown qualified tag). - fn resolve_record_tag(&self, tag: &str, span: Span) -> Result { + fn resolve_record_tag(&self, tag: &str, span: Span, verb: &str) -> Result { if let Some((module, rec)) = tag.split_once('.') { if let Some(bare) = self.decls.record_aliases.get(tag) { return Ok(bare.clone()); @@ -7484,6 +7497,14 @@ impl Infer { } if self.decls.local_records.contains(tag) { Ok(tag.to_string()) + } else if let Some(home) = self.decls.carried_record_home.get(tag) { + Err(TypeError { + message: format!( + "the record `{tag}` is declared in module `{home}`; import `{home}` \ + to {verb} its values here" + ), + span, + }) } else { Err(TypeError { message: format!("`{tag}` is not a record type"), @@ -7539,7 +7560,7 @@ impl Infer { span: Span, env: &Env, ) -> Result { - let owner = self.resolve_record_tag(ty, ty_span)?; + let owner = self.resolve_record_tag(ty, ty_span, "construct")?; let (record_ty, field_tys) = self.instantiate_record(&owner); let mut seen: HashSet = HashSet::new(); @@ -7985,7 +8006,7 @@ impl Infer { ty_span, fields, } => { - let owner = self.resolve_record_tag(ty, ty_span.span())?; + let owner = self.resolve_record_tag(ty, ty_span.span(), "match")?; let (record_ty, field_tys) = self.instantiate_record(&owner); self.unify(&record_ty, scrut_ty, span)?; let mut seen: HashSet = HashSet::new(); diff --git a/tests/project.rs b/tests/project.rs index f761051..6ec9dee 100644 --- a/tests/project.rs +++ b/tests/project.rs @@ -1271,6 +1271,54 @@ fn a_carried_record_cannot_be_constructed_without_a_direct_import() { ); } +#[test] +fn a_bare_carried_record_construction_names_the_declaring_module() { + // `Config` *is* a record type here (its fields are readable through the same + // name), so the refusal must say what is actually missing: the direct import + // that construction requires. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ("Main", "import Middle\nlet bad = Config { cSize = 1 }"), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].errors.len(), 1, "got: {:?}", errors[0].errors); + assert_eq!( + errors[0].errors[0].message, + "the record `Config` is declared in module `Inner`; import `Inner` to construct \ + its values here" + ); +} + +#[test] +fn a_bare_carried_record_pattern_names_the_declaring_module() { + // The pattern site gets the same treatment, worded for matching. + let project = build_mem( + "Main", + &[ + ("Inner", INNER), + ("Middle", MIDDLE), + ( + "Main", + "import Middle\n\ + let f w =\n match w:\n case Config { cSize = n }: n", + ), + ], + ); + let errors = project::check(&project); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].errors.len(), 1, "got: {:?}", errors[0].errors); + assert_eq!( + errors[0].errors[0].message, + "the record `Config` is declared in module `Inner`; import `Inner` to match \ + its values here" + ); +} + #[test] fn a_binding_colliding_with_a_module_alias_gets_a_mangled_import() { // `import Ids` + `let ids = …` used to emit `ids = …` after `import ids`, From 1c7f7dea2f2b47d06301615e56cb222c13274f56 Mon Sep 17 00:00:00 2001 From: simontreanor Date: Sat, 29 Aug 2026 10:55:38 +0100 Subject: [PATCH 3/3] types: carried records are a fallback tier for by-name field lookup A carried record used to join the field-owner multimap as a peer, so a dependency two hops away could make a working access ambiguous: a consumer with a local record declaring size stopped compiling when a module it never imports gained a record with the same field. By-name field lookup now applies the precedence the bare-name rule already established: records declared here or imported directly form the deciding tier, and a carried record owns a field only when no record in that tier declares it (field_owner_tier, consulted by record_of_field and the pending-field fallback for a base still unknown at the end of its binding). Ambiguity is reported within whichever tier decided, so two carried records that alone declare a field still tie and the message names them. Every resolution that worked before is unchanged, and the issue 75 repro still resolves, since its field has a single owner. --- DESIGN.md | 7 +++- INTERNALS.md | 7 +++- src/types/mod.rs | 103 +++++++++++++++++++++++++++++++---------------- tests/project.rs | 85 +++++++++++++++++++++++++++++++++----- 4 files changed, 157 insertions(+), 45 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 9d4a815..9b52a7b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -919,7 +919,12 @@ directly. A carried name already taken here (by a local type or a direct import) than reported, since the consumer never wrote that name: local and direct declarations win the bare name, the same record arriving along two import paths is recognized by its declaring module and admitted once, and a genuinely different type shadows the carried record, whose fields then feed the "unknown record -field" diagnostic (the message names the hidden record and the module that declares it). **Externs and +field" diagnostic (the message names the hidden record and the module that declares it). Field lookup +mirrors the same precedence: in a by-name field access (a base whose type is not yet known), records +declared here or imported directly form the deciding tier, and a carried record owns a field only when no +record in that tier declares it, so editing a dependency two hops away can never make a working access +ambiguous. Two carried records that alone declare a field still tie, and the ambiguity message names +them. **Externs and measures cross too:** an imported `extern` (`Mathx.cbrt`) is exported like a value (its scheme joins the interface) and — in the project lowering path — also **bound at top level in its own module** (`cbrt = math.cbrt`, `import math` hoisted) so a dependent module references it as `mathx.cbrt`; single-file lowering diff --git a/INTERNALS.md b/INTERNALS.md index a55dfe5..8d4a46d 100644 --- a/INTERNALS.md +++ b/INTERNALS.md @@ -466,7 +466,12 @@ records. Each import's interface is itself closed, so one pull per hop reaches a side `merge_imported_type_names` registers carried entries **last** and under the **bare identity name only** (no qualified key, no `record_aliases` entry, so constructing or pattern-matching a carried record still requires the direct import); a taken bare name skips the carried entry silently (`record_home` -recognizes the same record arriving along two paths), and a genuinely shadowed record's fields land in +recognizes the same record arriving along two paths). By-name field lookup applies the same precedence in +tiers (`Infer::field_owner_tier`, consulted by `record_of_field` and the pending-field fallback): local +and directly imported records decide first, and a carried record owns a field only when no direct one +declares it, so a carried record can never make an existing access ambiguous; a tie between carried +records with no direct owner is still the ambiguity error, naming the tier that decided. A genuinely +shadowed record's fields land in `Decls::field_hints`, which `record_of_field` / `record_of_field_on` render as "the record `Config` in module `Inner` declares this field" instead of a bare unknown. Constructing or matching a carried record bare is refused with the fix spelled out (`resolve_record_tag` via `Decls::carried_record_home`: "the diff --git a/src/types/mod.rs b/src/types/mod.rs index 84284c7..6cbca94 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -2669,10 +2669,12 @@ fn merge_imported_types( // Directly imported records first, then transitively carried ones, so the // field multimap keeps a stable order: local records (from `build_decls`), // then direct imports by module name, then carried records. A carried record - // joins `records` and `field_owner` like any other — it participates in - // field resolution and access-site ambiguity identically — but gets **no** - // qualified construction alias: constructing or pattern-matching it still - // requires importing its declaring module directly. + // joins `records` and `field_owner`, but in a by-name field lookup it is a + // **fallback tier**: a field it declares resolves to it only when no local or + // directly imported record declares that field (`field_owner_tier`), so a + // carried record can never make an existing access ambiguous. It also gets + // **no** qualified construction alias: constructing or pattern-matching it + // still requires importing its declaring module directly. for transitive in [false, true] { for module_name in &module_names { for rec in &imports[*module_name].records { @@ -7321,15 +7323,11 @@ impl Infer { Ok(Ty::Unit) } - /// The ambiguity error, naming both ways out. The parameter form is usually the + /// The ambiguity error, naming both ways out. `owners` is the tier that + /// decided the lookup ([`Infer::field_owner_tier`]), so the message names + /// only the records that actually competed. The parameter form is usually the /// nicer one and is easy to miss, since it arrived after the message did. - fn ambiguous_field(&self, field: &str, span: Span) -> TypeError { - let owners = self - .decls - .field_owner - .get(field) - .cloned() - .unwrap_or_default(); + fn ambiguous_field(&self, field: &str, owners: &[String], span: Span) -> TypeError { let names = owners .iter() .map(|r| format!("`{r}`")) @@ -7348,7 +7346,10 @@ impl Infer { } /// Whether this field name is declared by more than one visible record, so an - /// access on an unknown base cannot be resolved by name alone. + /// access on an unknown base cannot be resolved by name alone. Deliberately + /// counts **all** owners, carried included: deferring the access keeps the + /// chance to resolve it by the base's actual type once inference pins it, + /// which beats any by-name precedence. fn field_is_ambiguous(&self, field: &str) -> bool { self.decls .field_owner @@ -7356,27 +7357,60 @@ impl Infer { .is_some_and(|owners| owners.len() >= 2) } + /// The record names competing to own `field` in a **by-name** lookup, tiered + /// (`DESIGN.md` §6.1): records this module declares or imports directly + /// decide first, and records carried transitively are a fallback consulted + /// only when no direct one declares the field. This mirrors the bare-name + /// rule for carried type names (local and direct declarations win, since the + /// consumer never wrote the carried name): without the tier, editing a + /// dependency two hops away could make a working access ambiguous. + fn field_owner_tier(&self, field: &str) -> Vec { + let owners = self + .decls + .field_owner + .get(field) + .map(Vec::as_slice) + .unwrap_or(&[]); + let direct: Vec = owners + .iter() + .filter(|o| !self.decls.carried_record_home.contains_key(*o)) + .cloned() + .collect(); + if direct.is_empty() { + owners.to_vec() + } else { + direct + } + } + /// Settle every deferred field access, now that the enclosing binding is fully /// inferred. A base that some later statement pinned down resolves exactly as it - /// would have at the access site; one that is *still* unknown is the genuine - /// ambiguity, and is reported at the access with the ways out. + /// would have at the access site; one that is *still* unknown falls back to the + /// by-name lookup with its owner tiers ([`Infer::record_of_field`]), so a field + /// whose only competition is a carried record still resolves to the direct one, + /// and only a genuine within-tier tie is reported as the ambiguity. fn resolve_pending_fields(&mut self) -> Result<(), TypeError> { let pending = std::mem::take(&mut self.pending_fields); for p in pending { let base = self.apply(&p.base); - let Ty::Con(record, _) = &base else { - return Err(self.ambiguous_field(&p.field, p.span)); - }; - let Some(info) = self.decls.records.get(record) else { - return Err(self.ambiguous_field(&p.field, p.span)); + let owner = if let Ty::Con(record, _) = &base { + let Some(info) = self.decls.records.get(record) else { + return Err(self.ambiguous_field( + &p.field, + &self.field_owner_tier(&p.field), + p.span, + )); + }; + if !info.fields.iter().any(|(n, _)| *n == p.field) { + return Err(TypeError { + message: format!("record `{record}` has no field `{}`", p.field), + span: p.span, + }); + } + record.clone() + } else { + self.record_of_field(&p.field, p.span)? }; - if !info.fields.iter().any(|(n, _)| *n == p.field) { - return Err(TypeError { - message: format!("record `{record}` has no field `{}`", p.field), - span: p.span, - }); - } - let owner = record.clone(); let (record_ty, field_tys) = self.instantiate_record(&owner); self.unify(&record_ty, &base, p.span)?; let fty = field_tys @@ -7428,14 +7462,15 @@ impl Infer { } /// The bare identity name of the record type owning `field`, **by name alone** - /// (`DESIGN.md` §8.3) — the fallback for a base whose type is not yet known: - /// **0** owners is an unknown field, **1** owner resolves, **2+** is an ambiguity - /// error *at this access site*. Ambiguity is never an error at declaration or - /// import; module isolation is preserved. + /// (`DESIGN.md` §8.3) — the fallback for a base whose type is not yet known. + /// Owners are consulted in tiers ([`Infer::field_owner_tier`]): within the + /// deciding tier, **0** owners is an unknown field, **1** owner resolves, + /// **2+** is an ambiguity error *at this access site*. Ambiguity is never an + /// error at declaration or import; module isolation is preserved. fn record_of_field(&self, field: &str, span: Span) -> Result { - match self.decls.field_owner.get(field).map(Vec::as_slice) { - Some([only]) => Ok(only.clone()), - Some(owners) if owners.len() >= 2 => Err(self.ambiguous_field(field, span)), + match self.field_owner_tier(field).as_slice() { + [only] => Ok(only.clone()), + owners if owners.len() >= 2 => Err(self.ambiguous_field(field, owners, span)), _ => { // A record carried transitively but shadowed by a same-named type // is out of scope, yet its interface data still says where the diff --git a/tests/project.rs b/tests/project.rs index 6ec9dee..ffb376c 100644 --- a/tests/project.rs +++ b/tests/project.rs @@ -1110,10 +1110,13 @@ fn mutually_referencing_records_cross_transitively() { } #[test] -fn a_transitive_record_participates_in_field_ambiguity() { - // A local record and a transitively carried one both declare `cSize`; a bare - // access on an untyped base is ambiguous at the access site, exactly as with - // a directly imported record. +fn a_direct_record_wins_a_field_name_over_a_carried_one() { + // A local record and a transitively carried one both declare `cSize`. Carried + // records are a fallback tier for by-name field lookup (mirroring the + // bare-name rule: local and direct declarations win, the consumer never + // wrote the carried name), so the access resolves to `Local` and stays + // unambiguous — a record in a module this file cannot see must not be able + // to break a working access. let project = build_mem( "Main", &[ @@ -1122,17 +1125,81 @@ fn a_transitive_record_participates_in_field_ambiguity() { ( "Main", "import Middle\n\ - type Local = { cSize: int, other: int }\n\ - let get w = w.cSize", + type Local = { cSize: bool, other: int }\n\ + let get w = w.cSize\n\ + let v = get (Local { cSize = true, other = 1 })", ), ], ); let errors = project::check(&project); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); +} + +#[test] +fn e2e_a_carried_record_cannot_break_a_working_field_access() { + // The full breaking shape: the consumer's program never names `Inner`, so a + // shared field name there must not reach its `p.size` — the access keeps + // resolving to the local record and the program keeps running. + let files = compile( + "Main", + &[ + ( + "Inner", + "type Config = { size: int }\nlet make n = Config { size = n }", + ), + ( + "Middle", + "import Inner\n\ + type Wrap = { wConf: Inner.Config }\n\ + let mk n = Wrap { wConf = Inner.make n }", + ), + ( + "Main", + "import Middle\n\ + type Local = { size: bool }\n\ + let f p = p.size\n\ + print (f (Local { size = true }))", + ), + ], + ); + let dir = Scratch::new("e2e_tiered_field"); + if let Some(out) = run_project(&dir, &files, "main.py") { + assert_eq!(out.trim(), "True"); + } +} + +#[test] +fn two_carried_records_sharing_a_field_are_still_ambiguous() { + // With nothing local or direct declaring `size`, the carried tier decides + // the lookup, and a tie within it is the genuine ambiguity — the message + // names the carried records. + let project = build_mem( + "Main", + &[ + ( + "Twin", + "type A = { size: int }\n\ + type B = { size: bool }\n\ + let mkA n = A { size = n }\n\ + let mkB b = B { size = b }", + ), + ( + "Carry", + "import Twin\n\ + type WrapA = { wa: Twin.A }\n\ + type WrapB = { wb: Twin.B }\n\ + let mka n = WrapA { wa = Twin.mkA n }\n\ + let mkb b = WrapB { wb = Twin.mkB b }", + ), + ("Main", "import Carry\nlet get w = w.size"), + ], + ); + let errors = project::check(&project); assert_eq!(errors.len(), 1); + let message = &errors[0].errors[0].message; assert!( - errors[0].errors[0].message.contains("ambiguous"), - "got: {}", - errors[0].errors[0].message + message.contains("ambiguous") && message.contains("`A`") && message.contains("`B`"), + "got: {message}" ); }