Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow functions with return type '-> ()' to be bound #123

Merged
merged 3 commits into from
Jul 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions examples/example-deno-runtime/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type {
SerdeVariantRenaming,
StructWithGenerics,
} from "../example-protocol/bindings/ts-runtime/types.ts";
import {Result} from "../example-protocol/bindings/ts-runtime/types.ts";

let voidFunctionCalled = false;

Expand Down Expand Up @@ -203,6 +204,15 @@ const imports: Imports = {
voidFunctionCalled = true;
},

importVoidFunctionEmptyResult: (): Result<void, number> => {
return {
Ok: undefined
};
},

importVoidFunctionEmptyReturn: (): void => {
},

log: (message: string): void => {
// The plugin is not expected to log anything unless it panics:
fail("Plugin panic: " + message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ pub fn import_timestamp(arg: MyDateTime) -> MyDateTime;
#[fp_bindgen_support::fp_import_signature]
pub fn import_void_function();

#[fp_bindgen_support::fp_import_signature]
pub fn import_void_function_empty_result() -> Result<(), u32>;

#[fp_bindgen_support::fp_import_signature]
pub fn import_void_function_empty_return();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is functionally a duplicate of import_void_function() :)

Copy link
Contributor Author

@sagacity sagacity Jul 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but the protocol specifies a -> () return type here, which gets omitted during codegen. Once that is done, the two functions are indeed identical.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I overlooked that 👍


/// Logs a message to the (development) console.
#[fp_bindgen_support::fp_import_signature]
pub fn log(message: String);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,8 @@ fn create_import_object(store: &Store, env: &RuntimeInstanceData) -> ImportObjec
"__fp_gen_import_string" => Function :: new_native_with_env (store , env . clone () , _import_string) ,
"__fp_gen_import_timestamp" => Function :: new_native_with_env (store , env . clone () , _import_timestamp) ,
"__fp_gen_import_void_function" => Function :: new_native_with_env (store , env . clone () , _import_void_function) ,
"__fp_gen_import_void_function_empty_result" => Function :: new_native_with_env (store , env . clone () , _import_void_function_empty_result) ,
"__fp_gen_import_void_function_empty_return" => Function :: new_native_with_env (store , env . clone () , _import_void_function_empty_return) ,
"__fp_gen_log" => Function :: new_native_with_env (store , env . clone () , _log) ,
"__fp_gen_make_http_request" => Function :: new_native_with_env (store , env . clone () , _make_http_request) ,
}
Expand Down Expand Up @@ -1005,6 +1007,15 @@ pub fn _import_void_function(env: &RuntimeInstanceData) {
let result = super::import_void_function();
}

pub fn _import_void_function_empty_result(env: &RuntimeInstanceData) -> FatPtr {
let result = super::import_void_function_empty_result();
export_to_guest(env, &result)
}

pub fn _import_void_function_empty_return(env: &RuntimeInstanceData) {
let result = super::import_void_function_empty_return();
}

