From 52008804a9c8aca8342ae8fe2a4b2558a564d86b Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Wed, 27 May 2026 13:42:38 -0600 Subject: [PATCH 1/3] C++: sort types topologically before emitting definitions Specifically, we sort the types while taking the parameter and return types of resource functions into consideration. This ensures that types are declared (and, if necessary, defined) before they are used in the emitted source code. Fixes #1615 --- Cargo.lock | 1 + crates/cpp/Cargo.toml | 1 + crates/cpp/src/lib.rs | 134 +++++++++++++++++- crates/test/src/cpp.rs | 2 +- tests/codegen/issue1615-declaration-order.wit | 40 ++++++ 5 files changed, 174 insertions(+), 4 deletions(-) create mode 100644 tests/codegen/issue1615-declaration-order.wit diff --git a/Cargo.lock b/Cargo.lock index 4d5ad6b65..4934374c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1428,6 +1428,7 @@ dependencies = [ "anyhow", "clap", "heck", + "indexmap", "test-helpers", "wasm-encoder 0.249.0", "wasm-metadata 0.249.0", diff --git a/crates/cpp/Cargo.toml b/crates/cpp/Cargo.toml index 04a7603f9..7b6b96a26 100644 --- a/crates/cpp/Cargo.toml +++ b/crates/cpp/Cargo.toml @@ -26,6 +26,7 @@ wit-bindgen-c = { workspace = true } anyhow = { workspace = true } heck = { workspace = true } clap = { workspace = true, optional = true } +indexmap = { workspace = true } [dev-dependencies] test-helpers = { path = '../test-helpers' } diff --git a/crates/cpp/src/lib.rs b/crates/cpp/src/lib.rs index 0348ff7f9..473fd88eb 100644 --- a/crates/cpp/src/lib.rs +++ b/crates/cpp/src/lib.rs @@ -1,5 +1,6 @@ use anyhow::bail; use heck::{ToPascalCase, ToShoutySnakeCase, ToSnakeCase, ToUpperCamelCase}; +use indexmap::{IndexMap, IndexSet}; use std::{ collections::{HashMap, HashSet}, fmt::{self, Display, Write as FmtWrite}, @@ -891,9 +892,13 @@ impl CppInterfaceGenerator<'_> { } } - // Second pass: emit full type definitions - for (name, id) in iface_data.types.iter() { - self.define_type(name, *id); + // Second pass: emit full type definitions for all types. + // + // Here we sort the types topologically, taking into consideration the + // parameter and return types of functions associated with resource + // types. This ensures each type is declared before any uses of it. + for (name, id) in sort_types(self.resolve, &iface_data.types) { + self.define_type(name, id); } } @@ -3629,3 +3634,126 @@ fn is_special_method(func: &Function) -> SpecialMethod { SpecialMethod::None } } + +/// Sort the specified types topologically, taking the parameter and result +/// types of resource functions into consideration. +fn sort_types<'a>( + resolve: &Resolve, + types: &'a IndexMap, +) -> IndexMap<&'a str, TypeId> { + let mut sorted = IndexSet::new(); + let mut visited = HashSet::new(); + for id in types.values() { + sort(resolve, Type::Id(*id), &mut sorted, &mut visited); + } + let names = types + .iter() + .map(|(k, v)| (*v, k.as_str())) + .collect::>(); + sorted + .into_iter() + .filter_map(|v| names.get(&v).map(|k| (*k, v))) + .collect() +} + +fn sort(resolve: &Resolve, ty: Type, sorted: &mut IndexSet, visited: &mut HashSet) { + match ty { + Type::Bool + | Type::U8 + | Type::U16 + | Type::U32 + | Type::S8 + | Type::S16 + | Type::S32 + | Type::Char + | Type::U64 + | Type::S64 + | Type::F32 + | Type::F64 + | Type::String + | Type::ErrorContext => (), + Type::Id(id) => { + let ty = &resolve.types[id]; + match &ty.kind { + TypeDefKind::Record(record) => { + for field in &record.fields { + sort(resolve, field.ty, sorted, visited); + } + sorted.insert(id); + } + TypeDefKind::Variant(variant) => { + for case in &variant.cases { + if let Some(ty) = case.ty { + sort(resolve, ty, sorted, visited); + } + } + sorted.insert(id); + } + TypeDefKind::Enum(_) | TypeDefKind::Flags(_) => { + sorted.insert(id); + } + TypeDefKind::Handle(Handle::Borrow(resource) | Handle::Own(resource)) => { + sort(resolve, Type::Id(*resource), sorted, visited); + sorted.insert(id); + } + TypeDefKind::Option(some) => { + sort(resolve, *some, sorted, visited); + sorted.insert(id); + } + TypeDefKind::Result(result) => { + if let Some(ty) = result.ok { + sort(resolve, ty, sorted, visited); + } + if let Some(ty) = result.err { + sort(resolve, ty, sorted, visited); + } + sorted.insert(id); + } + TypeDefKind::Tuple(tuple) => { + for ty in &tuple.types { + sort(resolve, *ty, sorted, visited); + } + sorted.insert(id); + } + TypeDefKind::List(ty) | TypeDefKind::FixedLengthList(ty, _) => { + sort(resolve, *ty, sorted, visited); + } + TypeDefKind::Type(ty) => { + sort(resolve, *ty, sorted, visited); + } + TypeDefKind::Resource => { + // Here we avoid infinite recursion by remembering if we've + // seen this type already. + if !visited.contains(&id) { + visited.insert(id); + + if let TypeOwner::Interface(interface) = ty.owner { + for function in resolve.interfaces[interface].functions.values() { + if let Some(resource) = function.kind.resource() + && resource == id + { + for parameter in &function.params { + sort(resolve, parameter.ty, &mut *sorted, &mut *visited); + } + + if let Some(ty) = function.result { + sort(resolve, ty, &mut *sorted, &mut *visited); + } + } + } + } + + sorted.insert(id); + } + } + TypeDefKind::Stream(ty) | TypeDefKind::Future(ty) => { + if let Some(ty) = ty { + sort(resolve, *ty, sorted, visited); + } + sorted.insert(id); + } + kind => todo!("{kind:?}"), + } + } + } +} diff --git a/crates/test/src/cpp.rs b/crates/test/src/cpp.rs index 4946a1fef..293da6a3b 100644 --- a/crates/test/src/cpp.rs +++ b/crates/test/src/cpp.rs @@ -50,7 +50,7 @@ impl LanguageMethods for Cpp { return false; } return match name { - "issue1514-6.wit" | "named-fixed-length-list.wit" | "map.wit" => true, + "issue1514-6.wit" | "map.wit" => true, _ => false, } || config.async_; } diff --git a/tests/codegen/issue1615-declaration-order.wit b/tests/codegen/issue1615-declaration-order.wit new file mode 100644 index 000000000..84a6aec06 --- /dev/null +++ b/tests/codegen/issue1615-declaration-order.wit @@ -0,0 +1,40 @@ +package test:test; + +interface sqlite { + resource connection { + open: static func(database: string) -> result; + execute: func(statement: string, parameters: list) -> result; + last-insert-rowid: func() -> s64; + changes: func() -> u64; + } + + variant error { + no-such-database, + access-denied, + invalid-connection, + database-full, + io(string) + } + + record query-result { + columns: list, + rows: list, + } + + record row-result { + values: list + } + + variant value { + integer(s64), + real(f64), + text(string), + blob(list), + null + } +} + +world test { + import sqlite; + export foo: func(); +} From 27429079ac04b247554fe22e9b783a9187fe1521 Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Wed, 27 May 2026 14:48:56 -0600 Subject: [PATCH 2/3] remove redundant resource type forward declarations --- crates/cpp/src/lib.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/crates/cpp/src/lib.rs b/crates/cpp/src/lib.rs index 473fd88eb..eabcfac05 100644 --- a/crates/cpp/src/lib.rs +++ b/crates/cpp/src/lib.rs @@ -879,19 +879,6 @@ impl CppInterfaceGenerator<'_> { fn types(&mut self, iface: InterfaceId) { let iface_data = &self.resolve().interfaces[iface]; - // First pass: emit forward declarations for all resources - // This ensures resources can reference each other in method signatures - for (name, id) in iface_data.types.iter() { - let ty = &self.resolve().types[*id]; - if matches!(&ty.kind, TypeDefKind::Resource) { - let pascal = name.to_upper_camel_case(); - let guest_import = self.r#gen.imported_interfaces.contains(&iface); - let namespc = namespace(self.resolve, &ty.owner, !guest_import, &self.r#gen.opts); - self.r#gen.h_src.change_namespace(&namespc); - uwriteln!(self.r#gen.h_src.src, "class {pascal};"); - } - } - // Second pass: emit full type definitions for all types. // // Here we sort the types topologically, taking into consideration the From a3d1fbf1ca65a90218f3dea5e4517705d24cfcfd Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Wed, 27 May 2026 15:18:39 -0600 Subject: [PATCH 3/3] use `wit_parser::TypeIdVisitor` to simplify `sort_types` --- Cargo.lock | 1 + crates/cpp/Cargo.toml | 1 + crates/cpp/src/lib.rs | 173 +++++++++++++++-------------------------- crates/test/src/cpp.rs | 2 +- 4 files changed, 66 insertions(+), 111 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4934374c6..7b062d31a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1435,6 +1435,7 @@ dependencies = [ "wit-bindgen-c", "wit-bindgen-core", "wit-component", + "wit-parser", ] [[package]] diff --git a/crates/cpp/Cargo.toml b/crates/cpp/Cargo.toml index 7b6b96a26..d74076ba2 100644 --- a/crates/cpp/Cargo.toml +++ b/crates/cpp/Cargo.toml @@ -27,6 +27,7 @@ anyhow = { workspace = true } heck = { workspace = true } clap = { workspace = true, optional = true } indexmap = { workspace = true } +wit-parser = { workspace = true } [dev-dependencies] test-helpers = { path = '../test-helpers' } diff --git a/crates/cpp/src/lib.rs b/crates/cpp/src/lib.rs index eabcfac05..82bcf0764 100644 --- a/crates/cpp/src/lib.rs +++ b/crates/cpp/src/lib.rs @@ -21,6 +21,7 @@ use wit_bindgen_core::{ WorldKey, }, }; +use wit_parser::TypeIdVisitor; // mod wamr; mod symbol_name; @@ -879,11 +880,10 @@ impl CppInterfaceGenerator<'_> { fn types(&mut self, iface: InterfaceId) { let iface_data = &self.resolve().interfaces[iface]; - // Second pass: emit full type definitions for all types. - // - // Here we sort the types topologically, taking into consideration the - // parameter and return types of functions associated with resource - // types. This ensures each type is declared before any uses of it. + // Here we sort the types topologically before emitting code for them, + // taking into consideration the parameter and return types of functions + // associated with resource types. This ensures each type is declared + // before any uses of it. for (name, id) in sort_types(self.resolve, &iface_data.types) { self.define_type(name, id); } @@ -3628,119 +3628,72 @@ fn sort_types<'a>( resolve: &Resolve, types: &'a IndexMap, ) -> IndexMap<&'a str, TypeId> { - let mut sorted = IndexSet::new(); - let mut visited = HashSet::new(); - for id in types.values() { - sort(resolve, Type::Id(*id), &mut sorted, &mut visited); + struct Visitor<'a> { + resolve: &'a Resolve, + sorted: IndexSet, + visited: HashSet, } - let names = types - .iter() - .map(|(k, v)| (*v, k.as_str())) - .collect::>(); - sorted - .into_iter() - .filter_map(|v| names.get(&v).map(|k| (*k, v))) - .collect() -} -fn sort(resolve: &Resolve, ty: Type, sorted: &mut IndexSet, visited: &mut HashSet) { - match ty { - Type::Bool - | Type::U8 - | Type::U16 - | Type::U32 - | Type::S8 - | Type::S16 - | Type::S32 - | Type::Char - | Type::U64 - | Type::S64 - | Type::F32 - | Type::F64 - | Type::String - | Type::ErrorContext => (), - Type::Id(id) => { - let ty = &resolve.types[id]; - match &ty.kind { - TypeDefKind::Record(record) => { - for field in &record.fields { - sort(resolve, field.ty, sorted, visited); - } - sorted.insert(id); - } - TypeDefKind::Variant(variant) => { - for case in &variant.cases { - if let Some(ty) = case.ty { - sort(resolve, ty, sorted, visited); - } - } - sorted.insert(id); - } - TypeDefKind::Enum(_) | TypeDefKind::Flags(_) => { - sorted.insert(id); - } - TypeDefKind::Handle(Handle::Borrow(resource) | Handle::Own(resource)) => { - sort(resolve, Type::Id(*resource), sorted, visited); - sorted.insert(id); - } - TypeDefKind::Option(some) => { - sort(resolve, *some, sorted, visited); - sorted.insert(id); - } - TypeDefKind::Result(result) => { - if let Some(ty) = result.ok { - sort(resolve, ty, sorted, visited); - } - if let Some(ty) = result.err { - sort(resolve, ty, sorted, visited); - } - sorted.insert(id); - } - TypeDefKind::Tuple(tuple) => { - for ty in &tuple.types { - sort(resolve, *ty, sorted, visited); - } - sorted.insert(id); - } - TypeDefKind::List(ty) | TypeDefKind::FixedLengthList(ty, _) => { - sort(resolve, *ty, sorted, visited); - } - TypeDefKind::Type(ty) => { - sort(resolve, *ty, sorted, visited); - } - TypeDefKind::Resource => { - // Here we avoid infinite recursion by remembering if we've - // seen this type already. - if !visited.contains(&id) { - visited.insert(id); - - if let TypeOwner::Interface(interface) = ty.owner { - for function in resolve.interfaces[interface].functions.values() { - if let Some(resource) = function.kind.resource() - && resource == id - { - for parameter in &function.params { - sort(resolve, parameter.ty, &mut *sorted, &mut *visited); - } - - if let Some(ty) = function.result { - sort(resolve, ty, &mut *sorted, &mut *visited); - } + impl TypeIdVisitor for Visitor<'_> { + fn before_visit_type_id(&mut self, id: TypeId) -> bool { + let ty = &self.resolve.types[id]; + if let TypeDefKind::Resource = &ty.kind { + // Here we avoid infinite recursion by remembering if we've seen + // this type already. + if self.visited.contains(&id) { + false + } else { + self.visited.insert(id); + + // TypeIdVisitor does not consider the parameter and return + // types of resource associated functions to be transitive + // types, so we need to handle that ourselves here. + if let TypeOwner::Interface(interface) = ty.owner { + for function in self.resolve.interfaces[interface].functions.values() { + if let Some(resource) = function.kind.resource() + && resource == id + { + for parameter in &function.params { + self.visit_type(self.resolve, ¶meter.ty); + } + + if let Some(ty) = function.result { + self.visit_type(self.resolve, &ty); } } } - - sorted.insert(id); } + + true } - TypeDefKind::Stream(ty) | TypeDefKind::Future(ty) => { - if let Some(ty) = ty { - sort(resolve, *ty, sorted, visited); - } - sorted.insert(id); - } - kind => todo!("{kind:?}"), + } else { + true } } + + fn after_visit_type_id(&mut self, id: TypeId) { + self.sorted.insert(id); + } } + + let mut visitor = Visitor { + resolve, + sorted: Default::default(), + visited: Default::default(), + }; + + for &id in types.values() { + visitor.visit_type_id(resolve, id); + } + + let names = types + .iter() + .map(|(k, v)| (*v, k.as_str())) + .collect::>(); + + visitor + .sorted + .into_iter() + .filter_map(|v| names.get(&v).map(|k| (*k, v))) + .collect() } diff --git a/crates/test/src/cpp.rs b/crates/test/src/cpp.rs index 293da6a3b..4946a1fef 100644 --- a/crates/test/src/cpp.rs +++ b/crates/test/src/cpp.rs @@ -50,7 +50,7 @@ impl LanguageMethods for Cpp { return false; } return match name { - "issue1514-6.wit" | "map.wit" => true, + "issue1514-6.wit" | "named-fixed-length-list.wit" | "map.wit" => true, _ => false, } || config.async_; }