iter ctt.2: Registry.type_def_module re-key to (owning_module, bare_name)
This commit is contained in:
@@ -1637,7 +1637,9 @@ fn check_fn(f: &FnDef, env: &Env) -> Result<()> {
|
||||
// same key shape `workspace::build_registry` writes with, so
|
||||
// representation-equal types match deterministically.
|
||||
// normalize to match Registry::normalize_type_for_lookup contract
|
||||
let r_ty_norm = env.workspace_registry.normalize_type_for_lookup(&r_ty);
|
||||
let r_ty_norm = env
|
||||
.workspace_registry
|
||||
.normalize_type_for_lookup(env.current_module.as_str(), &r_ty);
|
||||
let key = (
|
||||
r.class.clone(),
|
||||
ailang_core::canonical::type_hash(&r_ty_norm),
|
||||
|
||||
@@ -118,7 +118,9 @@ pub fn monomorphise_workspace(ws: &Workspace) -> Result<Workspace> {
|
||||
let (f, defining_module) = match t {
|
||||
MonoTarget::ClassMethod { class, type_, defining_module, .. } => {
|
||||
// normalize to match Registry::normalize_type_for_lookup contract
|
||||
let t_ty_norm = ws_owned.registry.normalize_type_for_lookup(type_);
|
||||
let t_ty_norm = ws_owned
|
||||
.registry
|
||||
.normalize_type_for_lookup(defining_module.as_str(), type_);
|
||||
let registry_key =
|
||||
(class.clone(), ailang_core::canonical::type_hash(&t_ty_norm));
|
||||
let entry = ws_owned
|
||||
@@ -621,7 +623,9 @@ pub fn collect_mono_targets(
|
||||
continue;
|
||||
}
|
||||
// normalize to match Registry::normalize_type_for_lookup contract
|
||||
let r_ty_norm = env.workspace_registry.normalize_type_for_lookup(&r_ty);
|
||||
let r_ty_norm = env
|
||||
.workspace_registry
|
||||
.normalize_type_for_lookup(module_name, &r_ty);
|
||||
let key = (r.class.clone(), ailang_core::canonical::type_hash(&r_ty_norm));
|
||||
let entry = match env.workspace_registry.entries.get(&key) {
|
||||
Some(e) => e,
|
||||
@@ -1172,7 +1176,9 @@ pub(crate) fn collect_residuals_ordered(
|
||||
if !crate::is_fully_concrete(&r_ty) {
|
||||
return None;
|
||||
}
|
||||
let r_ty_norm = env.workspace_registry.normalize_type_for_lookup(&r_ty);
|
||||
let r_ty_norm = env
|
||||
.workspace_registry
|
||||
.normalize_type_for_lookup(module_name, &r_ty);
|
||||
let key = (r.class.clone(), ailang_core::canonical::type_hash(&r_ty_norm));
|
||||
let entry = env.workspace_registry.entries.get(&key)?;
|
||||
Some(MonoTarget::ClassMethod {
|
||||
|
||||
@@ -93,33 +93,39 @@ pub struct Registry {
|
||||
/// Populated in [`build_registry`] from the same scan that builds
|
||||
/// the entry map.
|
||||
///
|
||||
/// Note: keyed by bare type name only. If two modules each define a
|
||||
/// type with the same bare name (e.g. `type Foo` in both `M` and
|
||||
/// `N`), the second `insert` overwrites the first, and
|
||||
/// [`Self::normalize_type_for_lookup`] would collapse `M.Foo` and
|
||||
/// `N.Foo` to whichever module wins the race. Acceptable for the
|
||||
/// current corpus (all in-tree fixtures use distinct bare type
|
||||
/// names across modules); revisit if a future workspace breaks the
|
||||
/// assumption. Proper fix is to re-key as
|
||||
/// `(owning_module, bare_name) -> defining_module` and thread the
|
||||
/// calling module through every consumer site — out of ct.1's scope.
|
||||
pub type_def_module: BTreeMap<String, String>,
|
||||
/// Keyed by `(owning_module, bare_name)`, value is the
|
||||
/// defining module. The tuple key disambiguates same-named
|
||||
/// types declared in different modules — bare `Foo` from
|
||||
/// module M is `(M, "Foo")`, bare `Foo` from module N is
|
||||
/// `(N, "Foo")`, and the two carry distinct canonical
|
||||
/// qualifications under
|
||||
/// [`normalize_type_for_registry`]. Pre-ctt.2 the key was
|
||||
/// the bare name alone, and a workspace with two `type Foo`
|
||||
/// declarations silently overwrote one entry, then tripped
|
||||
/// `DuplicateInstance` on the loser-side instance after
|
||||
/// both qualified to `<winner>.Foo`.
|
||||
pub type_def_module: BTreeMap<(String, String), String>,
|
||||
}
|
||||
|
||||
impl Registry {
|
||||
/// ct.1.5a: produce the canonical form of `t` for registry-key
|
||||
/// hashing. Bare-non-primitive `Type::Con` names get qualified to
|
||||
/// `<defining_module>.<name>`; already-qualified names stay; bare
|
||||
/// names whose defining module is unknown stay as-is.
|
||||
/// `Type::Fn`/`Type::Forall`/`Type::Var` recurse / pass through
|
||||
/// structurally.
|
||||
/// ct.1.5a + ctt.2: produce the canonical form of `t` for
|
||||
/// registry-key hashing. Bare-non-primitive `Type::Con` names
|
||||
/// get qualified to `<defining_module>.<name>`; already-qualified
|
||||
/// names stay; bare names whose defining module is unknown stay
|
||||
/// as-is. `Type::Fn`/`Type::Forall`/`Type::Var` recurse / pass
|
||||
/// through structurally.
|
||||
///
|
||||
/// Every consumer that hashes an `inst.type_`-shaped expression to
|
||||
/// look it up in [`Self::entries`] must funnel through this
|
||||
/// `caller_module` is the module in whose scope `t` was authored.
|
||||
/// Bare-name lookups are keyed by `(caller_module, name)`, so a
|
||||
/// bare `Foo` written in module M resolves only to M's `Foo`,
|
||||
/// never to a same-named type from another module.
|
||||
///
|
||||
/// Every consumer that hashes an `inst.type_`-shaped expression
|
||||
/// to look it up in [`Self::entries`] must funnel through this
|
||||
/// helper, otherwise the registered-form and the queried-form
|
||||
/// disagree on whether the leading qualifier is present.
|
||||
pub fn normalize_type_for_lookup(&self, t: &Type) -> Type {
|
||||
normalize_type_for_registry(t, &self.type_def_module)
|
||||
pub fn normalize_type_for_lookup(&self, caller_module: &str, t: &Type) -> Type {
|
||||
normalize_type_for_registry(caller_module, t, &self.type_def_module)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -534,7 +540,7 @@ fn build_registry(
|
||||
// Pass 1: collect "where is X defined" maps, plus a class lookup
|
||||
// by name (needed for the method-completeness check).
|
||||
let mut class_def_module: BTreeMap<String, String> = BTreeMap::new();
|
||||
let mut type_def_module: BTreeMap<String, String> = BTreeMap::new();
|
||||
let mut type_def_module: BTreeMap<(String, String), String> = BTreeMap::new();
|
||||
let mut class_by_name: BTreeMap<String, &ClassDef> = BTreeMap::new();
|
||||
for (mod_name, m) in modules {
|
||||
for def in &m.defs {
|
||||
@@ -544,7 +550,10 @@ fn build_registry(
|
||||
class_by_name.insert(c.name.clone(), c);
|
||||
}
|
||||
Def::Type(t) => {
|
||||
type_def_module.insert(t.name.clone(), mod_name.clone());
|
||||
type_def_module.insert(
|
||||
(mod_name.clone(), t.name.clone()),
|
||||
mod_name.clone(),
|
||||
);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
@@ -654,7 +663,7 @@ fn build_registry(
|
||||
.cloned()
|
||||
.unwrap_or_else(|| "<unknown-class>".into());
|
||||
let type_mod = type_def_module
|
||||
.get(&type_repr)
|
||||
.get(&(mod_name.clone(), type_repr.clone()))
|
||||
.cloned()
|
||||
.unwrap_or_else(|| "<primitive-or-unknown>".into());
|
||||
let coherent = mod_name == &class_mod || mod_name == &type_mod;
|
||||
@@ -676,7 +685,11 @@ fn build_registry(
|
||||
// declaration from elsewhere produce the same key (both
|
||||
// refer to the same type under the canonical-form rule).
|
||||
let type_hash = canonical::type_hash(
|
||||
&normalize_type_for_registry(&inst.type_, &type_def_module),
|
||||
&normalize_type_for_registry(
|
||||
mod_name,
|
||||
&inst.type_,
|
||||
&type_def_module,
|
||||
),
|
||||
);
|
||||
let key = (inst.class.clone(), type_hash);
|
||||
if let Some(prior) = entries.get(&key) {
|
||||
@@ -878,18 +891,26 @@ fn is_primitive_type_name(name: &str) -> bool {
|
||||
/// The qualifier is the type's *defining* module (looked up in
|
||||
/// `type_def_module`), not the instance's owning module. The two
|
||||
/// coincide under a canonical-form-compliant workspace (bare implies
|
||||
/// local-to-defining-module), but using the defining-module lookup is
|
||||
/// local-to-caller-module), but using the defining-module lookup is
|
||||
/// robust against pre-`validate_canonical_type_names`-wired fixtures
|
||||
/// that may still carry bare cross-module refs.
|
||||
///
|
||||
/// ctt.2: bare-name lookups are keyed by `(caller_module, name)`,
|
||||
/// not by `name` alone. A bare `Foo` written from module M resolves
|
||||
/// to M's `Foo` only; same-named types in other modules are
|
||||
/// distinct entries under their own caller-keyed tuple.
|
||||
fn normalize_type_for_registry(
|
||||
caller_module: &str,
|
||||
t: &Type,
|
||||
type_def_module: &BTreeMap<String, String>,
|
||||
type_def_module: &BTreeMap<(String, String), String>,
|
||||
) -> Type {
|
||||
match t {
|
||||
Type::Con { name, args } => {
|
||||
let new_name = if name.contains('.') || is_primitive_type_name(name) {
|
||||
name.clone()
|
||||
} else if let Some(owner) = type_def_module.get(name) {
|
||||
} else if let Some(owner) =
|
||||
type_def_module.get(&(caller_module.to_string(), name.clone()))
|
||||
{
|
||||
format!("{owner}.{name}")
|
||||
} else {
|
||||
// Unknown bare non-primitive — leave as-is. Either it is
|
||||
@@ -902,17 +923,17 @@ fn normalize_type_for_registry(
|
||||
name: new_name,
|
||||
args: args
|
||||
.iter()
|
||||
.map(|a| normalize_type_for_registry(a, type_def_module))
|
||||
.map(|a| normalize_type_for_registry(caller_module, a, type_def_module))
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
Type::Fn { params, param_modes, ret, ret_mode, effects } => Type::Fn {
|
||||
params: params
|
||||
.iter()
|
||||
.map(|p| normalize_type_for_registry(p, type_def_module))
|
||||
.map(|p| normalize_type_for_registry(caller_module, p, type_def_module))
|
||||
.collect(),
|
||||
param_modes: param_modes.clone(),
|
||||
ret: Box::new(normalize_type_for_registry(ret, type_def_module)),
|
||||
ret: Box::new(normalize_type_for_registry(caller_module, ret, type_def_module)),
|
||||
ret_mode: *ret_mode,
|
||||
effects: effects.clone(),
|
||||
},
|
||||
@@ -922,10 +943,10 @@ fn normalize_type_for_registry(
|
||||
.iter()
|
||||
.map(|c| crate::ast::Constraint {
|
||||
class: c.class.clone(),
|
||||
type_: normalize_type_for_registry(&c.type_, type_def_module),
|
||||
type_: normalize_type_for_registry(caller_module, &c.type_, type_def_module),
|
||||
})
|
||||
.collect(),
|
||||
body: Box::new(normalize_type_for_registry(body, type_def_module)),
|
||||
body: Box::new(normalize_type_for_registry(caller_module, body, type_def_module)),
|
||||
},
|
||||
Type::Var { name } => Type::Var { name: name.clone() },
|
||||
}
|
||||
@@ -2522,8 +2543,14 @@ mod tests {
|
||||
/// guards against at the outer level.
|
||||
#[test]
|
||||
fn ct1_5a_normalize_recurses_into_forall_constraints() {
|
||||
let mut type_def_module: BTreeMap<String, String> = BTreeMap::new();
|
||||
type_def_module.insert("MyInt".to_string(), "other".to_string());
|
||||
let mut type_def_module: BTreeMap<(String, String), String> = BTreeMap::new();
|
||||
// Caller module is "caller" for this test; the type `MyInt`
|
||||
// lives in `other`. Under the tuple key the bare-name lookup
|
||||
// resolves only when the caller is "caller".
|
||||
type_def_module.insert(
|
||||
("caller".to_string(), "MyInt".to_string()),
|
||||
"other".to_string(),
|
||||
);
|
||||
|
||||
// Forall a. (TShow MyInt) => a -> a
|
||||
// The constraint's type carries a bare `MyInt` that should be
|
||||
@@ -2546,7 +2573,7 @@ mod tests {
|
||||
}),
|
||||
};
|
||||
|
||||
let out = normalize_type_for_registry(&input, &type_def_module);
|
||||
let out = normalize_type_for_registry("caller", &input, &type_def_module);
|
||||
|
||||
match out {
|
||||
Type::Forall { constraints, .. } => {
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
//! Regression for the `Registry.type_def_module` re-key (ctt.2).
|
||||
//!
|
||||
//! Two modules each declare `type Foo` with distinct ctors and
|
||||
//! provide an `instance MyC Foo`. Pre-ctt.2, both bare `Foo`s
|
||||
//! collide on the bare-name `BTreeMap<String, String>` key —
|
||||
//! `normalize_type_for_registry` qualifies both to whichever
|
||||
//! module won the insert race, and the loser's instance trips
|
||||
//! a false `DuplicateInstance`. Post-ctt.2, the tuple-keyed map
|
||||
//! distinguishes the two `Foo`s by owning module; both instances
|
||||
//! register cleanly under distinct canonical keys.
|
||||
//!
|
||||
//! This test goes red today (DuplicateInstance) and green after
|
||||
//! the re-key.
|
||||
|
||||
use ailang_core::canonical;
|
||||
use ailang_core::ast::Type;
|
||||
use ailang_core::load_workspace;
|
||||
use std::path::Path;
|
||||
|
||||
fn examples_dir() -> std::path::PathBuf {
|
||||
let manifest = env!("CARGO_MANIFEST_DIR");
|
||||
Path::new(manifest)
|
||||
.parent().expect("CARGO_MANIFEST_DIR has a parent (crates/)")
|
||||
.parent().expect("crates has a parent (workspace root)")
|
||||
.join("examples")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn two_modules_with_same_bare_foo_both_register() {
|
||||
let entry = examples_dir().join("ctt2_collision_main.ail.json");
|
||||
let ws = load_workspace(&entry)
|
||||
.expect("expected workspace load to succeed; both `type Foo` declarations live in distinct modules and must register under distinct canonical keys");
|
||||
|
||||
// Compute the canonical type hashes for the two expected entries.
|
||||
let main_foo_hash = canonical::type_hash(&Type::Con {
|
||||
name: "ctt2_collision_main.Foo".into(),
|
||||
args: vec![],
|
||||
});
|
||||
let lib_foo_hash = canonical::type_hash(&Type::Con {
|
||||
name: "ctt2_collision_lib.Foo".into(),
|
||||
args: vec![],
|
||||
});
|
||||
|
||||
let main_key = ("MyC".to_string(), main_foo_hash);
|
||||
let lib_key = ("MyC".to_string(), lib_foo_hash);
|
||||
|
||||
assert!(
|
||||
ws.registry.entries.contains_key(&main_key),
|
||||
"registry missing entry for (MyC, ctt2_collision_main.Foo); entries: {:?}",
|
||||
ws.registry.entries.keys().collect::<Vec<_>>()
|
||||
);
|
||||
assert!(
|
||||
ws.registry.entries.contains_key(&lib_key),
|
||||
"registry missing entry for (MyC, ctt2_collision_lib.Foo); entries: {:?}",
|
||||
ws.registry.entries.keys().collect::<Vec<_>>()
|
||||
);
|
||||
|
||||
// Defining-module sanity: each entry's defining_module matches
|
||||
// the module that wrote the instance.
|
||||
let main_entry = &ws.registry.entries[&main_key];
|
||||
assert_eq!(
|
||||
main_entry.defining_module, "ctt2_collision_main",
|
||||
"main-side entry's defining_module mismatched"
|
||||
);
|
||||
let lib_entry = &ws.registry.entries[&lib_key];
|
||||
assert_eq!(
|
||||
lib_entry.defining_module, "ctt2_collision_lib",
|
||||
"lib-side entry's defining_module mismatched"
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user