pub fn _log(env: &RuntimeInstanceData, message: FatPtr) {
let message = import_from_guest::<String>(env, message);
let result = super::log(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ export type Imports = {
importString: (arg: string) => string;
importTimestamp: (arg: MyDateTime) => MyDateTime;
importVoidFunction: () => void;
importVoidFunctionEmptyResult: () => Result<void, number>;
importVoidFunctionEmptyReturn: () => void;
log: (message: string) => void;
makeHttpRequest: (request: Request) => Promise<HttpResult>;
};
Expand Down Expand Up @@ -345,6 +347,12 @@ export async function createRuntime(
__fp_gen_import_void_function: () => {
importFunctions.importVoidFunction();
},
__fp_gen_import_void_function_empty_result: (): FatPtr => {
return serializeObject(importFunctions.importVoidFunctionEmptyResult());
},
__fp_gen_import_void_function_empty_return: () => {
importFunctions.importVoidFunctionEmptyReturn();
},
__fp_gen_log: (message_ptr: FatPtr) => {
const message = parseObject<string>(message_ptr);
importFunctions.log(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,11 @@ export type Response = {
};

/**
* A result that can be either successful (`Ok)` or represent an error (`Err`).
* A result that can be either successful (`Ok`) or represent an error (`Err`).
*/
export type Result<T, E> =
/**
* Represents a succesful result.
* Represents a successful result.
*/
| { Ok: T }
/**
Expand Down
6 changes: 6 additions & 0 deletions examples/example-protocol/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ fp_import! {
// No arguments, no return type:
fn import_void_function();

// No arguments, empty return type:
fn import_void_function_empty_return() -> ();

// No arguments, generic empty result type:
fn import_void_function_empty_result() -> Result<(), u32>;

// Passing primitives:
fn import_primitive_bool(arg: bool) -> bool;
fn import_primitive_f32(arg: f32) -> f32;
Expand Down
5 changes: 4 additions & 1 deletion examples/example-rust-runtime/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ use time::OffsetDateTime;
use types::*;

fn import_void_function() {}
fn import_void_function_empty_result() -> Result<(), u32> {
Ok(())
}
fn import_void_function_empty_return() -> () {}

fn import_primitive_bool(arg: bool) -> bool {
todo!()
Expand Down Expand Up @@ -103,7 +107,6 @@ fn log(msg: String) {
}

async fn make_http_request(opts: Request) -> Result<Response, RequestError> {

Ok(Response {
body: ByteBuf::from(r#"status: "confirmed"#.to_string()),
headers: opts.headers,
Expand Down
14 changes: 6 additions & 8 deletions fp-bindgen/src/functions.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::utils::normalize_return_type;
use crate::{docs::get_doc_lines, types::TypeIdent};
use quote::{format_ident, quote, ToTokens};
use std::{collections::BTreeSet, convert::TryFrom};
use syn::{token::Async, FnArg, ForeignItemFn, ReturnType};
use syn::{token::Async, FnArg, ForeignItemFn};

/// Maps from function name to the stringified function declaration.
#[derive(Debug, Default)]
Expand Down Expand Up @@ -71,13 +72,10 @@ impl Function {
},
})
.collect();
let return_type = match &item.sig.output {
ReturnType::Default => None,
ReturnType::Type(_, return_type) => Some(
TypeIdent::try_from(return_type.as_ref())
.unwrap_or_else(|_| panic!("Invalid return type for function {}", name)),
),
};
let return_type = normalize_return_type(&item.sig.output).map(|return_type| {
TypeIdent::try_from(return_type)
.unwrap_or_else(|_| panic!("Invalid return type for function {}", name))
});
let is_async = item.sig.asyncness.is_some();

Self {
Expand Down
2 changes: 1 addition & 1 deletion fp-bindgen/src/generators/rust_plugin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ fn format_type_with_ident(ty: &Type, ident: &TypeIdent, types: &TypeMap) -> Stri
.collect::<Vec<_>>()
.join(", ")
),
Type::Unit => "void".to_owned(),
Type::Unit => "()".to_owned(),
_ => ident.to_string(),
}
}
Expand Down
1 change: 1 addition & 0 deletions fp-bindgen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ mod serializable;
pub mod prelude;
pub mod primitives;
pub mod types;
mod utils;

use fp_bindgen_macros::primitive_impls;
use prelude::*;
Expand Down
14 changes: 12 additions & 2 deletions fp-bindgen/src/serializable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ where
Variant {
name: "Ok".to_owned(),
ty: Type::Tuple(vec![TypeIdent::from("T")]),
doc_lines: vec![" Represents a succesful result.".to_owned()],
doc_lines: vec![" Represents a successful result.".to_owned()],
attrs: VariantAttrs::default(),
},
Variant {
Expand All @@ -226,7 +226,7 @@ where
},
],
doc_lines: vec![
" A result that can be either successful (`Ok)` or represent an error (`Err`)."
" A result that can be either successful (`Ok`) or represent an error (`Err`)."
.to_owned(),
],
options: EnumOptions::default(),
Expand All @@ -240,6 +240,16 @@ where
}
}

impl Serializable for () {
fn ident() -> TypeIdent {
TypeIdent::from("()")
}

fn ty() -> Type {
Type::Unit
}
}

impl Serializable for String {
fn ident() -> TypeIdent {
TypeIdent::from("String")
Expand Down
63 changes: 38 additions & 25 deletions fp-bindgen/src/types/type_ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
fmt::Display,
str::FromStr,
};
use syn::{PathArguments, TypePath};
use syn::{PathArguments, TypePath, TypeTuple};

#[derive(Clone, Debug, Eq, Hash, PartialEq)]
pub struct TypeIdent {
Expand Down Expand Up @@ -111,30 +111,43 @@ impl TryFrom<&syn::Type> for TypeIdent {

fn try_from(ty: &syn::Type) -> Result<Self, Self::Error> {
match ty {
syn::Type::Path(TypePath { path, qself }) if qself.is_none() => Ok(Self {
name: path
.segments
.iter()
.map(|segment| segment.ident.to_string())
.collect::<Vec<_>>()
.join("::"),
generic_args: path
.segments
.last()
.and_then(|segment| match &segment.arguments {
PathArguments::AngleBracketed(args) => Some(
args.args
.iter()
.flat_map(|arg| match arg {
syn::GenericArgument::Type(ty) => TypeIdent::try_from(ty),
arg => Err(format!("Unsupported generic argument: {:?}", arg)),
})
.collect(),
),
_ => None,
})
.unwrap_or_default(),
}),
syn::Type::Path(TypePath { path, qself }) if qself.is_none() => {
let mut generic_args = vec![];
if let Some(segment) = path.segments.last() {
match &segment.arguments {
PathArguments::AngleBracketed(args) => {
for arg in &args.args {
match arg {
syn::GenericArgument::Type(ty) => {
generic_args.push(TypeIdent::try_from(ty)?);
}
arg => {
return Err(format!(
"Unsupported generic argument: {:?}",
arg
));
}
}
}
}
_ => {}
}
}

Ok(Self {
name: path
.segments
.iter()
.map(|segment| segment.ident.to_string())
.collect::<Vec<_>>()
.join("::"),
generic_args,
})
}
syn::Type::Tuple(TypeTuple {
elems,
paren_token: _,
}) if elems.is_empty() => Ok(TypeIdent::from("()")),
ty => Err(format!("Unsupported type: {:?}", ty)),
}
}
Expand Down
17 changes: 17 additions & 0 deletions fp-bindgen/src/utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
use syn::{ReturnType, Type};

// This is the same as the one in macros/src/lib.rs, which we unfortunately cannot export.
pub(crate) fn normalize_return_type(ty: &ReturnType) -> Option<&Type> {
match ty {
ReturnType::Default => None,
ReturnType::Type(_, ty) => {
match ty.as_ref() {
Type::Tuple(tuple) if tuple.elems.is_empty() => {
/* An empty '-> ()' return value */
None
}
r => Some(r),
}
}
}
}
21 changes: 8 additions & 13 deletions macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use syn::{
AttributeArgs, FnArg, ForeignItemFn, GenericParam, ItemFn, ItemType, ItemUse, Pat, PatPath,
Path, PathArguments, PathSegment, ReturnType,
};
use utils::flatten_using_statement;
use utils::{flatten_using_statement, normalize_return_type};

