From 03ef75660932068adf57ec09715516e8e1eaefef Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Wed, 12 Aug 2026 09:12:03 +0100 Subject: [PATCH] [hyperlight_component_util] Properly track positivity/negativity Previously, bindgen code relied upon state that it tracked at compile time to determine whether a particular definition was in a positive or negative position (see `State::is_export`). Unfortunately, since type definitions presently are maintained based on wit-style names, and the same wit interface can be both imported and exported from the main component, this did not work very well. This commit changes the generated types to include a type parameter that tracks whether they are being used in a positive or negative manner. It also changes the one current user of `State::is_export` to instead use that type parameter, ensuring that the single generated definition can be used both as an import and as an export. Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- src/hyperlight_common/src/component.rs | 70 ++++++ src/hyperlight_common/src/lib.rs | 4 + src/hyperlight_component_util/src/emit.rs | 56 +++-- src/hyperlight_component_util/src/guest.rs | 30 +-- src/hyperlight_component_util/src/host.rs | 30 +-- src/hyperlight_component_util/src/rtypes.rs | 228 ++++++++++++-------- src/hyperlight_host/tests/wit_test.rs | 45 +++- src/tests/rust_guests/witguest/src/main.rs | 60 ++++-- 8 files changed, 356 insertions(+), 167 deletions(-) create mode 100644 src/hyperlight_common/src/component.rs diff --git a/src/hyperlight_common/src/component.rs b/src/hyperlight_common/src/component.rs new file mode 100644 index 0000000000..524d90f114 --- /dev/null +++ b/src/hyperlight_common/src/component.rs @@ -0,0 +1,70 @@ +/* +Copyright 2026 The Hyperlight Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + */ + +//! Support types for the bindings that `host_bindgen!` generates. + +use crate::resource::BorrowedResourceGuard; + +mod private { + pub trait Sealed {} +} + +/// Whether an instance/resource/etc is being used in a positive or +/// negative position in the top-level component type that governs the +/// interface. That is to say, whether the functions provided by this +/// instance/resource (or exported by this component) are expected to +/// be implemented in the guest and called on the host or vice versa. +/// +/// We say that a piece of a top-level component type is in "negative +/// position" if it is on the left hand side of an odd number of +/// arrows, and positive otherwise. With only first-order component +/// types, this distinction collapses to whether it is part of an +/// import (negative) or export (positive), but with higher-order +/// components, this is no longer the case. For example, if a +/// component imports another component which itself imports some +/// functions, those functions are in positive position in the overall +/// type---because they are supplied by the guest when it instantiates +/// the component it imported---even though they are syntactically +/// imports. +pub trait Positivity: private::Sealed { + type NegativeOfThis: Positivity; + /// How a borrowed resource handle reaches the implementation. + type Borrow<'a, T: 'a>; +} + +/// A type is being used in a negative position in the overall type: +/// it is implemented by the host, and the guest calls it. +pub enum Negative {} + +/// A type is being used in a positive position in the overall type: +/// it is implemented by the guest, and the host calls it. +pub enum Positive {} + +impl private::Sealed for Negative {} +impl private::Sealed for Positive {} + +impl Positivity for Negative { + type NegativeOfThis = Positive; + /// A handle arrives as an index into the resource table, held borrowed + /// for the duration of the call. + type Borrow<'a, T: 'a> = BorrowedResourceGuard<'a, T>; +} + +impl Positivity for Positive { + type NegativeOfThis = Negative; + /// The host owns the value, so it hands out a plain reference. + type Borrow<'a, T: 'a> = &'a T; +} diff --git a/src/hyperlight_common/src/lib.rs b/src/hyperlight_common/src/lib.rs index 6e12d8cd46..f342cf842b 100644 --- a/src/hyperlight_common/src/lib.rs +++ b/src/hyperlight_common/src/lib.rs @@ -22,6 +22,10 @@ limitations under the License. extern crate alloc; +/// cbindgen:ignore +/// Support types for the bindings that `{host,guest}_bindgen!` generates +pub mod component; + pub mod flatbuffer_wrappers; /// cbindgen:ignore /// FlatBuffers-related utilities and (mostly) generated code diff --git a/src/hyperlight_component_util/src/emit.rs b/src/hyperlight_component_util/src/emit.rs index 1a4b7a9052..b0ac6296d7 100644 --- a/src/hyperlight_component_util/src/emit.rs +++ b/src/hyperlight_component_util/src/emit.rs @@ -135,7 +135,7 @@ fn component_first_camel(s: &str) -> String { /// A representation of a trait definition that we will eventually /// emit. This is used to allow easily adding onto the trait each time /// we see an extern decl. -#[derive(Debug, Default)] +#[derive(Clone, Debug, Default)] pub struct Trait { /// A set of supertrait constraints, each associated with a /// bindings module path @@ -194,11 +194,12 @@ impl Trait { /// Build a token stream for the type variable part of the trait /// declaration pub fn tv_toks(&mut self) -> TokenStream { + let p = quote! { P: ::hyperlight_common::component::Positivity }; if !self.tvs.is_empty() { let toks = self.tv_toks_inner(); - quote! { <#toks> } + quote! { <#p, #toks> } } else { - quote! {} + quote! { <#p> } } } /// Build a token stream for this entire trait definition @@ -226,12 +227,12 @@ impl Trait { /// A representation of a module definition that we will eventually /// emit. This is used to allow easily adding onto the module each time /// we see a relevant decl. -#[derive(Debug, Default)] +#[derive(Clone, Debug, Default)] pub struct Mod { pub submods: BTreeMap, pub items: TokenStream, pub traits: BTreeMap, - pub impls: BTreeMap<(Vec, Ident), TokenStream>, + pub impls: BTreeMap<(Vec, Ident), (TokenStream, TokenStream)>, } impl Mod { pub fn empty() -> Self { @@ -268,7 +269,7 @@ impl Mod { /// /// Currently, we don't track much information about these, so /// it's just a mutable token stream. - pub fn r#impl<'a>(&'a mut self, t: Vec, i: Ident) -> &'a mut TokenStream { + pub fn r#impl<'a>(&'a mut self, t: Vec, i: Ident) -> &'a mut (TokenStream, TokenStream) { self.impls.entry((t, i)).or_default() } /// See [`State::adjust_vars`]. @@ -295,9 +296,9 @@ impl Mod { tt.extend(t.into_tokens(n)); } tt.extend(self.items); - for ((ns, i), t) in self.impls { + for ((ns, i), (tvi, t)) in self.impls { tt.extend(quote! { - impl #(#ns)::* for #i { #t } + impl #(#ns)::* #tvi for #i { #t } }) } tt @@ -404,6 +405,10 @@ pub struct State<'a, 'b> { /// `self_param_var`, which will need to be fixed when extending /// higher-order component bindings generation to impls. pub self_param_var: Option, + /// The Rust type parameter used to represent the type that + /// provides the positivity of the (eventual use of the) current + /// component + pub positivity_param: Option, /// Whether we are emitting an implementation of the component /// interfaces, or just the types of the interface pub is_impl: bool, @@ -417,8 +422,6 @@ pub struct State<'a, 'b> { /// wasmtime guest emit. When that is refactored to use the host /// guest emit, this can go away. pub is_wasmtime_guest: bool, - /// Are we working on an export or an import of the component type? - pub is_export: bool, /// Set of interface names that collide across different packages /// (e.g. "types" appears in both wasi:filesystem/types and wasi:http/types). /// When a name is in this set, the parent namespace is prepended to @@ -471,11 +474,11 @@ impl<'a, 'b> State<'a, 'b> { vars_needs_vars, import_param_var: None, self_param_var: None, + positivity_param: None, is_impl: false, root_component_name: None, is_guest, is_wasmtime_guest, - is_export: false, colliding_import_names: HashSet::new(), } } @@ -492,12 +495,12 @@ impl<'a, 'b> State<'a, 'b> { cur_needs_vars: self.cur_needs_vars.as_deref_mut(), vars_needs_vars: self.vars_needs_vars, import_param_var: self.import_param_var.clone(), + positivity_param: self.positivity_param.clone(), self_param_var: self.self_param_var.clone(), is_impl: self.is_impl, root_component_name: self.root_component_name.clone(), is_guest: self.is_guest, is_wasmtime_guest: self.is_wasmtime_guest, - is_export: self.is_export, colliding_import_names: self.colliding_import_names.clone(), } } @@ -542,6 +545,17 @@ impl<'a, 'b> State<'a, 'b> { s.cur_needs_vars = Some(needs_vars); s } + /// Copy the state, replacing its [`State::root_mod`] reference, + /// allowing a caller to capture _only_ the effects on + /// [`State::cur_needs_vars`]/[`State::vars_needs_vars`] of an + /// emit run with the resultant state + pub fn for_var_effects_only FnOnce(&mut State<'c, 'b>)>(&mut self, f: F) { + let mut new_mod = self.root_mod.clone(); + let mut s = self.clone(); + s.root_mod = &mut new_mod; + f(&mut s); + } + /// Record that an emit sequence needed a var, given an absolute /// index for the var (i.e. ignoring [`State::var_offset`]) pub fn need_noff_var(&mut self, n: u32) { @@ -684,13 +698,17 @@ impl<'a, 'b> State<'a, 'b> { /// Add an import/export to [`State::origin`], reflecting that we are now /// looking at code underneath it /// - /// origin_was_export differs from s.is_export in that s.is_export - /// keeps track of whether the item overall was imported or exported - /// from the root component (taking into account positivity), whereas - /// origin_was_export just checks if this particular extern_decl was - /// imported or exported from its parent instance (and so e.g. an - /// export of an instance that is imported by the root component has - /// !s.is_export && origin_was_export) + /// origin_was_export does not keep track of whether the item + /// overall was imported or exported from the root component + /// (taking into account positivity); it just checks if this + /// particular extern_decl was imported or exported from its + /// parent instance (and so e.g. an export of an instance that is + /// imported by the root component has origin_was_export). Any + /// decisions that depend on positivity from the root component + /// should be made part of the + /// [`hyperlight_common::component::Positivity`] trait, which + /// correctly handles the fact that the same interface trait may + /// be used in both positive and negative positions. pub fn push_origin<'c>(&'c mut self, origin_was_export: bool, name: &'b str) -> State<'c, 'b> { let mut s = self.clone(); s.origin.push(if origin_was_export { diff --git a/src/hyperlight_component_util/src/guest.rs b/src/hyperlight_component_util/src/guest.rs index a56777d690..d453796db9 100644 --- a/src/hyperlight_component_util/src/guest.rs +++ b/src/hyperlight_component_util/src/guest.rs @@ -91,7 +91,10 @@ fn emit_import_extern_decl<'a, 'b, 'c>( // here, but that is not the case at the // moment. let path = s.resource_trait_path(r); - s.root_mod.r#impl(path, format_ident!("Host")).extend(decl); + s.root_mod + .r#impl(path, format_ident!("Host")) + .1 + .extend(decl); TokenStream::new() } } @@ -105,11 +108,11 @@ fn emit_import_extern_decl<'a, 'b, 'c>( }; let rtid = format_ident!("HostResource{}", noff); let path = s.resource_trait_path(kebab_to_type(ed.kebab_name)); - s.root_mod - .r#impl(path, format_ident!("Host")) - .extend(quote! { - type T = #rtid; - }); + let r#impl = s.root_mod.r#impl(path, format_ident!("Host")); + r#impl.0 = quote! { <::hyperlight_common::component::Negative> }; + r#impl.1.extend(quote! { + type T = #rtid; + }); TokenStream::new() } _ => quote! {}, @@ -152,7 +155,7 @@ fn emit_import_instance<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, it: & .chain(&[kebab_to_type(wn.name)]) .cloned() .collect::>(); - let trait_ref = rtypes::trait_ref(&mut s, true, &trait_path); + let trait_ref = rtypes::trait_ref(&mut s, rtypes::EmitPositivity::Opposite, true, &trait_path); s.root_mod.items.extend(quote! { impl #trait_ref for Host { #(#imports)* @@ -279,6 +282,7 @@ fn emit_component<'a, 'b, 'c>( let r#trait = kebab_to_type(wn.name); let import_trait = kebab_to_imports_name(wn.name); let export_trait = kebab_to_exports_name(wn.name); + s.positivity_param = Some(quote! { ::hyperlight_common::component::Positive }); // We don't set s.self_param_var or s.import_param_var at all // here, because they are currently obviated by the (s.is_guest && // s.is_impl) hack in rtypes::emit_resource_ref. For when we @@ -292,8 +296,8 @@ fn emit_component<'a, 'b, 'c>( resource::emit_tables( &mut s, rtsid.clone(), - quote! { #ns::#import_trait + ::core::marker::Send + 'static }, - Some(quote! { #ns::#export_trait }), + quote! { #ns::#import_trait<::hyperlight_common::component::Negative> + ::core::marker::Send + 'static }, + Some(quote! { #ns::#export_trait<::hyperlight_common::component::Positive, I> }), true, ); s.root_mod @@ -313,6 +317,7 @@ fn emit_component<'a, 'b, 'c>( .map(|ed| emit_import_extern_decl(&mut s, ed)) .collect::>(); s.var_offset = 0; + s.positivity_param = Some(quote! { ::hyperlight_common::component::Positive }); // We don't set s.self_param_var or s.import_param_var at all // here, because it is currently obviated by the (s.is_guest && // s.is_impl) hack in rtypes::emit_resource_ref. For when we @@ -320,10 +325,7 @@ fn emit_component<'a, 'b, 'c>( // // See Note [Origin paths and self parameters in impl codegen for higher-order components] // in emit.rs - - s.is_export = true; s.cur_trait = Some(export_trait.clone()); - let exports = ct .instance .unqualified @@ -333,7 +335,7 @@ fn emit_component<'a, 'b, 'c>( .collect::>(); s.root_mod.items.extend(quote! { - impl #ns::#import_trait for Host { + impl #ns::#import_trait<::hyperlight_common::component::Negative> for Host { #(#imports)* } }); @@ -365,7 +367,7 @@ pub fn emit_toplevel<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, n: &str, ct: &'c Comp /// Because Hyperlight guest functions can't close over any /// state, this function is used on each guest call to acquire /// any state that the guest functions might need. - pub trait Guest: #ns::#export_trait { + pub trait Guest: #ns::#export_trait<::hyperlight_common::component::Positive, Host> { fn with_guest_state R>(f: F) -> R; } /// Register all guest functions. diff --git a/src/hyperlight_component_util/src/host.rs b/src/hyperlight_component_util/src/host.rs index 14a82fec78..96f6043c9d 100644 --- a/src/hyperlight_component_util/src/host.rs +++ b/src/hyperlight_component_util/src/host.rs @@ -142,7 +142,7 @@ fn emit_export_instance<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, it: & quote! { #ns::#trait_name } }; s.root_mod.items.extend(quote! { - impl #trait_path <#(#tvs),*> for #wrapper_name { + impl, S: ::hyperlight_host::sandbox::Callable> #trait_path <::hyperlight_common::component::Positive #(,#tvs)*> for #wrapper_name { #(#exports)* } }); @@ -296,7 +296,8 @@ fn emit_import_extern_decl<'a, 'b, 'c>( .chain(&[kebab_to_type(wn.name)]) .cloned() .collect::>(); - let trait_ref = rtypes::trait_ref(&mut s, true, &trait_path); + let trait_ref = + rtypes::trait_ref(&mut s, rtypes::EmitPositivity::Opposite, true, &trait_path); let get_self = get_self.with_getter(tp, type_name, trait_ref, getter); emit_import_instance(&mut s, get_self, wn.clone(), it) } @@ -356,11 +357,12 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com let rtsid = format_ident!("{}Resources", r#trait); s.import_param_var = Some(format_ident!("I")); + s.positivity_param = Some(quote! { ::hyperlight_common::component::Positive }); s.colliding_import_names = find_colliding_import_names(&ct.imports); resource::emit_tables( &mut s, rtsid.clone(), - quote! { #ns::#import_trait }, + quote! { #ns::#import_trait<::hyperlight_common::component::Negative> }, None, false, ); @@ -373,7 +375,10 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com .map(|ed| { emit_import_extern_decl( &mut s, - SelfInfo::new(import_id.clone(), quote! { #ns::#import_trait }), + SelfInfo::new( + import_id.clone(), + quote! { #ns::#import_trait<::hyperlight_common::component::Negative> }, + ), ed, ) }) @@ -383,10 +388,11 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com s.root_component_name = Some((ns.clone(), wn.name)); s.cur_trait = Some(export_trait.clone()); s.import_param_var = Some(format_ident!("I")); + s.positivity_param = Some(quote! { ::hyperlight_common::component::Positive }); // See Note [Origin paths and self parameters in impl codegen for higher-order components] // in emit.rs - s.self_param_var = Some(quote! { > }); - s.is_export = true; + s.self_param_var = + Some(quote! { > }); let exports = ct .instance @@ -397,22 +403,22 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com .collect::>(); s.root_mod.items.extend(quote! { - pub struct #wrapper_name { + pub struct #wrapper_name, S: ::hyperlight_host::sandbox::Callable> { pub(crate) sb: S, pub(crate) rt: ::std::sync::Arc<::std::sync::Mutex<#rtsid>>, } - pub(crate) fn register_host_functions(sb: &mut S, i: I) -> ::std::sync::Arc<::std::sync::Mutex<#rtsid>> { + pub(crate) fn register_host_functions + ::std::marker::Send + 'static, S: ::hyperlight_host::func::Registerable>(sb: &mut S, i: I) -> ::std::sync::Arc<::std::sync::Mutex<#rtsid>> { let rts = ::std::sync::Arc::new(::std::sync::Mutex::new(#rtsid::new())); let #import_id = ::std::sync::Arc::new(::std::sync::Mutex::new(i)); #(#imports)* rts } - impl #ns::#export_trait for #wrapper_name { + impl + ::std::marker::Send, S: ::hyperlight_host::sandbox::Callable> #ns::#export_trait<::hyperlight_common::component::Positive, I> for #wrapper_name { #(#exports)* } - impl #ns::#r#trait for ::hyperlight_host::sandbox::UninitializedSandbox { - type Exports = #wrapper_name; - fn instantiate(mut self, i: I) -> Self::Exports { + impl #ns::#r#trait<::hyperlight_common::component::Positive> for ::hyperlight_host::sandbox::UninitializedSandbox { + type Exports + ::std::marker::Send> = #wrapper_name; + fn instantiate + ::std::marker::Send + 'static>(mut self, i: I) -> Self::Exports { let rts = register_host_functions(&mut self, i); let sb = self.evolve().unwrap(); #wrapper_name { diff --git a/src/hyperlight_component_util/src/rtypes.rs b/src/hyperlight_component_util/src/rtypes.rs index 0c50d89353..4ff6ea3513 100644 --- a/src/hyperlight_component_util/src/rtypes.rs +++ b/src/hyperlight_component_util/src/rtypes.rs @@ -33,26 +33,51 @@ use crate::etypes::{ Param, TypeBound, Tyvar, Value, }; +#[derive(Clone, Copy, Debug)] +pub(crate) enum EmitPositivity { + Same, + Opposite, +} +impl EmitPositivity { + fn invert(self) -> Self { + match self { + EmitPositivity::Same => EmitPositivity::Opposite, + EmitPositivity::Opposite => EmitPositivity::Same, + } + } +} + /// When referring to an instance or resource trait, emit a token /// stream that instantiates any types it is parametrized by with our /// own best understanding of how to name the relevant type variables -fn emit_tvis(s: &mut State, tvs: Vec) -> TokenStream { +fn emit_tvis(s: &mut State, ep: EmitPositivity, tvs: Vec) -> TokenStream { let tvs = tvs .iter() .map(|tv| emit_var_ref_noff(s, *tv, false)) .collect::>(); - if !tvs.is_empty() { - quote! { <#(#tvs),*> } - } else { - TokenStream::new() + let p = s.positivity_param.clone().unwrap_or(quote! { P }); + match ep { + EmitPositivity::Same => quote! { <#p #(,#tvs)*> }, + EmitPositivity::Opposite => { + quote! { <<#p as ::hyperlight_common::component::Positivity>::NegativeOfThis #(,#tvs)*> } + } } } /// Construct a token stream referencing a trait at a given trait path -pub(crate) fn trait_ref(s: &mut State, absolute: bool, path: &[Ident]) -> TokenStream { - let rp = s.root_path(); +pub(crate) fn trait_ref( + s: &mut State, + ep: EmitPositivity, + absolute: bool, + path: &[Ident], +) -> TokenStream { + let rp = if absolute { + s.root_path() + } else { + TokenStream::new() + }; let t = s.resolve_trait_immut(absolute, path); - let tvis = emit_tvis(s, t.tv_idxs()); + let tvis = emit_tvis(s, ep, t.tv_idxs()); quote! { #rp #(#path)::* #tvis } } @@ -87,13 +112,17 @@ fn emit_resource_ref(s: &mut State, n: u32, path: Vec) -> TokenStr if path.len() == 1 { let helper = s.cur_helper_mod.clone().unwrap(); let rtrait = kebab_to_type(path[0].name()); - let t = s.resolve_trait_immut(false, &[helper.clone(), rtrait.clone()]); - let tvis = emit_tvis(s, t.tv_idxs()); + let trait_ref = trait_ref( + s, + EmitPositivity::Same, + false, + &[helper.clone(), rtrait.clone()], + ); let mut sv = quote! { Self }; if let Some(s) = &s.self_param_var { sv = quote! { #s }; }; - return quote! { <#sv as #helper::#rtrait #tvis>::T }; + return quote! { <#sv as #trait_ref>::T }; }; // Generally speaking, the structure that we expect to see in @@ -108,10 +137,18 @@ fn emit_resource_ref(s: &mut State, n: u32, path: Vec) -> TokenStr } else if let Some(s) = &s.self_param_var { toks = quote! { #s } } - // todo:this will need a bit of adjustment to work well with + // todo: this will need a bit of adjustment to work well with // plainname externs, which may require keeping track of the last // interfacename we saw + let mut ep = EmitPositivity::Same; for (i, p) in path[0..path.len() - 1].iter().enumerate() { + // Don't update ep if the import is the first item on the path + // and we don't have a var pointing at a different imports + // trait instance, since that means we are in an `Imports` + // trait, and our `P` has already been negative'd. + if p.imported() && (i != 0 || s.import_param_var.is_some()) { + ep = ep.invert(); + } let iwn = split_wit_name(p.name()); let export_name = if p.imported() { import_member_names(&iwn, &s.colliding_import_names).0 @@ -134,7 +171,7 @@ fn emit_resource_ref(s: &mut State, n: u32, path: Vec) -> TokenStr .chain(namespace_suffix.iter()) .cloned() .collect::>(); - let trait_ref = trait_ref(s, true, &trait_path); + let trait_ref = trait_ref(s, ep, true, &trait_path); toks = quote! { <#toks::#export_name as #trait_ref> }; } @@ -367,11 +404,8 @@ pub fn emit_value(s: &mut State, vt: &Value) -> TokenStream { } } else { let vr = emit_var_ref(s, tv); - if s.is_export { - quote! { &#vr } - } else { - quote! { ::hyperlight_common::resource::BorrowedResourceGuard<#vr> } - } + let p = s.positivity_param.clone().unwrap_or(quote! { P }); + quote! { <#p as ::hyperlight_common::component::Positivity>::Borrow<'_, #vr> } } } }, @@ -653,9 +687,6 @@ fn emit_type_alias TokenStream>( /// Emit (via returning) a Rust trait item corresponding to this /// extern decl -/// -/// See note on emit.rs push_origin for the difference between -/// origin_was_export and s.is_export. fn emit_extern_decl<'a, 'b, 'c>( origin_was_export: bool, s: &'c mut State<'a, 'b>, @@ -761,33 +792,20 @@ fn emit_extern_decl<'a, 'b, 'c>( let wn = split_wit_name(ed.kebab_name); emit_instance(&mut s, wn.clone(), it); - let nsids = wn.namespace_idents(); - let repr = s.r#trait(&nsids, kebab_to_type(wn.name)); - let vs = if !repr.tvs.is_empty() { - let vs = repr.tvs.clone(); - let tvs = vs - .iter() - .map(|(_, (tv, _))| emit_var_ref(&mut s, &Tyvar::Bound(tv.unwrap()))); - quote! { <#(#tvs),*> } - } else { - TokenStream::new() - }; - let (member_tn, member_getter) = if origin_was_export { (kebab_to_type(wn.name), kebab_to_getter(wn.name)) } else { import_member_names(&wn, &s.colliding_import_names) }; - let rp = s.root_path(); - let tns = wn.namespace_path(); - let trait_tn = kebab_to_type(wn.name); - let trait_bound = if tns.is_empty() { - quote! { #rp #trait_tn } - } else { - quote! { #rp #tns::#trait_tn } - }; + let trait_path = wn + .namespace_idents() + .iter() + .chain(&[kebab_to_type(wn.name)]) + .cloned() + .collect::>(); + let trait_ref = trait_ref(&mut s, EmitPositivity::Same, true, &trait_path); quote! { - type #member_tn: #trait_bound #vs; + type #member_tn: #trait_ref; fn #member_getter(&mut self) -> impl ::core::borrow::BorrowMut; } } @@ -807,58 +825,80 @@ fn emit_instance<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, it: &'c Inst s.cur_helper_mod = Some(kebab_to_namespace(wn.name)); s.cur_trait = Some(name.clone()); - if !s.cur_trait().items.is_empty() { - // Temporary hack: we have visited this wit:package/instance - // before, so bail out instead of adding duplicates of - // everything. Since we don't really have strong semantic - // guarantees that the exact same contents will be in each - // occurrence of a wit:package/instance (and indeed they may - // well be stripped down to the essentials in each - // occurrence), this is NOT sound, and will need to be - // revisited. The correct approach here is to change - // emit_extern_decl to create function/resource items in a - // Trait that can be merged properly, instead of directly - // emitting tokens. - return; - } - let mut needs_vars = BTreeSet::new(); - let mut sv = s.with_needs_vars(&mut needs_vars); + // Temporary hack: if some items have already been generated for + // this wit:package/instance, implying that we have visited it + // before, we use [`State::for_var_effects_only`] to avoid adding + // duplicates of everything. We still bother running it, instead + // of bailing out entirely, to, as the name implies, get the + // effects on the variable tracking---otherwise the "second copy" + // of some Eq-bounded type variables will not properly acquire + // dependency tracking information. + // + // Since we don't really have strong semantic guarantees that the + // exact same contents will be in each occurrence of a + // wit:package/instance (and indeed they may well be stripped down + // to the essentials in each occurrence), this is NOT sound, and + // will need to be revisited. The really proper approach here is + // probably to properly spec some unification/principle type at + // the component level and preemptively run it on everything with + // the same extern name. + fn run_normally<'a, 'b, 'c>( + s: &'c mut State<'a, 'b>, + f: impl for<'d> FnOnce(&mut State<'d, 'b>), + ) { + f(s) + } + fn run_for_var_effects_only<'a, 'b, 'c>( + s: &'c mut State<'a, 'b>, + f: impl for<'d> FnOnce(&mut State<'d, 'b>), + ) { + s.for_var_effects_only(f) + } + let run = if s.cur_trait().items.is_empty() { + run_normally + } else { + run_for_var_effects_only + }; + run(&mut s, &mut |s: &mut State<'_, 'b>| { + let mut needs_vars = BTreeSet::new(); + let mut sv = s.with_needs_vars(&mut needs_vars); - let exports = it - .exports - .iter() - .map(|ed| emit_extern_decl(true, &mut sv, ed)) - .collect::>(); + let exports = it + .exports + .iter() + .map(|ed| emit_extern_decl(true, &mut sv, ed)) + .collect::>(); - // instantiations for the supertraits + // instantiations for the supertraits - let mut stvs = BTreeMap::new(); - let _ = sv.cur_trait(); // make sure it exists - let t = sv.cur_trait_immut(); - for (ti, _) in t.supertraits.iter() { - let t = sv.resolve_trait_immut(false, ti); - stvs.insert(ti.clone(), t.tv_idxs()); - } - // hack to make the local-definedness check work properly, since - // it usually should ignore the last origin component - sv.origin.push(ImportExport::Export("self")); - let mut stis = BTreeMap::new(); - for (id, tvs) in stvs.into_iter() { - stis.insert(id, emit_tvis(&mut sv, tvs)); - } - for (id, ts) in stis.into_iter() { - sv.cur_trait().supertraits.get_mut(&id).unwrap().extend(ts); - } + let mut stvs = BTreeMap::new(); + let _ = sv.cur_trait(); // make sure it exists + let t = sv.cur_trait_immut(); + for (ti, _) in t.supertraits.iter() { + let t = sv.resolve_trait_immut(false, ti); + stvs.insert(ti.clone(), t.tv_idxs()); + } + // hack to make the local-definedness check work properly, since + // it usually should ignore the last origin component + sv.origin.push(ImportExport::Export("self")); + let mut stis = BTreeMap::new(); + for (id, tvs) in stvs.into_iter() { + stis.insert(id, emit_tvis(&mut sv, EmitPositivity::Same, tvs)); + } + for (id, ts) in stis.into_iter() { + sv.cur_trait().supertraits.get_mut(&id).unwrap().extend(ts); + } - drop(sv); - tracing::debug!("after exports, ncur_needs_vars is {:?}", needs_vars); - for v in needs_vars { - let id = s.noff_var_id(v); - s.cur_trait().tvs.insert(id, (Some(v), TokenStream::new())); - } + drop(sv); + tracing::debug!("after exports, ncur_needs_vars is {:?}", needs_vars); + for v in needs_vars { + let id = s.noff_var_id(v); + s.cur_trait().tvs.insert(id, (Some(v), TokenStream::new())); + } - s.cur_trait().items.extend(quote! { #(#exports)* }); + s.cur_trait().items.extend(quote! { #(#exports)* }); + }); } /// Emit (via mutating `s`) a set of Rust trait declarations @@ -891,7 +931,6 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com s.adjust_vars(ct.instance.evars.len() as u32); s.import_param_var = Some(format_ident!("I")); - s.is_export = true; let export_name = kebab_to_exports_name(wn.name); *s.bound_vars = ct @@ -912,7 +951,10 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com .collect::>(); s.cur_trait().tvs.insert( format_ident!("I"), - (None, quote! { #import_name + ::core::marker::Send }), + ( + None, + quote! { #import_name + ::core::marker::Send }, + ), ); s.cur_trait().items.extend(quote! { #(#exports)* }); @@ -920,11 +962,11 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com s.cur_trait = None; s.cur_mod().items.extend(quote! { - pub trait #base_name { - type Exports: #export_name; + pub trait #base_name { + type Exports + ::core::marker::Send>: #export_name; // todo: can/should this 'static bound be avoided? // it is important right now because this is closed over in host functions - fn instantiate(self, imports: I) -> Self::Exports; + fn instantiate + ::core::marker::Send + 'static>(self, imports: I) -> Self::Exports; } }); } diff --git a/src/hyperlight_host/tests/wit_test.rs b/src/hyperlight_host/tests/wit_test.rs index ef3f8ff1cc..9838048429 100644 --- a/src/hyperlight_host/tests/wit_test.rs +++ b/src/hyperlight_host/tests/wit_test.rs @@ -16,6 +16,7 @@ limitations under the License. use std::sync::{Arc, Mutex}; +use hyperlight_common::component::{Negative, Positive}; use hyperlight_common::resource::BorrowedResourceGuard; use hyperlight_host::{GuestBinary, MultiUseSandbox, UninitializedSandbox}; use hyperlight_testing::wit_guest_as_pathbuf; @@ -66,7 +67,7 @@ impl Clone for Testvariant { struct Host {} -impl test::wit::Roundtrip for Host { +impl test::wit::Roundtrip for Host { fn roundtrip_bool(&mut self, x: bool) -> bool { x } @@ -230,7 +231,7 @@ impl Drop for TestResource { } } -impl test::wit::host_resource::Testresource for Host { +impl test::wit::host_resource::Testresource for Host { type T = Arc>; fn new(&mut self, x: String, last: char) -> Self::T { TestResource::new(x, last) @@ -262,7 +263,7 @@ impl test::wit::host_resource::Testresource for Host { } } -impl test::wit::HostResource for Host { +impl test::wit::HostResource for Host { fn roundtrip_own(&mut self, owned: Arc>) -> Arc> { owned } @@ -273,7 +274,7 @@ impl test::wit::HostResource for Host { } #[allow(refining_impl_trait)] -impl test::wit::TestImports for Host { +impl test::wit::TestImports for Host { type Roundtrip = Self; fn roundtrip(&mut self) -> &mut Self { self @@ -482,6 +483,7 @@ mod bindgen_test_case_bindings { hyperlight_component_macro::host_bindgen!(wit: "../tests/rust_guests/witguest/bindgen-test-cases"); } mod bindgen_test_cases { + use super::{Negative, Positive}; use crate::bindgen_test_case_bindings::*; #[test] @@ -495,7 +497,7 @@ mod bindgen_test_cases { #[allow(dead_code)] struct ExportHost; - impl test::bindgen_test_cases::Executor for ExportHost { + impl test::bindgen_test_cases::Executor for ExportHost { fn execute(&mut self) -> test::bindgen_test_cases::executor::ExecutionResult { test::bindgen_test_cases::executor::ExecutionResult { message: String::from("executed"), @@ -503,7 +505,7 @@ mod bindgen_test_cases { } } - impl test::bindgen_test_cases::Types for ExportHost { + impl test::bindgen_test_cases::Types for ExportHost { fn get_status(&mut self) -> test::bindgen_test_cases::types::Status { test::bindgen_test_cases::types::Status { message: String::from("ok"), @@ -511,8 +513,11 @@ mod bindgen_test_cases { } } - impl test::bindgen_test_cases::UsesExportedTypes - for ExportHost + impl + test::bindgen_test_cases::UsesExportedTypes< + Positive, + test::bindgen_test_cases::types::Status, + > for ExportHost { fn get_status(&mut self) -> test::bindgen_test_cases::types::Status { test::bindgen_test_cases::types::Status { @@ -522,8 +527,8 @@ mod bindgen_test_cases { } #[allow(refining_impl_trait)] - impl - test::bindgen_test_cases::BindgenTestCasesExports for ExportHost + impl + Send> + test::bindgen_test_cases::BindgenTestCasesExports for ExportHost { type Executor = Self; fn executor(&mut self) -> &mut Self { @@ -592,3 +597,23 @@ mod deeply_nested { "#, }); } + +mod imports_exports_same_interface_with_host_resource { + hyperlight_component_macro::host_bindgen!({ + inline: r#" + (component + (type (export "world") (component + (export "test:rie/world" (component + (import "test:rie/definer" (instance $DI + (export "r" (type $R (sub resource))) + (export "f" (func (param "r" (own $R)))))) + (alias export $DI "r" (type $R)) + (import "test:rie/user" (instance + (export "r2" (type $R2 (eq $R))) + (export "g" (func (param "r" (borrow $R2)))))) + (export "test:rie/user" (instance + (export "r2" (type $R2 (eq $R))) + (export "g" (func (param "r" (borrow $R2))))))))))) + "#, + }); +} diff --git a/src/tests/rust_guests/witguest/src/main.rs b/src/tests/rust_guests/witguest/src/main.rs index 8fcf5a4e57..37ad9ca89c 100644 --- a/src/tests/rust_guests/witguest/src/main.rs +++ b/src/tests/rust_guests/witguest/src/main.rs @@ -18,6 +18,7 @@ limitations under the License. #![no_main] extern crate alloc; +extern crate hyperlight_common; extern crate hyperlight_guest; use alloc::string::String; @@ -26,12 +27,13 @@ use spin::Mutex; mod bindings; use bindings::*; +use hyperlight_common::component::{Negative, Positive}; struct Guest { - host_resource: Option<::T>, + host_resource: Option<>::T>, } -impl test::wit::Roundtrip for Guest { +impl test::wit::Roundtrip for Guest { fn roundtrip_bool(&mut self, x: bool) -> bool { (Host {}).roundtrip_bool(x) } @@ -169,43 +171,43 @@ impl test::wit::Roundtrip for Guest { use alloc::string::ToString; use test::wit::host_resource::Testresource; -impl test::wit::TestHostResource<::T> for Guest { +impl test::wit::TestHostResource>::T> for Guest { fn test_uses_locally(&mut self) -> bool { let mut host = Host {}; - let r = ::new(&mut host, "str".to_string(), 'z'); - ::append_char(&mut host, &r, 'a'); - ::append_char(&mut host, &r, 'b'); - let r = ::roundtrip_own(&mut host, r); - let r = ::roundtrip_own(&mut host, r); - ::append_char(&mut host, &r, 'c'); - ::return_own(&mut host, r); + let r = >::new(&mut host, "str".to_string(), 'z'); + >::append_char(&mut host, &r, 'a'); + >::append_char(&mut host, &r, 'b'); + let r = >::roundtrip_own(&mut host, r); + let r = >::roundtrip_own(&mut host, r); + >::append_char(&mut host, &r, 'c'); + >::return_own(&mut host, r); true } - fn test_makes(&mut self) -> ::T { + fn test_makes(&mut self) -> >::T { let mut host = Host {}; - ::new(&mut host, "str".to_string(), 'z') + >::new(&mut host, "str".to_string(), 'z') } - fn test_accepts_borrow(&mut self, r: &::T) { + fn test_accepts_borrow(&mut self, r: &>::T) { let mut host = Host {}; - ::append_char(&mut host, r, 'a'); + >::append_char(&mut host, r, 'a'); } - fn test_accepts_own(&mut self, r: ::T) { + fn test_accepts_own(&mut self, r: >::T) { let mut host = Host {}; // TODO: add test about the old contents of this being // dropped, when #810 is fixed. - ::append_char(&mut host, &r, 'b'); + >::append_char(&mut host, &r, 'b'); self.host_resource = Some(r); } - fn test_returns(&mut self) -> ::T { + fn test_returns(&mut self) -> >::T { let mut host = Host {}; let r = self.host_resource.take().unwrap(); - ::append_char(&mut host, &r, 'c'); + >::append_char(&mut host, &r, 'c'); r } } #[allow(refining_impl_trait)] -impl test::wit::TestExports for Guest { +impl test::wit::TestExports for Guest { type Roundtrip = Self; fn roundtrip(&mut self) -> &mut Self { self @@ -266,3 +268,23 @@ mod deeply_nested { "#, }); } + +mod imports_exports_same_interface_with_host_resource { + hyperlight_component_macro::guest_bindgen!({ + inline: r#" + (component + (type (export "world") (component + (export "test:rie/world" (component + (import "test:rie/definer" (instance $DI + (export "r" (type $R (sub resource))) + (export "f" (func (param "r" (own $R)))))) + (alias export $DI "r" (type $R)) + (import "test:rie/user" (instance + (export "r2" (type $R2 (eq $R))) + (export "g" (func (param "r" (borrow $R2)))))) + (export "test:rie/user" (instance + (export "r2" (type $R2 (eq $R))) + (export "g" (func (param "r" (borrow $R2))))))))))) + "#, + }); +}