From 0e72edb80e4e1f1611417ea542f4e1bfb2d4a661 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 16 Mar 2020 18:14:58 -0700 Subject: [PATCH] wiggle-generate: always pass GuestPtr by reference with the prev approach, it would be passed by reference sometimes (e.g. when used as an Array argument) but by value most of the time. this was inconsistient. theres no need to pass the owned version, all operations are &self. --- crates/wiggle/crates/generate/src/funcs.rs | 9 ++++---- .../crates/generate/src/module_trait.rs | 23 +++++++++++++++---- crates/wiggle/tests/flags.rs | 2 +- crates/wiggle/tests/pointers.rs | 8 +++---- crates/wiggle/tests/structs.rs | 9 +++++--- crates/wiggle/tests/wasi.rs | 18 +++++++-------- 6 files changed, 44 insertions(+), 25 deletions(-) diff --git a/crates/wiggle/crates/generate/src/funcs.rs b/crates/wiggle/crates/generate/src/funcs.rs index 6433f7712a..2c82ba79e8 100644 --- a/crates/wiggle/crates/generate/src/funcs.rs +++ b/crates/wiggle/crates/generate/src/funcs.rs @@ -2,6 +2,7 @@ use proc_macro2::TokenStream; use quote::quote; use crate::lifetimes::anon_lifetime; +use crate::module_trait::passed_by_reference; use crate::names::Names; pub fn define_func(names: &Names, func: &witx::InterfaceFunc) -> TokenStream { @@ -67,10 +68,10 @@ pub fn define_func(names: &Names, func: &witx::InterfaceFunc) -> TokenStream { .map(|p| marshal_arg(names, p, error_handling(p.name.as_str()))); let trait_args = func.params.iter().map(|param| { let name = names.func_param(¶m.name); - match param.tref.type_().passed_by() { - witx::TypePassedBy::Value { .. } => quote!(#name), - witx::TypePassedBy::Pointer { .. } => quote!(&#name), - witx::TypePassedBy::PointerLengthPair { .. } => quote!(&#name), + if passed_by_reference(&*param.tref.type_()) { + quote!(&#name) + } else { + quote!(#name) } }); diff --git a/crates/wiggle/crates/generate/src/module_trait.rs b/crates/wiggle/crates/generate/src/module_trait.rs index bd94946bba..b09f237028 100644 --- a/crates/wiggle/crates/generate/src/module_trait.rs +++ b/crates/wiggle/crates/generate/src/module_trait.rs @@ -5,6 +5,21 @@ use crate::lifetimes::{anon_lifetime, LifetimeExt}; use crate::names::Names; use witx::Module; +pub fn passed_by_reference(ty: &witx::Type) -> bool { + let passed_by = match ty.passed_by() { + witx::TypePassedBy::Value { .. } => false, + witx::TypePassedBy::Pointer { .. } | witx::TypePassedBy::PointerLengthPair { .. } => true, + }; + match ty { + witx::Type::Builtin(b) => match &*b { + witx::BuiltinType::String => true, + _ => passed_by, + }, + witx::Type::Pointer(_) | witx::Type::ConstPointer(_) | witx::Type::Array(_) => true, + _ => passed_by, + } +} + pub fn define_module_trait(names: &Names, m: &Module) -> TokenStream { let traitname = names.trait_name(&m.name); let traitmethods = m.funcs().map(|f| { @@ -25,10 +40,10 @@ pub fn define_module_trait(names: &Names, m: &Module) -> TokenStream { let args = f.params.iter().map(|arg| { let arg_name = names.func_param(&arg.name); let arg_typename = names.type_ref(&arg.tref, lifetime.clone()); - let arg_type = match arg.tref.type_().passed_by() { - witx::TypePassedBy::Value { .. } => quote!(#arg_typename), - witx::TypePassedBy::Pointer { .. } => quote!(&#arg_typename), - witx::TypePassedBy::PointerLengthPair { .. } => quote!(&#arg_typename), + let arg_type = if passed_by_reference(&*arg.tref.type_()) { + quote!(&#arg_typename) + } else { + quote!(#arg_typename) }; quote!(#arg_name: #arg_type) }); diff --git a/crates/wiggle/tests/flags.rs b/crates/wiggle/tests/flags.rs index 7a6ddcaebf..4270ef50c7 100644 --- a/crates/wiggle/tests/flags.rs +++ b/crates/wiggle/tests/flags.rs @@ -14,7 +14,7 @@ impl<'a> flags::Flags for WasiCtx<'a> { fn configure_car( &self, old_config: types::CarConfig, - other_config_ptr: GuestPtr, + other_config_ptr: &GuestPtr, ) -> Result { let other_config = other_config_ptr.read().map_err(|e| { eprintln!("old_config_ptr error: {}", e); diff --git a/crates/wiggle/tests/pointers.rs b/crates/wiggle/tests/pointers.rs index 188fa9822c..cbabd76563 100644 --- a/crates/wiggle/tests/pointers.rs +++ b/crates/wiggle/tests/pointers.rs @@ -13,9 +13,9 @@ impl<'a> pointers::Pointers for WasiCtx<'a> { fn pointers_and_enums<'b>( &self, input1: types::Excuse, - input2_ptr: GuestPtr<'b, types::Excuse>, - input3_ptr: GuestPtr<'b, types::Excuse>, - input4_ptr_ptr: GuestPtr<'b, GuestPtr<'b, types::Excuse>>, + input2_ptr: &GuestPtr<'b, types::Excuse>, + input3_ptr: &GuestPtr<'b, types::Excuse>, + input4_ptr_ptr: &GuestPtr<'b, GuestPtr<'b, types::Excuse>>, ) -> Result<(), types::Errno> { println!("BAZ input1 {:?}", input1); let input2: types::Excuse = input2_ptr.read().map_err(|e| { @@ -52,7 +52,7 @@ impl<'a> pointers::Pointers for WasiCtx<'a> { println!("input4 {:?}", input4); // Write ptr value to mutable ptr: - input4_ptr_ptr.write(input2_ptr).map_err(|e| { + input4_ptr_ptr.write(*input2_ptr).map_err(|e| { eprintln!("input4_ptr_ptr error: {}", e); types::Errno::InvalidArg })?; diff --git a/crates/wiggle/tests/structs.rs b/crates/wiggle/tests/structs.rs index 0b00057d9a..0be912feeb 100644 --- a/crates/wiggle/tests/structs.rs +++ b/crates/wiggle/tests/structs.rs @@ -44,10 +44,13 @@ impl<'a> structs::Structs for WasiCtx<'a> { fn return_pair_of_ptrs<'b>( &self, - first: GuestPtr<'b, i32>, - second: GuestPtr<'b, i32>, + first: &GuestPtr<'b, i32>, + second: &GuestPtr<'b, i32>, ) -> Result, types::Errno> { - Ok(types::PairIntPtrs { first, second }) + Ok(types::PairIntPtrs { + first: *first, + second: *second, + }) } } diff --git a/crates/wiggle/tests/wasi.rs b/crates/wiggle/tests/wasi.rs index 6ad5612b69..df7be73a8e 100644 --- a/crates/wiggle/tests/wasi.rs +++ b/crates/wiggle/tests/wasi.rs @@ -23,7 +23,7 @@ impl<'a> GuestErrorType<'a> for types::Errno { } impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { - fn args_get(&self, _argv: GuestPtr>, _argv_buf: GuestPtr) -> Result<()> { + fn args_get(&self, _argv: &GuestPtr>, _argv_buf: &GuestPtr) -> Result<()> { unimplemented!("args_get") } @@ -33,8 +33,8 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { fn environ_get( &self, - _environ: GuestPtr>, - _environ_buf: GuestPtr, + _environ: &GuestPtr>, + _environ_buf: &GuestPtr, ) -> Result<()> { unimplemented!("environ_get") } @@ -153,7 +153,7 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { fn fd_prestat_dir_name( &self, _fd: types::Fd, - _path: GuestPtr, + _path: &GuestPtr, _path_len: types::Size, ) -> Result<()> { unimplemented!("fd_prestat_dir_name") @@ -175,7 +175,7 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { fn fd_readdir( &self, _fd: types::Fd, - _buf: GuestPtr, + _buf: &GuestPtr, _buf_len: types::Size, _cookie: types::Dircookie, ) -> Result { @@ -260,7 +260,7 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { &self, _fd: types::Fd, _path: &GuestPtr<'_, str>, - _buf: GuestPtr, + _buf: &GuestPtr, _buf_len: types::Size, ) -> Result { unimplemented!("path_readlink") @@ -295,8 +295,8 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { fn poll_oneoff( &self, - _in_: GuestPtr, - _out: GuestPtr, + _in_: &GuestPtr, + _out: &GuestPtr, _nsubscriptions: types::Size, ) -> Result { unimplemented!("poll_oneoff") @@ -314,7 +314,7 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { unimplemented!("sched_yield") } - fn random_get(&self, _buf: GuestPtr, _buf_len: types::Size) -> Result<()> { + fn random_get(&self, _buf: &GuestPtr, _buf_len: types::Size) -> Result<()> { unimplemented!("random_get") }