mod primitives;
mod serializable;
Expand Down Expand Up @@ -128,19 +128,14 @@ fn parse_statements(token_stream: TokenStream) -> ParsedStatements {
}
}

match &function.sig.output {
ReturnType::Default => { /* No return value. */ }
ReturnType::Type(_, ty) => {
type_paths.insert(extract_path_from_type(ty.as_ref()).unwrap_or_else(
|| {
panic!(
"Only value types are supported. \
if let Some(ty) = normalize_return_type(&function.sig.output) {
type_paths.insert(extract_path_from_type(ty).unwrap_or_else(|| {
panic!(
"Only value types are supported. \
Incompatible return type in function declaration: {:?}",
function.sig
)
},
));
}
function.sig
)
}));
}

functions.push(function.into_token_stream().to_string());
Expand Down
18 changes: 17 additions & 1 deletion macros/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use std::collections::VecDeque;
use proc_macro::TokenStream;
use proc_macro2::Ident;
use syn::{
punctuated::Punctuated, Generics, Item, ItemUse, Path, PathArguments, PathSegment, Type,
punctuated::Punctuated, Generics, Item, ItemUse, Path, PathArguments, PathSegment, ReturnType,
Type,
};

pub(crate) fn extract_path_from_type(ty: &Type) -> Option<Path> {
Expand Down Expand Up @@ -80,3 +81,18 @@ pub(crate) fn flatten_using_statement(using: ItemUse) -> impl Iterator<Item = Pa

result.into_iter()
}

pub(crate) fn normalize_return_type(ty: &ReturnType) -> Option<&Type> {
match ty {
ReturnType::Default => None,
ReturnType::Type(_, ty) => {
match ty.as_ref() {
Type::Tuple(tuple) if tuple.elems.is_empty() => {
/* An empty '-> ()' return value */
None
}
r => Some(r),
}
}
}
}