use wit_parser::TypeIdVisitor to simplify sort_types

Author: dicej

Cargo.lock

@@ -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' }

crates/cpp/Cargo.toml

@@ -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<String, TypeId>,
 ) -> 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<TypeId>,
+        visited: HashSet<TypeId>,
     }
-    let names = types
-        .iter()
-        .map(|(k, v)| (*v, k.as_str()))
-        .collect::<HashMap<_, _>>();
-    sorted
-        .into_iter()
-        .filter_map(|v| names.get(&v).map(|k| (*k, v)))
-        .collect()
-}
 
-fn sort(resolve: &Resolve, ty: Type, sorted: &mut IndexSet<TypeId>, visited: &mut HashSet<TypeId>) {
-    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, &parameter.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::<HashMap<_, _>>();
+
+    visitor
+        .sorted
+        .into_iter()
+        .filter_map(|v| names.get(&v).map(|k| (*k, v)))
+        .collect()
 }

crates/cpp/src/lib.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_;
     }