Skip to content

Commit

Permalink
rid-macro: adding export return types (#13)
Browse files Browse the repository at this point in the history
This adds support for more return types for function exports via `#[rid::export]`.
It contains some WIP for HashMap support as well. While working on that I realized that in order to send the `keys()` as a `Vec` it needs to be supported first.

I added lots of tests which verify that the following export return types are now supported:

```rs
Vec<&struct>
Vec<&primitive>
Vec<&String>
Vec<&CString>
Vec<&str>
```

In the case of vecs with a _string-like_ type the string value is not resolved until the item in the Vec is accessed in order to push out copying the data to convert to a  `*const char` until the last possible moment.

```rs
// owned
u8, i8, u16, i16 .. u64, i64
bool

// refererences
&u8, &i8, &u16, &i16 .. &u64, &i64
&bool
```

```rs
String, CString
&String, &CString, &str
```

They are sent as `*const char` to Dart.

Enums are converted to `i8` when sent to Dart and serialized back into an enum on the Dart end.

* rid-macro: hash map prepared

- composite supports two inner types
- hash map match branches added with todos
- compiles and all tests pass

* rid-macro: hash map rendering without access methods

* rid-macro: organizing vec dart render in separate dir

* rid-macro: renderable access trait and rendering hash map rust access

* deps: upgrade cbindgen to v0.20.0

* rid-macro: ref respected for return type

* rid-macro: fixed bool return type for exports

* rid-macro: fixed string returns from export

* rid-macro: using access type to distinguish return type render

* test: adding todo app integration test

* rid-macro: fix export Dart return type for enum

* test: adding export tests for current status

* rid-macro: export returning primitive refs

* rid-macro: export for string refs including as_str()

* rid-macro: fix numerous vec export issues

- key to determine if vec access is needed fixed
- vec type names fixed
- now multiple vecs can be exported within the same app which was broken
  before

* rid-macro: rust_type::dart_wrapper_rust_ident now returns string

* rid-macro: vec to return type rendering

- handling enums + struct separately
- handling owned vs. referenced primitives

* rid-macro: export supporting Vec<&enum> return

* rid-macro: Vec<&String> via const char pointer

* rid-macro: adding access_string_ref to utils module

* rid-macro: removing snapshot assertions, covered by integration tests

* rid-macro: Vec<&String> exported via vec of pointers

- strings are resolved individually via access
- this is more efficient as strings are only cloned + provided to dart
  when the vec items is accessed
- to distinguish when to resolve strings immediately and render proper
  return type introduced a RustTypeContext

* rid-macro: semi working Vec<&CString> solution

- realized that CString isn't ffi safe, nor is str

* rid-macro: simplifying access of `Vec<&String|&CString>`

- now we immediately resolve to *const char

* rid-macro: improved way to expose CString struct

* rid-macro: handling Vec<&str> exports
  • Loading branch information
thlorenz committed Aug 21, 2021
1 parent 97fd45b commit 89dddfe
Show file tree
Hide file tree
Showing 81 changed files with 2,555 additions and 561 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/target
/.idea
/**/.idea
tests/dart/**/src/wip.rs
tests/dart/export/test/wip.dart
.vimrc
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ exclude = [
"examples/dart/clock",
"examples/dart/todo",
"examples/dart/wip",
"tests/dart/field_access"
"tests/dart/field_access",
"tests/dart/apps",
"tests/dart/export"
]

[dependencies]
Expand Down
6 changes: 4 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
ROOT:=$(shell dirname $(realpath $(firstword $(MAKEFILE_LIST))))
test:
cargo test --all
cd $(ROOT)/tests/dart/field_access && $(MAKE) test-all
cargo test --all && \
cd $(ROOT)/tests/dart/field_access && $(MAKE) test-all && \
cd $(ROOT)/tests/dart/export && $(MAKE) test-all && \
cd $(ROOT)/tests/dart/apps && $(MAKE) test-all
2 changes: 1 addition & 1 deletion rid-build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ doctest = false
[dependencies]
rid_common = { path = "../rid-common" }
rid_ffi = { path = "../rid-ffi" }
cbindgen = "0.18.0"
cbindgen = "0.20.0"
getrandom = { version = "0.2", features = ["js"] }
tempfile = "3.2.0"

Expand Down
1 change: 1 addition & 0 deletions rid-build/src/parsed_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ impl ParsedBindings {

let updated_binding =
replace_struct_aliases(binding, &structs, &struct_aliases);
// let updated_binding = fix_cbindgen_issues(&updated_binding);

let dart_code = join_sections(dart_sections);
let swift_calls: Vec<String> = function_headers
Expand Down
2 changes: 2 additions & 0 deletions rid-common/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ pub const STRING_TO_NATIVE_INT8: &str = "toNativeInt8";
/// Method invoked to free a CString by resolving and dropping it.
pub const CSTRING_FREE: &str = "rid_cstring_free";

pub const STRING_REF_ACCESS: &str = "rid_access_string_ref";

/// Name of the module containing all of global rid util methods like string free.
pub const UTILS_MODULE: &str = "__rid_utils_module";

Expand Down
14 changes: 11 additions & 3 deletions rid-macro-impl/src/common/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub struct ExpandState {
}

pub enum ImplementationType {
VecAccess,
CollectionAccess,
DartEnum,
UtilsModule,
Free,
Expand All @@ -22,8 +22,8 @@ pub enum ImplementationType {
impl fmt::Display for ImplementationType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ImplementationType::VecAccess => {
write!(f, "VecAccess")
ImplementationType::CollectionAccess => {
write!(f, "CollectionAccess")
}
ImplementationType::DartEnum => {
write!(f, "DartEnum")
Expand Down Expand Up @@ -68,6 +68,7 @@ impl ExpandState {
}
}

#[cfg(not(test))]
pub fn unique_ident(&mut self, ident: Ident) -> Ident {
let idents = self.emitted_idents.as_mut().unwrap();
let count: u8 = idents.get(&ident).unwrap_or(&0_u8) + 1;
Expand All @@ -76,6 +77,13 @@ impl ExpandState {
id
}

// Test results need to be the same, no matter if we run just one or all in whichever order
#[cfg(test)]
pub fn unique_ident(&mut self, ident: Ident) -> Ident {
let id = format_ident!("{}_1", ident);
id
}

pub fn need_implemtation<K: fmt::Display, V>(
&mut self,
impl_type: &ImplementationType,
Expand Down
23 changes: 23 additions & 0 deletions rid-macro-impl/src/common/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,29 @@ pub fn resolve_vec_ptr(ty: &syn::Ident) -> TokenStream {
}
}

pub fn resolve_hash_set_ptr(ty: &syn::Ident) -> TokenStream {
quote_spanned! { ty.span() =>
unsafe {
assert!(!ptr.is_null());
let ptr: *mut ::std::collections::HashSet<#ty> = &mut *ptr;
ptr.as_mut().expect("resolve_hash_set_ptr.as_mut failed")
}
}
}

pub fn resolve_hash_map_ptr(
key_ty: &syn::Ident,
val_ty: &syn::Ident,
) -> TokenStream {
quote_spanned! { key_ty.span() =>
unsafe {
assert!(!ptr.is_null());
let ptr: *mut ::std::collections::HashMap<#key_ty, #val_ty> = &mut *ptr;
ptr.as_mut().expect("resolve_hash_map_ptr.as_mut failed")
}
}
}

pub fn resolve_string_ptr(arg: &syn::Ident, reassign: bool) -> TokenStream {
if reassign {
quote_spanned! { arg.span() =>
Expand Down
27 changes: 27 additions & 0 deletions rid-macro-impl/src/common/utils_module_tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ pub fn utils_module_tokens() -> TokenStream {
if get_state()
.needs_implementation(&ImplementationType::UtilsModule, UTILS_MODULE)
{
let cstring_struct_declaration = cstring_struct_declaration();
let str_struct_declaration = str_struct_declaration();
let cstring_free = cstring_free();
quote! {
mod __rid_utils_module {
#str_struct_declaration
#cstring_struct_declaration
#cstring_free
}
}
Expand All @@ -28,6 +32,9 @@ pub fn utils_module_tokens() -> TokenStream {
}
}

// -----------------
// CString
// -----------------
fn cstring_free() -> TokenStream {
let cstring_free_ident = format_ident!("{}", CSTRING_FREE);
quote_spanned! {
Expand All @@ -40,3 +47,23 @@ fn cstring_free() -> TokenStream {
}
}
}

// cbindgen doesn't know what CString is, also technically it is not FFI safe.
// We just use it to point to items in a collection, Never actually sending a pointer directly and
// resolving it later.
fn cstring_struct_declaration() -> TokenStream {
quote! {
#[no_mangle]
pub struct CString {}
}
}

// -----------------
// Str
// -----------------
fn str_struct_declaration() -> TokenStream {
quote! {
#[no_mangle]
pub struct str {}
}
}
9 changes: 4 additions & 5 deletions rid-macro-impl/src/export/attach.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::{
use crate::{attrs::parse_rid_attrs, common::abort};
use quote::{format_ident, quote};

use crate::render_common::RenderableAccess;
use proc_macro2::TokenStream;
use render_dart::render_instance_method_extension;
use syn::Ident;
Expand Down Expand Up @@ -81,7 +82,7 @@ pub fn rid_export_impl(
let mut ptr_type_aliases_map =
HashMap::<String, TokenStream>::new();
let mut frees = HashMap::<String, PointerTypeAlias>::new();
let mut vec_accesses = HashMap::<Ident, VecAccess>::new();
let mut vec_accesses = HashMap::<String, VecAccess>::new();
let rust_fn_tokens = &parsed
.methods
.iter()
Expand Down Expand Up @@ -119,9 +120,7 @@ pub fn rid_export_impl(
);
}
}
vec_access.map(|x| {
vec_accesses.insert(x.vec_type.rust_ident().clone(), x)
});
vec_access.map(|x| vec_accesses.insert(x.key(), x));

tokens
})
Expand All @@ -135,7 +134,7 @@ pub fn rid_export_impl(

let vec_access_tokens = if config.render_vec_access {
let needed_vec_accesses = get_state().need_implemtation(
&ImplementationType::VecAccess,
&ImplementationType::CollectionAccess,
vec_accesses,
);
render_vec_accesses(
Expand Down
70 changes: 68 additions & 2 deletions rid-macro-impl/src/export/export_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,18 @@ use crate::export::ExportConfig;
// More detailed tests with arg/return combinations are tested inside ../render_rust/render_function_export_test.rs
// -----------------

fn render_export(input: TokenStream) -> TokenStream {
fn render(input: TokenStream, config: ExportConfig) -> TokenStream {
let item = syn::parse2::<syn::Item>(input).unwrap();
let args = syn::AttributeArgs::new();
rid_export_impl(item, args, ExportConfig::for_tests())
rid_export_impl(item, args, config)
}

fn render_export(input: TokenStream) -> TokenStream {
render(input, ExportConfig::for_tests())
}

fn render_export_full(input: TokenStream) -> TokenStream {
render(input, ExportConfig::default())
}

// -----------------
Expand All @@ -23,6 +31,9 @@ mod struct_impl_methods {

use super::*;

// -----------------
// Returning Self
// -----------------
#[test]
fn no_args_returning_self() {
let _attrs = TokenStream::new();
Expand Down Expand Up @@ -50,4 +61,59 @@ mod struct_impl_methods {
let tokens = render_export(input);
assert_eq!(tokens.to_string().trim(), expected.to_string().trim());
}

// -----------------
// Returning Vec
// -----------------
#[test]
fn no_args_returning_vec_u8_ref() {
let _attrs = TokenStream::new();
let input: TokenStream = quote! {
#[rid::export]
impl MyStruct {
#[rid::export]
fn get_u8s() -> Vec<&u8> {}
}
};

let expected = quote! {
#[allow(non_snake_case)]
mod __rid_MyStruct_impl_1 {
use super::*;
fn rid_export_MyStruct_get_u8s() -> rid::RidVec<u8> {
let ret = MyStruct::get_u8s();
let ret: Vec<u8> = ret.into_iter().map(|x| *x).collect();
let ret_ptr = rid::RidVec::from(ret);
ret_ptr
}
}
};

let tokens = render_export(input);
dump_tokens(&tokens);
assert_eq!(tokens.to_string().trim(), expected.to_string().trim());
}
}

// DEBUG: export, use this to debug export issues
mod _debug_export_issues {
use crate::common::dump_tokens;

use super::*;

// #[test]
fn debug_export_issue() {
let input: TokenStream = quote! {
impl Store {
#[rid::export]
#[rid::enums(Filter)]
pub fn filters_ref(&self) -> Vec<&Filter> {
self.filters.iter().collect()
}
}
};

let tokens = render_export_full(input);
dump_tokens(&tokens);
}
}
23 changes: 17 additions & 6 deletions rid-macro-impl/src/model/field_access/render_dart_field_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
attrs::TypeInfoMap,
common::abort,
parse::{dart_type::DartType, ParsedStruct, ParsedStructField},
render_common::VecAccessRender,
render_common::AccessRender,
render_dart::RenderDartTypeOpts,
};
use rid_common::{DART_FFI, FFI_GEN_BIND, RID_FFI};
Expand All @@ -14,7 +14,7 @@ pub struct RenderDartFieldAccessConfig {
pub comment: String,
pub render: bool,
pub tokens: bool,
pub vec_accesses: VecAccessRender,
pub accesses: AccessRender,
}

impl Default for RenderDartFieldAccessConfig {
Expand All @@ -23,7 +23,7 @@ impl Default for RenderDartFieldAccessConfig {
comment: "/// ".to_string(),
render: true,
tokens: true,
vec_accesses: VecAccessRender::Default,
accesses: AccessRender::Default,
}
}
}
Expand All @@ -34,18 +34,18 @@ impl RenderDartFieldAccessConfig {
comment: "".to_string(),
render: false,
tokens: false,
vec_accesses: VecAccessRender::Omit,
accesses: AccessRender::Omit,
}
}
}

impl RenderDartFieldAccessConfig {
pub fn for_dart_tests(vec_accesses: VecAccessRender) -> Self {
pub fn for_dart_tests(accesses: AccessRender) -> Self {
Self {
comment: "".to_string(),
render: true,
tokens: false,
vec_accesses,
accesses,
}
}
}
Expand Down Expand Up @@ -198,11 +198,22 @@ impl DartType {
rid_ffi = RID_FFI,
ffi_method = ffi_method_ident
),
// -----------------
// Collection Types
// -----------------
DartType::Vec(_, _) => format!(
"{{ return {rid_ffi}.{ffi_method}(this); }}",
rid_ffi = RID_FFI,
ffi_method = ffi_method_ident
),
DartType::HashMap(_, _, _) => format!(
"{{ return {rid_ffi}.{ffi_method}(this); }}",
rid_ffi = RID_FFI,
ffi_method = ffi_method_ident
),
// -----------------
// Invalid
// -----------------
DartType::Unit => {
abort!(ffi_method_ident, "Dart get to void field not supported")
}
Expand Down
Loading

0 comments on commit 89dddfe

Please sign in to comment.