Open bendk opened 2 months ago
This pass would convert ComponentInterface and Config into something simpler from the POV of the template code.
The ComponentInterface
exists only for the template code, so can we make it more fit-for-purpose rather than attempting to re-describe it in a similar-but-different way for each binding?
Most of these complications are all about renaming things - both Config
and the Oracle
doing slightly different but the same things - it's all about renaming things to match local requirements.
The actual problem I see is more about how we do all these transformations "on the fly" while working within askama - so I agree entirely that a kind of pre-transform pass before rendering makes a lot of sense - but can't we just transform the CI itself? That has all the up-sides and less downsides?
The actual problem I see is more about how we do all these transformations "on the fly" while working within askama - so I agree entirely that a kind of pre-transform pass before rendering makes a lot of sense - but can't we just transform the CI itself? That has all the up-sides and less downsides?
Great question. My feeling is that we're going to want to end up with different structs, similar to how we started with a single CodeType and CodeOracle trait but ended up with separate traits for each platform. The reason is that each platform has slightly different requirements and one interface is not going to work, for example Swift needs to know things like should we omit argument labels that make no sense for other languages. I also wonder if new languages might need even bigger changes. If we added C, it would probably have some concept of "FFIConverter", but it's not going to be a class, maybe it'll be a set of functions with a shared prefix.
That said, we could start by transforming the CI itself and seeing how far we can push that. This seems a great way to handle name transformations for example. If we reached the point where we decided we wanted to start using language-specific structs, I don't think this work would be wasted -- that code should be pretty easy to port over.
After working on https://github.com/mozilla/uniffi-rs/pull/2191, and trying to create a Python struct which holds all the modifications for the templates: It's not that straight forward.
Currently, the ComponentInterface
holds a lot of functions (LOC ~1k). If we change from a general ComponentInterface
to a {Language}Interface
, each of these interfaces have to basically implement 1:1 all the same functions as the ComponentInterface
. We are calling them inside each template. They are very generic like "get me all the FFI function definitions".
For this, one idea I have is change the ComponentInterface
to be a trait, and have default implementations. So instead of:
impl ComponentInterface {
pub fn new(crate_name: &str) -> Self {
assert!(!crate_name.is_empty());
Self {
types: TypeUniverse::new(NamespaceMetadata {
crate_name: crate_name.to_string(),
..Default::default()
}),
..Default::default()
}
}
/// Parse a `ComponentInterface` from a string containing a WebIDL definition.
pub fn from_webidl(idl: &str, module_path: &str) -> Result<Self> {
ensure!(
!module_path.is_empty(),
"you must specify a valid crate name"
);
let group = uniffi_udl::parse_udl(idl, module_path)?;
Self::from_metadata(group)
}
...
We do
pub trait LanguageComponentInterface {
fn new(crate_name: &str) -> Self
where
Self: Sized,
{
assert!(!crate_name.is_empty());
Self {
types: TypeUniverse::new(NamespaceMetadata {
crate_name: crate_name.to_string(),
..Default::default()
}),
..Default::default()
}
}
/// Parse a `ComponentInterface` from a string containing a WebIDL definition.
fn from_webidl(idl: &str, module_path: &str) -> Result<Self>
where
Self: Sized,
{
ensure!(
!module_path.is_empty(),
"you must specify a valid crate name"
);
let group = uniffi_udl::parse_udl(idl, module_path)?;
Self::from_metadata(group)
}
And then have a macro which implements this trait on each of the {Language}Interface
structs.
Example macro:
macro_rules! impl_language_component_interface {
($struct_name:ident) => {
impl LanguageComponentInterface for $struct_name {
fn new(crate_name: &str) -> Self {
assert!(!crate_name.is_empty());
Self {
types: TypeUniverse::new(NamespaceMetadata {
crate_name: crate_name.to_string(),
..Default::default()
}),
enums: BTreeMap::new(),
records: BTreeMap::new(),
functions: Vec::new(),
objects: Vec::new(),
callback_interfaces: Vec::new(),
errors: HashSet::new(),
callback_interface_throws_types: BTreeSet::new(),
}
}
fn get_types(&self) -> &TypeUniverse {
&self.types
}
fn get_types_mut(&mut self) -> &mut TypeUniverse {
&mut self.types
}
}
};
}
Which leads to something like that:
pub struct PythonComponentInterface {
types: TypeUniverse,
enums: BTreeMap<String, Enum>,
records: BTreeMap<String, Record>,
functions: Vec<Function>,
objects: Vec<Object>,
callback_interfaces: Vec<CallbackInterface>,
errors: HashSet<String>,
callback_interface_throws_types: BTreeSet<Type>,
}
impl_language_component_interface!(PythonComponentInterface);
Of course, the question is: What if the templating for reach language changes drastically? I am not sure about that. But currently we are very tightly coupled. Breaking this up leads to a lot of "not finished" code pieces. I think moving the impl ComponentInterface
into a trait, and implementing this trait for each {Language}Interface
could be the very first step. It's basically renaming and adjusting the types throughout the codebase.
The second step could be this PR which moves the filters out of the template and adjusts the {Language}Interface
directly.
UPDATE: I started a branch where I implemented a POC for this suggested structure. This draft holds all the changes: https://github.com/mozilla/uniffi-rs/pull/2198
I think it would simplify and improve the code to add pass before constructing the template object that we use to render the foreign bindings.
This pass would convert
ComponentInterface
andConfig
into something simpler from the POV of the template code. The template struct would then contain these as fields. For example, something like this:The template code could then render this more-or-less directly, without filters or extra processing (
fun {{ function.name }}
rather thanfun {{ function.name|fn_name }}
).I think there would be quite a few benefits of doing this:
Config
andComponentInterface
when we want to render a type name. In #2176, Bastian is adding the ability to rename things and this is going to require passing theConfig
in more places.