Skip to content

Commit

Permalink
Fix topological sort of types
Browse files Browse the repository at this point in the history
Structurally recursive types were not considered. For example, if `enum
Foo` had a variant with a field of type `Vec<Bar>`, `Foo` was not
considered to depend on `Bar` even though it needs to so that
definitions are in the right order in the generated C++ header.

Resolves #43
  • Loading branch information
stefunctional committed Aug 28, 2024
1 parent 1525db3 commit a2f9e95
Showing 1 changed file with 38 additions and 55 deletions.
93 changes: 38 additions & 55 deletions bindgen/src/bindings/cpp/gen_cpp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::{
use anyhow::{Context, Result};
use askama::Template;
use serde::{Deserialize, Serialize};
use topological_sort::TopologicalSort;
use topological_sort::{DependencyLink, TopologicalSort};
use uniffi_bindgen::{
backend::TemplateExpression,
interface::{AsType, FfiType, Type, UniffiTrait},
Expand Down Expand Up @@ -150,61 +150,33 @@ impl<'a> CppWrapperHeader<'a> {
&self,
types: impl Iterator<Item = &'a Type>,
) -> impl Iterator<Item = Type> {
let mut definition_topology = TopologicalSort::<String>::new();

// We take into account only the Record and Enum types, as they are the
// only types that can have member variables that reference other structures
for type_ in self.ci.iter_types() {
match type_ {
Type::Record { name, .. } => {
if let Some(record) = self.ci.get_record_definition(name) {
for field in record.iter_types() {
match field.as_type() {
Type::Record {
name: field_name, ..
}
| Type::Enum {
name: field_name, ..
}
| Type::Object {
name: field_name, ..
}
| Type::Custom {
name: field_name, ..
} => {
definition_topology.add_dependency(field_name, name);
}
_ => {}
}
}
}
}
Type::Enum { name, .. } => {
if let Some(enum_) = self.ci.get_enum_definition(name) {
enum_.variants().iter().for_each(|v| {
v.fields().iter().for_each(|f| match f.as_type() {
Type::Record {
name: field_name, ..
}
| Type::Enum {
name: field_name, ..
}
| Type::Object {
name: field_name, ..
}
| Type::Custom {
name: field_name, ..
} => {
definition_topology.add_dependency(field_name, name);
}
_ => {}
});
});
}
let mut definition_topology = self
.ci
.iter_types()
.filter_map(|type_| {
// We take into account only the Record and Enum types, as they are the
// only types that can have member variables that reference other structures
match type_ {
Type::Record { name, .. } => self
.ci
.get_record_definition(name)
.map(|record| (name, record.iter_types())),
Type::Enum { name, .. } => self
.ci
.get_enum_definition(name)
.map(|enum_| (name, enum_.iter_types())),
_ => None,
}
_ => {}
}
}
})
.flat_map(|(name, types)| {
types
.filter_map(type_name)
.map(|field_name| DependencyLink {
prec: field_name,
succ: name,
})
})
.collect::<TopologicalSort<_>>();

let mut sorted: Vec<Type> = Vec::new();
while !definition_topology.peek_all().is_empty() {
Expand Down Expand Up @@ -236,6 +208,17 @@ impl<'a> CppWrapperHeader<'a> {
}
}

fn type_name(ty: &Type) -> Option<&str> {
match ty {
Type::Record { name, .. }
| Type::Object { name, .. }
| Type::Enum { name, .. }
| Type::External { name, .. }
| Type::Custom { name, .. } => Some(name),
_ => None,
}
}

#[allow(dead_code)]
#[derive(Template)]
#[template(syntax = "cpp", escape = "none", path = "wrapper.cpp")]
Expand Down

0 comments on commit a2f9e95

Please sign in to comment.