UniFFI Dart Binding Generator Refactor and Added Async Support #37
Conversation
Co-authored-by: Benjamin Kampmann <[email protected]>
Co-authored-by: Benjamin Kampmann <[email protected]>
Co-authored-by: Benjamin Kampmann <[email protected]>
src/gen/enums.rs
Outdated
| final $(var_name(f.name())) = $(var_name(f.name()))_lifted.value; | ||
| new_offset += $(var_name(f.name()))_lifted.bytesRead; | ||
| ) | ||
| return LiftRetVal($(class_name(obj.name()))$cls_name._( | ||
| $(for f in obj.fields() => $(var_name(f.name())),) | ||
| ), new_offset); | ||
| ), new_offset - buf.offsetInBytes); |
There was a problem hiding this comment.
this seems incorrect, why are you substracting the original offset?
There was a problem hiding this comment.
I'll revert this, interestingly it didn't trigger an error in the tests for both chases
There was a problem hiding this comment.
is this just a move? if so, where is the code of the original being removed?
There was a problem hiding this comment.
this branch is a backup of what we had before I made any "destructive" changes that would make it harder to track the history of changes
src/gen/primitives/mod.rs
Outdated
There was a problem hiding this comment.
same here... is that actually new or just a move? If so, where is the diff to remove the original?
There was a problem hiding this comment.
Yeah, most of the code would consist of moves from yours and my history, with changes whenever approaches need to be combined or the better one is chosen and refactors are required
| @@ -64,7 +64,7 @@ pub fn generate_enum(obj: &Enum, type_helper: &dyn TypeHelperRenderer) -> dart:: | |||
| } | |||
|
|
|||
| class $ffi_converter_name { | |||
| static $cls_name lift(RustBuffer buffer) { | |||
| static $cls_name lift( RustBuffer buffer) { | |||
There was a problem hiding this comment.
the diff of this file contains mainly spaces after ( ... why?
src/gen/functions.rs
Outdated
|
|
||
| use crate::gen::oracle::DartCodeOracle; | ||
| use crate::gen::render::AsRenderable; | ||
|
|
||
| use super::oracle::AsCodeType; | ||
| use super::render::TypeHelperRenderer; | ||
|
|
||
| // #[allow(unused_variables)] |
src/gen/mod.rs
Outdated
| @@ -30,6 +31,25 @@ pub struct Config { | |||
| external_packages: HashMap<String, String>, | |||
| } | |||
|
|
|||
| // impl MergeWith for Config { | |||
There was a problem hiding this comment.
and this? why re-add this?
src/gen/objects.rs
Outdated
| pub fn generate_method(func: &Method, type_helper: &dyn TypeHelperRenderer) -> dart::Tokens { | ||
| #[allow(unused_variables)] | ||
| pub fn generate_method(func: &Method, type_helper: &dyn TypeHelperRenderer) -> dart::Tokens { | ||
| // let api = "_api"; |
src/gen/types.rs
Outdated
| @@ -166,6 +168,268 @@ impl Renderer<(FunctionDefinition, dart::Tokens)> for TypeHelpersRenderer<'_> { | |||
| import "package:ffi/ffi.dart"; | |||
| $(imports) | |||
|
|
|||
| // class UniffiInternalError implements Exception { | |||
There was a problem hiding this comment.
I don't get it. what are all these about?
| @@ -136,14 +137,19 @@ impl TypeHelperRenderer for TypeHelpersRenderer<'_> { | |||
| } | |||
|
|
|||
| impl Renderer<(FunctionDefinition, dart::Tokens)> for TypeHelpersRenderer<'_> { | |||
| // TODO: Implimient a two pass system where the first pass will render the main code, and the second pass will render the helper code | |||
There was a problem hiding this comment.
please do not put FIXME or TODOs in the code. They belong as issues in the issue tracker.
src/gen/oracle.rs
Outdated
| @@ -270,7 +270,23 @@ impl<T: AsType> AsCodeType for T { | |||
| Type::Record { name, module_path } => { | |||
| Box::new(records::RecordCodeType::new(name, module_path)) | |||
| } | |||
| _ => todo!("As Type for Type::{:?}", self.as_type()), | |||
| _ => todo!("As Type for Type::{:?}", self.as_type()), // Type::Bytes => Box::new(primitives::BytesCodeType), | |||
There was a problem hiding this comment.
what are these comments about?
|
I've reviewed only the newly added commits compared to what I have already seen, but I don't quite understand what is going on there. There is a lot of commented code added for an known reason but nothing really seem of material value ..? |
|
It has become impossible to follow this. Let's fix the CI, merge this and handle the fallout from there. |
chavic
changed the title
okay, this is mostly due to the way I handled merging the two branches... but there's a backup of the code before I did that in this branch |
Here's a summary overview of what's in this PR, some things were skipped because the codebase was in a satisfactory state, and it may not include other possible improvements (undone)... This should be enough to continue adding features
async_poll,async_complete,async_free)external_type_package_namemethodDurationCodeTypeLiftRetValstruct for more precise liftingUniffiInternalErrorTypeHelpersRenderer, keeping useful methods from both codebases