From 931ff7e1de5dc54746424bda7a4cb27623b1481f Mon Sep 17 00:00:00 2001 From: Brian Hardock Date: Thu, 16 Apr 2026 15:20:26 -0600 Subject: [PATCH] Remove the invariant that interfaces must be sorted Signed-off-by: Brian Hardock --- .../tests/merge/topo-sort-needed/README.md | 14 -- .../tests/merge/topo-sort-needed/from/a.wit | 19 --- .../tests/merge/topo-sort-needed/into/a.wit | 7 - .../merge/topo-sort-needed/merge/bar.wit | 21 --- crates/wit-parser/src/resolve/mod.rs | 157 +----------------- 5 files changed, 1 insertion(+), 217 deletions(-) delete mode 100644 crates/wit-component/tests/merge/topo-sort-needed/README.md delete mode 100644 crates/wit-component/tests/merge/topo-sort-needed/from/a.wit delete mode 100644 crates/wit-component/tests/merge/topo-sort-needed/into/a.wit delete mode 100644 crates/wit-component/tests/merge/topo-sort-needed/merge/bar.wit diff --git a/crates/wit-component/tests/merge/topo-sort-needed/README.md b/crates/wit-component/tests/merge/topo-sort-needed/README.md deleted file mode 100644 index 660ef4123d..0000000000 --- a/crates/wit-component/tests/merge/topo-sort-needed/README.md +++ /dev/null @@ -1,14 +0,0 @@ -During a merge, when `from` contributes a **new** interface that doesn't exist in `into`, that interface gets appended to the **end** of the arena. If an existing interface in `into` now depends on that newly-appended interface (because extra types/functions referencing it were also added), the ordering invariant is violated — the dependency comes *after* its dependent. - -The `topo-sort-needed` test sets up exactly this scenario: - -- **`from`** has two interfaces: `dep` (defines `type t = u32`) and `user` (has `use dep.{t}`, shared function `get`, and extra function `get-dep` that returns `t`). -- **`into`** has only `user` with just the `get` function — no `dep` at all. - -When merging `from` into `into`: - -1. `user` is matched between both sides (same name, same package). The shared function `get` checks out structurally. -2. `dep` has no counterpart in `into`, so it's **appended to the end** of the arena — after `user`. -3. The extra function `get-dep` and the `use dep.{t}` type from `from`'s `user` are added to `into`'s `user`, creating a dependency from `user` → `dep`. - -Now the arena has `[user, dep]` but the dependency goes `user → dep` — `dep` should come first. Without the topological sort, `assert_topologically_sorted` panics with `assertion failed: other_interface_pos <= my_interface_pos`. With the sort, the arena is reordered to `[dep, user]` and the invariant holds. \ No newline at end of file diff --git a/crates/wit-component/tests/merge/topo-sort-needed/from/a.wit b/crates/wit-component/tests/merge/topo-sort-needed/from/a.wit deleted file mode 100644 index 46e5627824..0000000000 --- a/crates/wit-component/tests/merge/topo-sort-needed/from/a.wit +++ /dev/null @@ -1,19 +0,0 @@ -package foo:bar; - -/// `from` has `dep` and `user`. `user` has a function `get` shared with -/// `into`, and an extra function `get-dep` that references `dep.{t}`. -/// During the merge, `dep` is added to the arena after `user` (because -/// it's a brand-new interface). The extra `use dep.{t}` type in `user` -/// then creates a dependency user->dep where dep comes later in the -/// arena. The topological sort corrects this. - -interface dep { - type t = u32; -} - -interface user { - use dep.{t}; - - get: func() -> u32; - get-dep: func() -> t; -} diff --git a/crates/wit-component/tests/merge/topo-sort-needed/into/a.wit b/crates/wit-component/tests/merge/topo-sort-needed/into/a.wit deleted file mode 100644 index 35b9d0814b..0000000000 --- a/crates/wit-component/tests/merge/topo-sort-needed/into/a.wit +++ /dev/null @@ -1,7 +0,0 @@ -package foo:bar; - -/// `into` only has `user` with `get` — no `dep` interface at all. - -interface user { - get: func() -> u32; -} diff --git a/crates/wit-component/tests/merge/topo-sort-needed/merge/bar.wit b/crates/wit-component/tests/merge/topo-sort-needed/merge/bar.wit deleted file mode 100644 index 9f1309de76..0000000000 --- a/crates/wit-component/tests/merge/topo-sort-needed/merge/bar.wit +++ /dev/null @@ -1,21 +0,0 @@ -package foo:bar; - -/// `into` only has `user` with `get` — no `dep` interface at all. -interface user { - use dep.{t}; - - get: func() -> u32; - - get-dep: func() -> t; -} - -/// `from` has `dep` and `user`. `user` has a function `get` shared with -/// `into`, and an extra function `get-dep` that references `dep.{t}`. -/// During the merge, `dep` is added to the arena after `user` (because -/// it's a brand-new interface). The extra `use dep.{t}` type in `user` -/// then creates a dependency user->dep where dep comes later in the -/// arena. The topological sort corrects this. -interface dep { - type t = u32; -} - diff --git a/crates/wit-parser/src/resolve/mod.rs b/crates/wit-parser/src/resolve/mod.rs index 596839457e..00910eb81e 100644 --- a/crates/wit-parser/src/resolve/mod.rs +++ b/crates/wit-parser/src/resolve/mod.rs @@ -714,11 +714,6 @@ package {name} is defined in two different locations:\n\ log::trace!("now have {} packages", self.packages.len()); - // Ensure the interfaces arena is in topological order after the - // merge. Newly-added interfaces may have been appended after - // existing interfaces that now depend on them. - self.topologically_sort_interfaces(Some(&mut remap)); - // Re-elaborate all worlds after the merge. Merging may have added // extra types to existing interfaces that introduce new interface // dependencies not yet present in the world's imports. @@ -1598,149 +1593,6 @@ package {name} is defined in two different locations:\n\ pushed[id.index()] = true; } - /// Rebuilds the interfaces arena in topological order and updates all - /// InterfaceId references throughout this `Resolve`. - /// - /// After a merge, newly-added interfaces may appear after existing - /// interfaces that now depend on them. This method re-orders the - /// interfaces arena so that dependencies always precede dependents - /// within each package. - /// - /// The optional `remap` is updated so that its interface entries reflect - /// the new interface IDs. - fn topologically_sort_interfaces(&mut self, remap: Option<&mut Remap>) { - // Compute a global topological order: iterate packages in topological - // order, and within each package topologically sort interfaces based - // on their `use`-type dependencies. - let mut order = Vec::with_capacity(self.interfaces.len()); - let mut visited = vec![false; self.interfaces.len()]; - - for pkg_id in self.topological_packages() { - let pkg = &self.packages[pkg_id]; - for (_, &iface_id) in pkg.interfaces.iter() { - self.visit_interface_topo(iface_id, pkg_id, &mut visited, &mut order); - } - } - - // Also include interfaces that don't belong to any package (shouldn't - // normally happen, but be safe). - for (id, _) in self.interfaces.iter() { - if !visited[id.index()] { - order.push(id); - } - } - - // Check if already in order — if so, skip the rebuild. - let already_sorted = order - .iter() - .zip(order.iter().skip(1)) - .all(|(a, b)| a.index() < b.index()); - if already_sorted { - return; - } - - // Build old-to-new mapping. - let mut old_to_new: Vec> = vec![None; self.interfaces.len()]; - - // Consume the old arena and put items into a vec for random access. - let old_arena = mem::take(&mut self.interfaces); - let mut items: Vec> = old_arena - .into_iter() - .map(|(_, iface)| Some(iface)) - .collect(); - - // Rebuild the arena in topological order. - for &old_id in &order { - let iface = items[old_id.index()].take().unwrap(); - let new_id = self.interfaces.alloc(iface); - old_to_new[old_id.index()] = Some(new_id); - } - - // Helper closure to map an old InterfaceId to the new one. - let map_iface = |id: InterfaceId| -> InterfaceId { old_to_new[id.index()].unwrap() }; - - // Update all InterfaceId references throughout the Resolve. - - // 1. Package::interfaces - for (_, pkg) in self.packages.iter_mut() { - for (_, id) in pkg.interfaces.iter_mut() { - *id = map_iface(*id); - } - } - - // 2. Types: TypeOwner::Interface - for (_, ty) in self.types.iter_mut() { - if let TypeOwner::Interface(id) = &mut ty.owner { - *id = map_iface(*id); - } - } - - // 3. Worlds: imports/exports with WorldKey::Interface and WorldItem::Interface - for (_, world) in self.worlds.iter_mut() { - let imports = mem::take(&mut world.imports); - for (key, mut item) in imports { - let new_key = match key { - WorldKey::Interface(id) => WorldKey::Interface(map_iface(id)), - other => other, - }; - if let WorldItem::Interface { id, .. } = &mut item { - *id = map_iface(*id); - } - world.imports.insert(new_key, item); - } - let exports = mem::take(&mut world.exports); - for (key, mut item) in exports { - let new_key = match key { - WorldKey::Interface(id) => WorldKey::Interface(map_iface(id)), - other => other, - }; - if let WorldItem::Interface { id, .. } = &mut item { - *id = map_iface(*id); - } - world.exports.insert(new_key, item); - } - } - - // 4. Interface::clone_of - for (_, iface) in self.interfaces.iter_mut() { - if let Some(id) = &mut iface.clone_of { - *id = map_iface(*id); - } - } - - // 5. Update the Remap if provided. - if let Some(remap) = remap { - for entry in remap.interfaces.iter_mut() { - if let Some(id) = entry { - *id = map_iface(*id); - } - } - } - } - - /// Depth-first visit for topological sorting of interfaces within a package. - fn visit_interface_topo( - &self, - id: InterfaceId, - pkg_id: PackageId, - visited: &mut Vec, - order: &mut Vec, - ) { - if visited[id.index()] { - return; - } - visited[id.index()] = true; - - // Visit same-package dependencies first. - for dep in self.interface_direct_deps(id) { - if self.interfaces[dep].package == Some(pkg_id) { - self.visit_interface_topo(dep, pkg_id, visited, order); - } - } - - order.push(id); - } - #[doc(hidden)] pub fn assert_valid(&self) { let mut package_interfaces = Vec::new(); @@ -1906,14 +1758,7 @@ package {name} is defined in two different locations:\n\ let my_package_pos = positions.get_index_of(&my_package).unwrap(); let other_package_pos = positions.get_index_of(&other_package).unwrap(); - if my_package_pos == other_package_pos { - let interfaces = &positions[&my_package]; - let my_interface_pos = interfaces.get_index_of(&my_interface).unwrap(); - let other_interface_pos = interfaces.get_index_of(&other_interface).unwrap(); - assert!(other_interface_pos <= my_interface_pos); - } else { - assert!(other_package_pos < my_package_pos); - } + assert!(other_package_pos <= my_package_pos); } }