From 5d7fe00c1e1ce5fb7e4687dabb1d4884e68eebba Mon Sep 17 00:00:00 2001 From: Joshua Dutton Date: Thu, 27 Jul 2023 17:28:13 -0400 Subject: [PATCH 01/12] register constructor --- crates/rune-macros/src/any.rs | 26 +++++++++- crates/rune-macros/src/context.rs | 4 ++ crates/rune/src/compile/context_error.rs | 9 ++++ crates/rune/src/module.rs | 21 ++++++++ crates/rune/src/module/module.rs | 3 ++ crates/rune/src/query/query.rs | 3 ++ examples/examples/external_struct.rs | 63 ++++++++++++++++++++++++ 7 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 examples/examples/external_struct.rs diff --git a/crates/rune-macros/src/any.rs b/crates/rune-macros/src/any.rs index 4f2bb86d5..303374e8b 100644 --- a/crates/rune-macros/src/any.rs +++ b/crates/rune-macros/src/any.rs @@ -138,7 +138,7 @@ pub(crate) fn expand_install_with( match &input.data { syn::Data::Struct(st) => { - expand_struct_install_with(cx, installers, st, tokens, attr)?; + expand_struct_install_with(cx, installers, ident, st, tokens, attr, generics)?; } syn::Data::Enum(en) => { expand_enum_install_with(cx, installers, ident, en, tokens, attr, generics)?; @@ -164,9 +164,11 @@ pub(crate) fn expand_install_with( fn expand_struct_install_with( cx: &Context, installers: &mut Vec, + ident: &syn::Ident, st: &syn::DataStruct, tokens: &Tokens, attr: &TypeAttr, + generics: &syn::Generics, ) -> Result<(), ()> { for (n, field) in st.fields.iter().enumerate() { let attrs = cx.field_attrs(&field.attrs)?; @@ -217,13 +219,33 @@ fn expand_struct_install_with( match &st.fields { syn::Fields::Named(fields) => { + let constructor = if attr.constructor { + let args = fields.named.iter().map(|f| { + let ident = f.ident.as_ref().expect("ident"); + let typ = &f.ty; + quote!(#ident: #typ) + }); + + let fields = fields.named.iter().map(|f| f.ident.as_ref()); + + Some(quote!(|#(#args),*| { + #ident { + #(#fields),* + } + })) + } else { + None + }; + let fields = fields.named.iter().flat_map(|f| { let ident = f.ident.as_ref()?; Some(syn::LitStr::new(&ident.to_string(), ident.span())) }); + let constructor = constructor.as_ref().map(|c| quote!(.constructor(#c)?)); + installers.push(quote! { - module.type_meta::()?.make_named_struct(&[#(#fields,)*])?.static_docs(&#docs); + module.type_meta::()?.make_named_struct(&[#(#fields,)*])?#constructor.static_docs(&#docs); }); } syn::Fields::Unnamed(fields) => { diff --git a/crates/rune-macros/src/context.rs b/crates/rune-macros/src/context.rs index b41dc8733..df95614c9 100644 --- a/crates/rune-macros/src/context.rs +++ b/crates/rune-macros/src/context.rs @@ -72,6 +72,8 @@ pub(crate) struct TypeAttr { pub(crate) parse: ParseKind, /// `#[rune(item = )]`. pub(crate) item: Option, + /// `#[rune(constructible)]`. + pub(crate) constructor: bool, /// Parsed documentation. pub(crate) docs: Vec, } @@ -441,6 +443,8 @@ impl Context { // Parse `#[rune(install_with = )]` meta.input.parse::()?; attr.install_with = Some(parse_path_compat(meta.input)?); + } else if meta.path == CONSTRUCTOR { + attr.constructor = true; } else { return Err(syn::Error::new_spanned( &meta.path, diff --git a/crates/rune/src/compile/context_error.rs b/crates/rune/src/compile/context_error.rs index 10e64bccd..6f6f4cb06 100644 --- a/crates/rune/src/compile/context_error.rs +++ b/crates/rune/src/compile/context_error.rs @@ -77,6 +77,9 @@ pub enum ContextError { ConflictingVariant { item: ItemBuf, }, + ConstructorConflict { + type_info: TypeInfo, + }, ValueError { error: VmError, }, @@ -196,6 +199,12 @@ impl fmt::Display for ContextError { ContextError::ConflictingVariant { item } => { write!(f, "Variant with `{item}` already exists")?; } + ContextError::ConstructorConflict { type_info } => { + write!( + f, + "Constructor for type `{type_info}` has already been registered" + )?; + } ContextError::ValueError { error } => { write!(f, "Error when converting to constant value: {error}")?; } diff --git a/crates/rune/src/module.rs b/crates/rune/src/module.rs index 8e76d9efc..a89dc85eb 100644 --- a/crates/rune/src/module.rs +++ b/crates/rune/src/module.rs @@ -116,6 +116,8 @@ pub(crate) struct ModuleType { pub(crate) type_info: TypeInfo, /// The specification for the type. pub(crate) spec: Option, + /// Handler to use if this type can be constructed through a regular function call. + pub(crate) constructor: Option>, /// Documentation for the type. pub(crate) docs: Docs, } @@ -485,6 +487,7 @@ where { docs: &'a mut Docs, spec: &'a mut Option, + constructor: &'a mut Option>, item: &'a Item, _marker: PhantomData<&'a mut T>, } @@ -555,6 +558,24 @@ where }) } + /// Register a constructor method for the current type. + pub fn constructor(self, constructor: F) -> Result + where + F: Function, + { + if self.constructor.is_some() { + return Err(ContextError::ConstructorConflict { + type_info: T::type_info(), + }); + } + + *self.constructor = Some(Arc::new(move |stack, args| { + constructor.fn_call(stack, args) + })); + + Ok(self) + } + fn make_struct(self, fields: Fields) -> Result { let old = self.spec.replace(TypeSpecification::Struct(fields)); diff --git a/crates/rune/src/module/module.rs b/crates/rune/src/module/module.rs index 7af830558..b5519e497 100644 --- a/crates/rune/src/module/module.rs +++ b/crates/rune/src/module/module.rs @@ -197,6 +197,7 @@ impl Module { type_parameters, type_info, spec: None, + constructor: None, docs: Docs::EMPTY, }); @@ -207,6 +208,7 @@ impl Module { Ok(TypeMut { docs: &mut ty.docs, spec: &mut ty.spec, + constructor: &mut ty.constructor, item: &ty.item, _marker: PhantomData, }) @@ -229,6 +231,7 @@ impl Module { Ok(TypeMut { docs: &mut ty.docs, spec: &mut ty.spec, + constructor: &mut ty.constructor, item: &ty.item, _marker: PhantomData, }) diff --git a/crates/rune/src/query/query.rs b/crates/rune/src/query/query.rs index cf6a6f2e1..f0de1bbaf 100644 --- a/crates/rune/src/query/query.rs +++ b/crates/rune/src/query/query.rs @@ -337,6 +337,9 @@ impl<'a, 'arena> Query<'a, 'arena> { parameters, }; + self.unit + .insert_meta(location.as_spanned(), &meta, self.pool, self.inner)?; + self.insert_meta(meta.clone()) .with_span(location.as_spanned())?; diff --git a/examples/examples/external_struct.rs b/examples/examples/external_struct.rs new file mode 100644 index 000000000..81ac422b6 --- /dev/null +++ b/examples/examples/external_struct.rs @@ -0,0 +1,63 @@ +use std::sync::Arc; + +use rune::runtime::Vm; +use rune::termcolor::{ColorChoice, StandardStream}; +use rune::{Any, ContextError, Diagnostics, Module}; + +#[derive(Default, Debug, Any, PartialEq, Eq)] +#[rune(constructor)] +struct External { + #[rune(get, set)] + suite_name: String, + #[rune(get, set)] + room_number: usize, +} + +fn main() -> rune::Result<()> { + let m = module()?; + + let mut context = rune_modules::default_context()?; + context.install(m)?; + let runtime = Arc::new(context.runtime()); + + let mut sources = rune::sources! { + entry => { + pub fn main() { + let external = External { + suite_name: "Fowler", + room_number: 1300, + }; + + external + } + } + }; + + let mut diagnostics = Diagnostics::new(); + + let result = rune::prepare(&mut sources) + .with_context(&context) + .with_diagnostics(&mut diagnostics) + .build(); + + if !diagnostics.is_empty() { + let mut writer = StandardStream::stderr(ColorChoice::Always); + diagnostics.emit(&mut writer, &sources)?; + } + + let unit = result?; + + let mut vm = Vm::new(runtime, Arc::new(unit)); + + let output = vm.call(["main"], ())?; + //let output: External = rune::from_value(output)?; + println!("{:?}", output); + + Ok(()) +} + +pub fn module() -> Result { + let mut module = Module::new(); + module.ty::()?; + Ok(module) +} From 47c47a95b55d07ba3775f9ba07e3cfd7144ec055 Mon Sep 17 00:00:00 2001 From: Joshua Dutton Date: Thu, 27 Jul 2023 17:58:02 -0400 Subject: [PATCH 02/12] call constructor when available --- crates/rune-macros/src/any.rs | 40 ++++++++++----------- crates/rune-macros/src/context.rs | 2 +- crates/rune/src/compile/context.rs | 49 ++++++++++++++++++++------ crates/rune/src/compile/v1/assemble.rs | 3 ++ crates/rune/src/hir/hir.rs | 1 + crates/rune/src/hir/lowering.rs | 10 +++++- examples/examples/external_struct.rs | 2 +- 7 files changed, 72 insertions(+), 35 deletions(-) diff --git a/crates/rune-macros/src/any.rs b/crates/rune-macros/src/any.rs index 303374e8b..aee918616 100644 --- a/crates/rune-macros/src/any.rs +++ b/crates/rune-macros/src/any.rs @@ -138,7 +138,7 @@ pub(crate) fn expand_install_with( match &input.data { syn::Data::Struct(st) => { - expand_struct_install_with(cx, installers, ident, st, tokens, attr, generics)?; + expand_struct_install_with(cx, installers, ident, st, tokens, attr)?; } syn::Data::Enum(en) => { expand_enum_install_with(cx, installers, ident, en, tokens, attr, generics)?; @@ -168,7 +168,6 @@ fn expand_struct_install_with( st: &syn::DataStruct, tokens: &Tokens, attr: &TypeAttr, - generics: &syn::Generics, ) -> Result<(), ()> { for (n, field) in st.fields.iter().enumerate() { let attrs = cx.field_attrs(&field.attrs)?; @@ -219,31 +218,30 @@ fn expand_struct_install_with( match &st.fields { syn::Fields::Named(fields) => { - let constructor = if attr.constructor { - let args = fields.named.iter().map(|f| { - let ident = f.ident.as_ref().expect("ident"); - let typ = &f.ty; - quote!(#ident: #typ) - }); - - let fields = fields.named.iter().map(|f| f.ident.as_ref()); - - Some(quote!(|#(#args),*| { - #ident { - #(#fields),* - } - })) - } else { - None - }; + let constructor = attr + .constructor + .then(|| { + let args = fields.named.iter().map(|f| { + let ident = f.ident.as_ref().expect("named fields must have an Ident"); + let typ = &f.ty; + quote!(#ident: #typ) + }); + + let field_names = fields.named.iter().map(|f| f.ident.as_ref()); + + quote!(|#(#args),*| { + #ident { + #(#field_names),* + } + }) + }) + .map(|c| quote!(.constructor(#c)?)); let fields = fields.named.iter().flat_map(|f| { let ident = f.ident.as_ref()?; Some(syn::LitStr::new(&ident.to_string(), ident.span())) }); - let constructor = constructor.as_ref().map(|c| quote!(.constructor(#c)?)); - installers.push(quote! { module.type_meta::()?.make_named_struct(&[#(#fields,)*])?#constructor.static_docs(&#docs); }); diff --git a/crates/rune-macros/src/context.rs b/crates/rune-macros/src/context.rs index df95614c9..317c006e5 100644 --- a/crates/rune-macros/src/context.rs +++ b/crates/rune-macros/src/context.rs @@ -72,7 +72,7 @@ pub(crate) struct TypeAttr { pub(crate) parse: ParseKind, /// `#[rune(item = )]`. pub(crate) item: Option, - /// `#[rune(constructible)]`. + /// `#[rune(constructor)]`. pub(crate) constructor: bool, /// Parsed documentation. pub(crate) docs: Vec, diff --git a/crates/rune/src/compile/context.rs b/crates/rune/src/compile/context.rs index 4b099daa4..287e76b49 100644 --- a/crates/rune/src/compile/context.rs +++ b/crates/rune/src/compile/context.rs @@ -429,17 +429,44 @@ impl Context { let kind = if let Some(spec) = &ty.spec { match spec { - TypeSpecification::Struct(fields) => meta::Kind::Struct { - fields: match fields { - Fields::Named(fields) => meta::Fields::Named(meta::FieldsNamed { - fields: fields.iter().copied().map(Box::::from).collect(), - }), - Fields::Unnamed(args) => meta::Fields::Unnamed(*args), - Fields::Empty => meta::Fields::Empty, - }, - constructor: None, - parameters, - }, + TypeSpecification::Struct(fields) => { + let constructor = match &ty.constructor { + Some(c) => { + let hash = Hash::type_hash(&item); + + let signature = meta::Signature { + #[cfg(feature = "doc")] + is_async: false, + #[cfg(feature = "doc")] + args: Some(match fields { + Fields::Named(names) => names.len(), + Fields::Unnamed(args) => *args, + Fields::Empty => 0, + }), + #[cfg(feature = "doc")] + return_type: Some(ty.hash), + #[cfg(feature = "doc")] + argument_types: Box::from([]), + }; + + self.insert_native_fn(hash, c)?; + Some(signature) + } + None => None, + }; + + meta::Kind::Struct { + fields: match fields { + Fields::Named(fields) => meta::Fields::Named(meta::FieldsNamed { + fields: fields.iter().copied().map(Box::::from).collect(), + }), + Fields::Unnamed(args) => meta::Fields::Unnamed(*args), + Fields::Empty => meta::Fields::Empty, + }, + constructor, + parameters, + } + } TypeSpecification::Enum(en) => { for (index, variant) in en.variants.iter().enumerate() { let Some(fields) = &variant.fields else { diff --git a/crates/rune/src/compile/v1/assemble.rs b/crates/rune/src/compile/v1/assemble.rs index 284d395d8..9a2f2351f 100644 --- a/crates/rune/src/compile/v1/assemble.rs +++ b/crates/rune/src/compile/v1/assemble.rs @@ -1976,6 +1976,9 @@ fn expr_object<'hir>( hir::ExprObjectKind::StructVariant { hash } => { cx.asm.push(Inst::StructVariant { hash, slot }, span); } + hir::ExprObjectKind::Constructor { hash, args } => { + cx.asm.push(Inst::Call { hash, args }, span); + } hir::ExprObjectKind::Anonymous => { cx.asm.push(Inst::Object { slot }, span); } diff --git a/crates/rune/src/hir/hir.rs b/crates/rune/src/hir/hir.rs index 41072ae40..1943a53e7 100644 --- a/crates/rune/src/hir/hir.rs +++ b/crates/rune/src/hir/hir.rs @@ -580,6 +580,7 @@ pub(crate) enum ExprObjectKind { UnitStruct { hash: Hash }, Struct { hash: Hash }, StructVariant { hash: Hash }, + Constructor { hash: Hash, args: usize }, Anonymous, } diff --git a/crates/rune/src/hir/lowering.rs b/crates/rune/src/hir/lowering.rs index db8ceeb3e..89a9ad4e7 100644 --- a/crates/rune/src/hir/lowering.rs +++ b/crates/rune/src/hir/lowering.rs @@ -410,10 +410,18 @@ pub(crate) fn expr_object<'hir>( } meta::Kind::Struct { fields: meta::Fields::Named(st), + constructor, .. } => { check_object_fields(&st.fields, item)?; - hir::ExprObjectKind::Struct { hash: meta.hash } + + match constructor { + Some(_) => hir::ExprObjectKind::Constructor { + hash: meta.hash, + args: st.fields.len(), + }, + None => hir::ExprObjectKind::Struct { hash: meta.hash }, + } } meta::Kind::Variant { fields: meta::Fields::Named(st), diff --git a/examples/examples/external_struct.rs b/examples/examples/external_struct.rs index 81ac422b6..a22437301 100644 --- a/examples/examples/external_struct.rs +++ b/examples/examples/external_struct.rs @@ -50,7 +50,7 @@ fn main() -> rune::Result<()> { let mut vm = Vm::new(runtime, Arc::new(unit)); let output = vm.call(["main"], ())?; - //let output: External = rune::from_value(output)?; + let output: External = rune::from_value(output)?; println!("{:?}", output); Ok(()) From 4fde80e772d06e13a8161bc835dfe01ab9fa7d6c Mon Sep 17 00:00:00 2001 From: Josh Dutton Date: Sun, 30 Jul 2023 10:04:35 -0400 Subject: [PATCH 03/12] reorder fields according to type definition --- crates/rune/src/compile/context.rs | 26 ++++++++++++++++--- crates/rune/src/compile/meta.rs | 11 ++++++-- crates/rune/src/compile/v1/assemble.rs | 31 +++++++++++++++++++++- crates/rune/src/hir/hir.rs | 2 ++ crates/rune/src/hir/lowering.rs | 36 +++++++++++++++----------- crates/rune/src/query/query.rs | 11 +++----- crates/rune/src/runtime/inst.rs | 9 +++++++ crates/rune/src/runtime/stack.rs | 8 ++++++ crates/rune/src/runtime/unit.rs | 4 +-- crates/rune/src/runtime/vm.rs | 10 +++++++ examples/examples/external_enum.rs | 1 + examples/examples/external_struct.rs | 9 +++++++ 12 files changed, 128 insertions(+), 30 deletions(-) diff --git a/crates/rune/src/compile/context.rs b/crates/rune/src/compile/context.rs index 287e76b49..6bf4e6db6 100644 --- a/crates/rune/src/compile/context.rs +++ b/crates/rune/src/compile/context.rs @@ -458,7 +458,14 @@ impl Context { meta::Kind::Struct { fields: match fields { Fields::Named(fields) => meta::Fields::Named(meta::FieldsNamed { - fields: fields.iter().copied().map(Box::::from).collect(), + fields: fields + .iter() + .copied() + .enumerate() + .map(|(position, name)| { + (Box::::from(name), meta::FieldMeta { position }) + }) + .collect(), }), Fields::Unnamed(args) => meta::Fields::Unnamed(*args), Fields::Empty => meta::Fields::Empty, @@ -522,7 +529,13 @@ impl Context { fields: names .iter() .copied() - .map(Box::::from) + .enumerate() + .map(|(position, name)| { + ( + Box::::from(name), + meta::FieldMeta { position }, + ) + }) .collect(), }) } @@ -901,7 +914,14 @@ impl Context { index, fields: match fields { Fields::Named(fields) => meta::Fields::Named(meta::FieldsNamed { - fields: fields.iter().copied().map(Box::::from).collect(), + fields: fields + .iter() + .copied() + .enumerate() + .map(|(position, name)| { + (Box::::from(name), meta::FieldMeta { position }) + }) + .collect(), }), Fields::Unnamed(args) => meta::Fields::Unnamed(*args), Fields::Empty => meta::Fields::Empty, diff --git a/crates/rune/src/compile/meta.rs b/crates/rune/src/compile/meta.rs index d7dc8a913..ad540c00c 100644 --- a/crates/rune/src/compile/meta.rs +++ b/crates/rune/src/compile/meta.rs @@ -3,7 +3,7 @@ use core::fmt; use crate::no_std::borrow::Cow; -use crate::no_std::collections::HashSet; +use crate::no_std::collections::HashMap; use crate::no_std::path::Path; use crate::no_std::prelude::*; @@ -287,7 +287,14 @@ pub struct Import { #[non_exhaustive] pub struct FieldsNamed { /// Fields associated with the type. - pub(crate) fields: HashSet>, + pub(crate) fields: HashMap, FieldMeta>, +} + +/// Metadata for a single named field. +#[derive(Debug, Clone)] +pub struct FieldMeta { + /// Position of the field in its containing type declaration. + pub(crate) position: usize, } /// Item and the module that the item belongs to. diff --git a/crates/rune/src/compile/v1/assemble.rs b/crates/rune/src/compile/v1/assemble.rs index 9a2f2351f..21d0b0a13 100644 --- a/crates/rune/src/compile/v1/assemble.rs +++ b/crates/rune/src/compile/v1/assemble.rs @@ -1957,9 +1957,38 @@ fn expr_object<'hir>( ) -> compile::Result> { let guard = cx.scopes.child(span)?; - for assign in hir.assignments { + let mut order = Vec::with_capacity(hir.assignments.len()); + + for assign in hir.assignments.iter() { expr(cx, &assign.assign, Needs::Value)?.apply(cx)?; cx.scopes.alloc(&span)?; + + let position = assign.position.ok_or(compile::Error::msg( + span, + format!("missing position for field assignment {}", assign.key.1), + ))?; + + order.push(position); + } + + for stack_position in 0..hir.assignments.len() { + loop { + let desired_position = order[stack_position]; + + if stack_position == desired_position { + break; + } + + order.swap(stack_position, desired_position); + + cx.asm.push( + Inst::Swap { + a: stack_position, + b: desired_position, + }, + span, + ); + } } let slot = diff --git a/crates/rune/src/hir/hir.rs b/crates/rune/src/hir/hir.rs index 1943a53e7..aac1f075a 100644 --- a/crates/rune/src/hir/hir.rs +++ b/crates/rune/src/hir/hir.rs @@ -602,6 +602,8 @@ pub(crate) struct FieldAssign<'hir> { pub(crate) key: (Span, &'hir str), /// The assigned expression of the field. pub(crate) assign: Expr<'hir>, + /// The position of the field in its containing type declaration. + pub(crate) position: Option, } /// A literal vector. diff --git a/crates/rune/src/hir/lowering.rs b/crates/rune/src/hir/lowering.rs index 89a9ad4e7..f0fd0d6c3 100644 --- a/crates/rune/src/hir/lowering.rs +++ b/crates/rune/src/hir/lowering.rs @@ -330,7 +330,7 @@ pub(crate) fn expr_object<'hir>( let span = ast; let mut keys_dup = HashMap::new(); - let assignments = &*iter!(&ast.assignments, |(ast, _)| { + let assignments = &mut *iter!(&ast.assignments, |(ast, _)| { let key = object_key(cx, &ast.key)?; if let Some(existing) = keys_dup.insert(key.1, key.0) { @@ -362,25 +362,31 @@ pub(crate) fn expr_object<'hir>( hir::FieldAssign { key: (key.0.span(), key.1), assign, + position: None, } }); - let check_object_fields = |fields: &HashSet<_>, item: &Item| { + let mut check_object_fields = |fields: &HashMap<_, meta::FieldMeta>, item: &Item| { let mut fields = fields.clone(); - for assign in assignments { - if !fields.remove(assign.key.1) { - return Err(compile::Error::new( - assign.key.0, - ErrorKind::LitObjectNotField { - field: assign.key.1.into(), - item: item.to_owned(), - }, - )); - } + for assign in assignments.iter_mut() { + match fields.remove(assign.key.1) { + Some(field_meta) => { + assign.position = Some(field_meta.position); + } + None => { + return Err(compile::Error::new( + assign.key.0, + ErrorKind::LitObjectNotField { + field: assign.key.1.into(), + item: item.to_owned(), + }, + )); + } + }; } - if let Some(field) = fields.into_iter().next() { + if let Some(field) = fields.into_keys().next() { return Err(compile::Error::new( span, ErrorKind::LitObjectMissingField { @@ -405,7 +411,7 @@ pub(crate) fn expr_object<'hir>( fields: meta::Fields::Empty, .. } => { - check_object_fields(&HashSet::new(), item)?; + check_object_fields(&HashMap::new(), item)?; hir::ExprObjectKind::UnitStruct { hash: meta.hash } } meta::Kind::Struct { @@ -1175,7 +1181,7 @@ fn pat<'hir>(cx: &mut Ctxt<'hir, '_, '_>, ast: &ast::Pat) -> compile::Result = st.fields.keys().cloned().collect(); for binding in bindings.iter() { if !fields.remove(binding.key()) { diff --git a/crates/rune/src/query/query.rs b/crates/rune/src/query/query.rs index f0de1bbaf..594274e44 100644 --- a/crates/rune/src/query/query.rs +++ b/crates/rune/src/query/query.rs @@ -9,7 +9,7 @@ use crate::no_std::sync::Arc; use crate::ast::{Span, Spanned}; use crate::compile::context::ContextMeta; use crate::compile::ir; -use crate::compile::meta; +use crate::compile::meta::{self, FieldMeta}; use crate::compile::{ self, CompileVisitor, ComponentRef, Doc, DynLocation, ErrorKind, ImportStep, IntoComponent, Item, ItemBuf, ItemId, ItemMeta, Located, Location, ModId, ModMeta, Names, Pool, Prelude, @@ -337,9 +337,6 @@ impl<'a, 'arena> Query<'a, 'arena> { parameters, }; - self.unit - .insert_meta(location.as_spanned(), &meta, self.pool, self.inner)?; - self.insert_meta(meta.clone()) .with_span(location.as_spanned())?; @@ -1193,11 +1190,11 @@ impl<'a, 'arena> Query<'a, 'arena> { ast::Fields::Empty => meta::Fields::Empty, ast::Fields::Unnamed(tuple) => meta::Fields::Unnamed(tuple.len()), ast::Fields::Named(st) => { - let mut fields = HashSet::new(); + let mut fields = HashMap::with_capacity(st.len()); - for (ast::Field { name, .. }, _) in st { + for (position, (ast::Field { name, .. }, _)) in st.iter().enumerate() { let name = name.resolve(cx)?; - fields.insert(name.into()); + fields.insert(name.into(), FieldMeta { position }); } meta::Fields::Named(meta::FieldsNamed { fields }) diff --git a/crates/rune/src/runtime/inst.rs b/crates/rune/src/runtime/inst.rs index 4ca5e7049..a94fe803a 100644 --- a/crates/rune/src/runtime/inst.rs +++ b/crates/rune/src/runtime/inst.rs @@ -452,6 +452,15 @@ pub enum Inst { /// Offset to swap value from. offset: usize, }, + /// Swap two values on the stack using their offsets relative to the bottom + /// of the stack. + #[musli(packed)] + Swap { + /// Offset to the first value. + a: usize, + /// Offset to the second value. + b: usize, + }, /// Pop the current stack frame and restore the instruction pointer from it. /// /// The stack frame will be cleared, and the value on the top of the stack diff --git a/crates/rune/src/runtime/stack.rs b/crates/rune/src/runtime/stack.rs index d18821e07..8d090e9c8 100644 --- a/crates/rune/src/runtime/stack.rs +++ b/crates/rune/src/runtime/stack.rs @@ -323,6 +323,14 @@ impl Stack { Ok(self.drain(count)?.collect::>()) } + /// Swap the value at position a with the value at position b. + pub(crate) fn swap(&mut self, a: usize, b: usize) -> Result<(), StackError> { + let a = self.stack_bottom.checked_add(a).ok_or(StackError)?; + let b = self.stack_bottom.checked_add(b).ok_or(StackError)?; + self.stack.swap(a, b); + Ok(()) + } + /// Modify stack top by subtracting the given count from it while checking /// that it is in bounds of the stack. /// diff --git a/crates/rune/src/runtime/unit.rs b/crates/rune/src/runtime/unit.rs index 9a8dafc8d..8c04bc006 100644 --- a/crates/rune/src/runtime/unit.rs +++ b/crates/rune/src/runtime/unit.rs @@ -184,12 +184,12 @@ impl Unit { .map(|keys| &keys[..]) } - /// Lookup runt-time information for the given type hash. + /// Lookup run-time information for the given type hash. pub(crate) fn lookup_rtti(&self, hash: Hash) -> Option<&Arc> { self.logic.rtti.get(&hash) } - /// Lookup variant runt-time information for the given variant hash. + /// Lookup variant run-time information for the given variant hash. pub(crate) fn lookup_variant_rtti(&self, hash: Hash) -> Option<&Arc> { self.logic.variant_rtti.get(&hash) } diff --git a/crates/rune/src/runtime/vm.rs b/crates/rune/src/runtime/vm.rs index c1307b453..8f6145986 100644 --- a/crates/rune/src/runtime/vm.rs +++ b/crates/rune/src/runtime/vm.rs @@ -1545,6 +1545,13 @@ impl Vm { VmResult::Ok(()) } + /// Swap two values on the stack. + #[cfg_attr(feature = "bench", inline(never))] + fn op_swap(&mut self, a: usize, b: usize) -> VmResult<()> { + vm_try!(self.stack.swap(a, b)); + VmResult::Ok(()) + } + /// Perform a jump operation. #[cfg_attr(feature = "bench", inline(never))] fn op_jump(&mut self, jump: usize) -> VmResult<()> { @@ -3048,6 +3055,9 @@ impl Vm { Inst::Dup => { vm_try!(self.op_dup()); } + Inst::Swap { a, b: to } => { + vm_try!(self.op_swap(a, to)); + } Inst::Replace { offset } => { vm_try!(self.op_replace(offset)); } diff --git a/examples/examples/external_enum.rs b/examples/examples/external_enum.rs index 3f34542dd..7fc49a978 100644 --- a/examples/examples/external_enum.rs +++ b/examples/examples/external_enum.rs @@ -63,6 +63,7 @@ fn main() -> rune::Result<()> { println!("{:?}", output); let output = vm.call(["main"], (External::Second(42, 12345),))?; + dbg!(&output.type_info().into_result().unwrap()); let output: External = rune::from_value(output)?; println!("{:?}", output); diff --git a/examples/examples/external_struct.rs b/examples/examples/external_struct.rs index a22437301..6a0f41b33 100644 --- a/examples/examples/external_struct.rs +++ b/examples/examples/external_struct.rs @@ -11,6 +11,12 @@ struct External { suite_name: String, #[rune(get, set)] room_number: usize, + #[rune(get, set)] + is_reserved: bool, + #[rune(get, set)] + num_beds: usize, + #[rune(get, set)] + num_baths: usize, } fn main() -> rune::Result<()> { @@ -24,6 +30,9 @@ fn main() -> rune::Result<()> { entry => { pub fn main() { let external = External { + is_reserved: true, + num_baths: 3, + num_beds: 2, suite_name: "Fowler", room_number: 1300, }; From f55572a8b5c4659c8d9b612da908dd4ff84704d3 Mon Sep 17 00:00:00 2001 From: Josh Dutton Date: Sun, 30 Jul 2023 10:31:11 -0400 Subject: [PATCH 04/12] only reorder before constructor function --- crates/rune/src/compile/v1/assemble.rs | 74 ++++++++++++++++---------- crates/rune/src/runtime/vm.rs | 4 +- 2 files changed, 47 insertions(+), 31 deletions(-) diff --git a/crates/rune/src/compile/v1/assemble.rs b/crates/rune/src/compile/v1/assemble.rs index 21d0b0a13..f4be3c903 100644 --- a/crates/rune/src/compile/v1/assemble.rs +++ b/crates/rune/src/compile/v1/assemble.rs @@ -1957,38 +1957,9 @@ fn expr_object<'hir>( ) -> compile::Result> { let guard = cx.scopes.child(span)?; - let mut order = Vec::with_capacity(hir.assignments.len()); - for assign in hir.assignments.iter() { expr(cx, &assign.assign, Needs::Value)?.apply(cx)?; cx.scopes.alloc(&span)?; - - let position = assign.position.ok_or(compile::Error::msg( - span, - format!("missing position for field assignment {}", assign.key.1), - ))?; - - order.push(position); - } - - for stack_position in 0..hir.assignments.len() { - loop { - let desired_position = order[stack_position]; - - if stack_position == desired_position { - break; - } - - order.swap(stack_position, desired_position); - - cx.asm.push( - Inst::Swap { - a: stack_position, - b: desired_position, - }, - span, - ); - } } let slot = @@ -2006,6 +1977,7 @@ fn expr_object<'hir>( cx.asm.push(Inst::StructVariant { hash, slot }, span); } hir::ExprObjectKind::Constructor { hash, args } => { + reorder_field_assignments(cx, hir, span)?; cx.asm.push(Inst::Call { hash, args }, span); } hir::ExprObjectKind::Anonymous => { @@ -2023,6 +1995,50 @@ fn expr_object<'hir>( Ok(Asm::top(span)) } +/// Reorder the position of the field assignments on the stack so that they +/// match the expected argument order when invoking the constructor function. +fn reorder_field_assignments<'hir>( + cx: &mut Ctxt<'_, 'hir, '_>, + hir: &hir::ExprObject<'hir>, + span: &dyn Spanned, +) -> compile::Result<()> { + let mut order = Vec::with_capacity(hir.assignments.len()); + + for assign in hir.assignments { + match assign.position { + Some(position) => order.push(position), + None => { + return Err(compile::Error::msg( + span, + format!("missing position for field {}", assign.key.1), + )) + } + }; + } + + for stack_position in 0..hir.assignments.len() { + loop { + let desired_position = order[stack_position]; + + if stack_position == desired_position { + break; + } + + order.swap(stack_position, desired_position); + + cx.asm.push( + Inst::Swap { + a: stack_position, + b: desired_position, + }, + span, + ); + } + } + + Ok(()) +} + /// Assemble a range expression. #[instrument(span = span)] fn expr_range<'hir>( diff --git a/crates/rune/src/runtime/vm.rs b/crates/rune/src/runtime/vm.rs index 8f6145986..0de5a674f 100644 --- a/crates/rune/src/runtime/vm.rs +++ b/crates/rune/src/runtime/vm.rs @@ -3055,8 +3055,8 @@ impl Vm { Inst::Dup => { vm_try!(self.op_dup()); } - Inst::Swap { a, b: to } => { - vm_try!(self.op_swap(a, to)); + Inst::Swap { a, b } => { + vm_try!(self.op_swap(a, b)); } Inst::Replace { offset } => { vm_try!(self.op_replace(offset)); From f3948628dfc3feafb6b379fc8bd1858c63862ddd Mon Sep 17 00:00:00 2001 From: Josh Dutton Date: Sun, 30 Jul 2023 10:38:44 -0400 Subject: [PATCH 05/12] avoid non const error path --- crates/rune/src/compile/v1/assemble.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/rune/src/compile/v1/assemble.rs b/crates/rune/src/compile/v1/assemble.rs index f4be3c903..3ad69385f 100644 --- a/crates/rune/src/compile/v1/assemble.rs +++ b/crates/rune/src/compile/v1/assemble.rs @@ -2005,15 +2005,14 @@ fn reorder_field_assignments<'hir>( let mut order = Vec::with_capacity(hir.assignments.len()); for assign in hir.assignments { - match assign.position { - Some(position) => order.push(position), - None => { - return Err(compile::Error::msg( - span, - format!("missing position for field {}", assign.key.1), - )) - } + let Some(position) = assign.position else { + return Err(compile::Error::msg( + span, + format_args!("Missing position for field assignment {}", assign.key.1), + )); }; + + order.push(position); } for stack_position in 0..hir.assignments.len() { From de7a594df553c06e313558e61cd3c809e54ab2f1 Mon Sep 17 00:00:00 2001 From: Josh Dutton Date: Sun, 30 Jul 2023 13:26:22 -0400 Subject: [PATCH 06/12] use offset from top of stack --- crates/rune/src/compile/v1/assemble.rs | 21 +++- crates/rune/src/runtime/inst.rs | 2 +- crates/rune/src/runtime/stack.rs | 4 +- crates/rune/src/tests/external_constructor.rs | 115 ++++++++++++++++++ examples/examples/external_struct.rs | 53 +++++--- 5 files changed, 166 insertions(+), 29 deletions(-) diff --git a/crates/rune/src/compile/v1/assemble.rs b/crates/rune/src/compile/v1/assemble.rs index 3ad69385f..ec667991e 100644 --- a/crates/rune/src/compile/v1/assemble.rs +++ b/crates/rune/src/compile/v1/assemble.rs @@ -2015,20 +2015,29 @@ fn reorder_field_assignments<'hir>( order.push(position); } - for stack_position in 0..hir.assignments.len() { + for current_position in 0..hir.assignments.len() { loop { - let desired_position = order[stack_position]; + let desired_position = order[current_position]; - if stack_position == desired_position { + if current_position == desired_position { break; } - order.swap(stack_position, desired_position); + order.swap(current_position, desired_position); + + let current_offset = hir.assignments.len() - current_position; + + let Some(desired_offset) = hir.assignments.len().checked_sub(desired_position) else { + return Err(compile::Error::msg( + span, + format_args!("Wrong number of field assignments: have {} but field position is {}", hir.assignments.len(), desired_position), + )); + }; cx.asm.push( Inst::Swap { - a: stack_position, - b: desired_position, + a: current_offset, + b: desired_offset, }, span, ); diff --git a/crates/rune/src/runtime/inst.rs b/crates/rune/src/runtime/inst.rs index a94fe803a..b497eb930 100644 --- a/crates/rune/src/runtime/inst.rs +++ b/crates/rune/src/runtime/inst.rs @@ -452,7 +452,7 @@ pub enum Inst { /// Offset to swap value from. offset: usize, }, - /// Swap two values on the stack using their offsets relative to the bottom + /// Swap two values on the stack using their offsets relative to the top /// of the stack. #[musli(packed)] Swap { diff --git a/crates/rune/src/runtime/stack.rs b/crates/rune/src/runtime/stack.rs index 8d090e9c8..33974ecd0 100644 --- a/crates/rune/src/runtime/stack.rs +++ b/crates/rune/src/runtime/stack.rs @@ -325,8 +325,8 @@ impl Stack { /// Swap the value at position a with the value at position b. pub(crate) fn swap(&mut self, a: usize, b: usize) -> Result<(), StackError> { - let a = self.stack_bottom.checked_add(a).ok_or(StackError)?; - let b = self.stack_bottom.checked_add(b).ok_or(StackError)?; + let a = self.stack.len().checked_sub(a).ok_or(StackError)?; + let b = self.stack.len().checked_sub(b).ok_or(StackError)?; self.stack.swap(a, b); Ok(()) } diff --git a/crates/rune/src/tests/external_constructor.rs b/crates/rune/src/tests/external_constructor.rs index 5c5d5d68d..02b225d76 100644 --- a/crates/rune/src/tests/external_constructor.rs +++ b/crates/rune/src/tests/external_constructor.rs @@ -76,3 +76,118 @@ fn construct_enum() { let output: Enum = from_value(output).unwrap(); assert_eq!(output, Enum::Output(2 * 3 * 4)); } + +/// Tests pattern matching and constructing over an external struct from within +/// Rune. +#[test] +fn construct_struct() { + #[derive(Debug, Any, PartialEq, Eq)] + #[rune(constructor)] + struct Request { + #[rune(get)] + url: String, + } + + #[derive(Debug, Any, PartialEq, Eq)] + #[rune(constructor)] + struct Response { + #[rune(get, set)] + status_code: u32, + #[rune(get, set)] + body: String, + #[rune(get, set)] + content_type: String, + #[rune(get, set)] + content_length: u32, + } + + fn make_module() -> Result { + let mut module = Module::new(); + module.ty::()?; + module.ty::()?; + Ok(module) + } + + let m = make_module().expect("Module should be buildable"); + + let mut context = Context::new(); + context.install(m).expect("Context should build"); + let runtime = Arc::new(context.runtime()); + + let mut sources = sources! { + entry => { + pub fn main(req) { + let foo = 10; + + let rsp = match req.url { + "/" => Response { + status_code: 200, + body: "ok", + content_type: "text/plain", + content_length: 2, + }, + "/account" => Response { + content_type: "text/plain", + content_length: 12, + body: "unauthorized", + status_code: 401, + }, + _ => Response { + body: "not found", + status_code: 404, + content_length: 9, + content_type: "text/plain", + } + }; + + rsp + } + } + }; + + let unit = prepare(&mut sources) + .with_context(&context) + .build() + .expect("Unit should build"); + + let mut vm = Vm::new(runtime, Arc::new(unit)); + + for (req, rsp) in vec![ + ( + Request { url: "/".into() }, + Response { + status_code: 200, + body: "ok".into(), + content_type: "text/plain".into(), + content_length: 2, + }, + ), + ( + Request { + url: "/account".into(), + }, + Response { + status_code: 401, + body: "unauthorized".into(), + content_type: "text/plain".into(), + content_length: 12, + }, + ), + ( + Request { + url: "/cart".into(), + }, + Response { + status_code: 404, + body: "not found".into(), + content_type: "text/plain".into(), + content_length: 9, + }, + ), + ] { + let output = vm.call(["main"], (req,)).unwrap(); + let output: Response = from_value(output).unwrap(); + assert_eq!(output.status_code, rsp.status_code); + assert_eq!(output.body, rsp.body); + } +} diff --git a/examples/examples/external_struct.rs b/examples/examples/external_struct.rs index 6a0f41b33..045a6ec1d 100644 --- a/examples/examples/external_struct.rs +++ b/examples/examples/external_struct.rs @@ -5,18 +5,18 @@ use rune::termcolor::{ColorChoice, StandardStream}; use rune::{Any, ContextError, Diagnostics, Module}; #[derive(Default, Debug, Any, PartialEq, Eq)] -#[rune(constructor)] -struct External { - #[rune(get, set)] - suite_name: String, - #[rune(get, set)] - room_number: usize, +struct Request { #[rune(get, set)] - is_reserved: bool, + url: String, +} + +#[derive(Default, Debug, Any, PartialEq, Eq)] +#[rune(constructor)] +struct Response { #[rune(get, set)] - num_beds: usize, + status_code: usize, #[rune(get, set)] - num_baths: usize, + body: String, } fn main() -> rune::Result<()> { @@ -28,16 +28,19 @@ fn main() -> rune::Result<()> { let mut sources = rune::sources! { entry => { - pub fn main() { - let external = External { - is_reserved: true, - num_baths: 3, - num_beds: 2, - suite_name: "Fowler", - room_number: 1300, + pub fn main(req) { + let rsp = match req.url { + "/" => Response { + status_code: 200, + body: "ok", + }, + _ => Response { + status_code: 400, + body: "not found", + } }; - external + rsp } } }; @@ -58,8 +61,17 @@ fn main() -> rune::Result<()> { let mut vm = Vm::new(runtime, Arc::new(unit)); - let output = vm.call(["main"], ())?; - let output: External = rune::from_value(output)?; + let output = vm.call(["main"], (Request { url: "/".into() },))?; + let output: Response = rune::from_value(output)?; + println!("{:?}", output); + + let output = vm.call( + ["main"], + (Request { + url: "/account".into(), + },), + )?; + let output: Response = rune::from_value(output)?; println!("{:?}", output); Ok(()) @@ -67,6 +79,7 @@ fn main() -> rune::Result<()> { pub fn module() -> Result { let mut module = Module::new(); - module.ty::()?; + module.ty::()?; + module.ty::()?; Ok(module) } From bcf6baa84f7425f740725fd5e34a61db798c6a2b Mon Sep 17 00:00:00 2001 From: Josh Dutton Date: Sun, 30 Jul 2023 13:28:02 -0400 Subject: [PATCH 07/12] push var on to stack --- crates/rune/src/tests/external_constructor.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/rune/src/tests/external_constructor.rs b/crates/rune/src/tests/external_constructor.rs index 02b225d76..97cba5b83 100644 --- a/crates/rune/src/tests/external_constructor.rs +++ b/crates/rune/src/tests/external_constructor.rs @@ -117,17 +117,17 @@ fn construct_struct() { let mut sources = sources! { entry => { pub fn main(req) { - let foo = 10; + let content_type = "text/plain"; let rsp = match req.url { "/" => Response { status_code: 200, body: "ok", - content_type: "text/plain", + content_type, content_length: 2, }, "/account" => Response { - content_type: "text/plain", + content_type, content_length: 12, body: "unauthorized", status_code: 401, @@ -136,7 +136,7 @@ fn construct_struct() { body: "not found", status_code: 404, content_length: 9, - content_type: "text/plain", + content_type, } }; From 0d5e20d867ecefe827d97dcaa73b9a54aa37ded8 Mon Sep 17 00:00:00 2001 From: Josh Dutton Date: Sun, 30 Jul 2023 13:31:21 -0400 Subject: [PATCH 08/12] assert against whole response --- crates/rune/src/tests/external_constructor.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/rune/src/tests/external_constructor.rs b/crates/rune/src/tests/external_constructor.rs index 97cba5b83..e3ada08b3 100644 --- a/crates/rune/src/tests/external_constructor.rs +++ b/crates/rune/src/tests/external_constructor.rs @@ -77,8 +77,8 @@ fn construct_enum() { assert_eq!(output, Enum::Output(2 * 3 * 4)); } -/// Tests pattern matching and constructing over an external struct from within -/// Rune. +/// Tests constructing an external struct from within Rune, and receiving +/// external structs as an argument. #[test] fn construct_struct() { #[derive(Debug, Any, PartialEq, Eq)] @@ -187,7 +187,6 @@ fn construct_struct() { ] { let output = vm.call(["main"], (req,)).unwrap(); let output: Response = from_value(output).unwrap(); - assert_eq!(output.status_code, rsp.status_code); - assert_eq!(output.body, rsp.body); + assert_eq!(output, rsp); } } From 960eb50834f8f0fc21368e4bac9543673d531d4f Mon Sep 17 00:00:00 2001 From: Josh Dutton Date: Sun, 30 Jul 2023 13:32:38 -0400 Subject: [PATCH 09/12] revert dbg statement --- examples/examples/external_enum.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/examples/external_enum.rs b/examples/examples/external_enum.rs index 7fc49a978..3f34542dd 100644 --- a/examples/examples/external_enum.rs +++ b/examples/examples/external_enum.rs @@ -63,7 +63,6 @@ fn main() -> rune::Result<()> { println!("{:?}", output); let output = vm.call(["main"], (External::Second(42, 12345),))?; - dbg!(&output.type_info().into_result().unwrap()); let output: External = rune::from_value(output)?; println!("{:?}", output); From 86ee5131eaef1663c45214bd7ccc0aaba12e7360 Mon Sep 17 00:00:00 2001 From: Josh Dutton Date: Sun, 30 Jul 2023 13:39:17 -0400 Subject: [PATCH 10/12] test nested struct --- crates/rune/src/tests/external_constructor.rs | 47 ++++++++++++++----- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/crates/rune/src/tests/external_constructor.rs b/crates/rune/src/tests/external_constructor.rs index e3ada08b3..4ff72fb6c 100644 --- a/crates/rune/src/tests/external_constructor.rs +++ b/crates/rune/src/tests/external_constructor.rs @@ -95,6 +95,13 @@ fn construct_struct() { status_code: u32, #[rune(get, set)] body: String, + #[rune(get, set)] + headers: Headers, + } + + #[derive(Debug, Any, Clone, PartialEq, Eq)] + #[rune(constructor)] + struct Headers { #[rune(get, set)] content_type: String, #[rune(get, set)] @@ -105,6 +112,7 @@ fn construct_struct() { let mut module = Module::new(); module.ty::()?; module.ty::()?; + module.ty::()?; Ok(module) } @@ -119,24 +127,33 @@ fn construct_struct() { pub fn main(req) { let content_type = "text/plain"; + // Field order has been purposefully scrambled here, to test + // that they can be given in any order and still compile + // correctly. let rsp = match req.url { "/" => Response { status_code: 200, body: "ok", - content_type, - content_length: 2, + headers: Headers { + content_length: 2, + content_type, + }, }, "/account" => Response { - content_type, - content_length: 12, + headers: Headers { + content_type, + content_length: 12, + }, body: "unauthorized", status_code: 401, }, _ => Response { body: "not found", status_code: 404, - content_length: 9, - content_type, + headers: Headers { + content_type, + content_length: 9, + }, } }; @@ -158,8 +175,10 @@ fn construct_struct() { Response { status_code: 200, body: "ok".into(), - content_type: "text/plain".into(), - content_length: 2, + headers: Headers { + content_type: "text/plain".into(), + content_length: 2, + }, }, ), ( @@ -169,8 +188,10 @@ fn construct_struct() { Response { status_code: 401, body: "unauthorized".into(), - content_type: "text/plain".into(), - content_length: 12, + headers: Headers { + content_type: "text/plain".into(), + content_length: 12, + }, }, ), ( @@ -180,8 +201,10 @@ fn construct_struct() { Response { status_code: 404, body: "not found".into(), - content_type: "text/plain".into(), - content_length: 9, + headers: Headers { + content_type: "text/plain".into(), + content_length: 9, + }, }, ), ] { From 4f73981d136a3dca3d3f8028244cfe5cff0a4496 Mon Sep 17 00:00:00 2001 From: Josh Dutton Date: Sun, 30 Jul 2023 14:34:17 -0400 Subject: [PATCH 11/12] rename to external type --- crates/rune/src/compile/v1/assemble.rs | 2 +- crates/rune/src/hir/hir.rs | 2 +- crates/rune/src/hir/lowering.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/rune/src/compile/v1/assemble.rs b/crates/rune/src/compile/v1/assemble.rs index ec667991e..ab8a8bc1a 100644 --- a/crates/rune/src/compile/v1/assemble.rs +++ b/crates/rune/src/compile/v1/assemble.rs @@ -1976,7 +1976,7 @@ fn expr_object<'hir>( hir::ExprObjectKind::StructVariant { hash } => { cx.asm.push(Inst::StructVariant { hash, slot }, span); } - hir::ExprObjectKind::Constructor { hash, args } => { + hir::ExprObjectKind::ExternalType { hash, args } => { reorder_field_assignments(cx, hir, span)?; cx.asm.push(Inst::Call { hash, args }, span); } diff --git a/crates/rune/src/hir/hir.rs b/crates/rune/src/hir/hir.rs index aac1f075a..1d4493203 100644 --- a/crates/rune/src/hir/hir.rs +++ b/crates/rune/src/hir/hir.rs @@ -580,7 +580,7 @@ pub(crate) enum ExprObjectKind { UnitStruct { hash: Hash }, Struct { hash: Hash }, StructVariant { hash: Hash }, - Constructor { hash: Hash, args: usize }, + ExternalType { hash: Hash, args: usize }, Anonymous, } diff --git a/crates/rune/src/hir/lowering.rs b/crates/rune/src/hir/lowering.rs index f0fd0d6c3..5af2c4586 100644 --- a/crates/rune/src/hir/lowering.rs +++ b/crates/rune/src/hir/lowering.rs @@ -422,7 +422,7 @@ pub(crate) fn expr_object<'hir>( check_object_fields(&st.fields, item)?; match constructor { - Some(_) => hir::ExprObjectKind::Constructor { + Some(_) => hir::ExprObjectKind::ExternalType { hash: meta.hash, args: st.fields.len(), }, From b8ac03be40eeba78dfd843ca194a718307ca500e Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Sun, 30 Jul 2023 22:27:12 +0200 Subject: [PATCH 12/12] Swap assignments based on the current stack frame --- crates/rune/src/compile/v1/assemble.rs | 32 +++++++++++++------------- crates/rune/src/runtime/inst.rs | 4 ++-- crates/rune/src/runtime/stack.rs | 12 ++++++++-- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/crates/rune/src/compile/v1/assemble.rs b/crates/rune/src/compile/v1/assemble.rs index ab8a8bc1a..1439e49fd 100644 --- a/crates/rune/src/compile/v1/assemble.rs +++ b/crates/rune/src/compile/v1/assemble.rs @@ -1957,6 +1957,8 @@ fn expr_object<'hir>( ) -> compile::Result> { let guard = cx.scopes.child(span)?; + let base = cx.scopes.total(span)?; + for assign in hir.assignments.iter() { expr(cx, &assign.assign, Needs::Value)?.apply(cx)?; cx.scopes.alloc(&span)?; @@ -1977,7 +1979,7 @@ fn expr_object<'hir>( cx.asm.push(Inst::StructVariant { hash, slot }, span); } hir::ExprObjectKind::ExternalType { hash, args } => { - reorder_field_assignments(cx, hir, span)?; + reorder_field_assignments(cx, hir, base, span)?; cx.asm.push(Inst::Call { hash, args }, span); } hir::ExprObjectKind::Anonymous => { @@ -2000,6 +2002,7 @@ fn expr_object<'hir>( fn reorder_field_assignments<'hir>( cx: &mut Ctxt<'_, 'hir, '_>, hir: &hir::ExprObject<'hir>, + base: usize, span: &dyn Spanned, ) -> compile::Result<()> { let mut order = Vec::with_capacity(hir.assignments.len()); @@ -2015,32 +2018,29 @@ fn reorder_field_assignments<'hir>( order.push(position); } - for current_position in 0..hir.assignments.len() { + for a in 0..hir.assignments.len() { loop { - let desired_position = order[current_position]; + let Some(&b) = order.get(a) else { + return Err(compile::Error::msg( + span, + "Order out-of-bounds", + )); + }; - if current_position == desired_position { + if a == b { break; } - order.swap(current_position, desired_position); + order.swap(a, b); - let current_offset = hir.assignments.len() - current_position; - - let Some(desired_offset) = hir.assignments.len().checked_sub(desired_position) else { + let (Some(a), Some(b)) = (base.checked_add(a), base.checked_add(b)) else { return Err(compile::Error::msg( span, - format_args!("Wrong number of field assignments: have {} but field position is {}", hir.assignments.len(), desired_position), + "Field repositioning out-of-bounds", )); }; - cx.asm.push( - Inst::Swap { - a: current_offset, - b: desired_offset, - }, - span, - ); + cx.asm.push(Inst::Swap { a, b }, span); } } diff --git a/crates/rune/src/runtime/inst.rs b/crates/rune/src/runtime/inst.rs index b497eb930..7930a780a 100644 --- a/crates/rune/src/runtime/inst.rs +++ b/crates/rune/src/runtime/inst.rs @@ -452,8 +452,8 @@ pub enum Inst { /// Offset to swap value from. offset: usize, }, - /// Swap two values on the stack using their offsets relative to the top - /// of the stack. + /// Swap two values on the stack using their offsets relative to the current + /// stack frame. #[musli(packed)] Swap { /// Offset to the first value. diff --git a/crates/rune/src/runtime/stack.rs b/crates/rune/src/runtime/stack.rs index 33974ecd0..590dcffce 100644 --- a/crates/rune/src/runtime/stack.rs +++ b/crates/rune/src/runtime/stack.rs @@ -325,8 +325,16 @@ impl Stack { /// Swap the value at position a with the value at position b. pub(crate) fn swap(&mut self, a: usize, b: usize) -> Result<(), StackError> { - let a = self.stack.len().checked_sub(a).ok_or(StackError)?; - let b = self.stack.len().checked_sub(b).ok_or(StackError)?; + let a = self + .stack_bottom + .checked_add(a) + .filter(|&n| n < self.stack.len()) + .ok_or(StackError)?; + let b = self + .stack_bottom + .checked_add(b) + .filter(|&n| n < self.stack.len()) + .ok_or(StackError)?; self.stack.swap(a, b); Ok(()) }