diff --git a/examples/custom_functions.rs b/examples/custom_functions.rs index 5abf6b6..0cd0b3e 100644 --- a/examples/custom_functions.rs +++ b/examples/custom_functions.rs @@ -1,13 +1,11 @@ //! A simple exmaple -use std::{cell::RefCell, rc::Weak}; - use bevy::{ log::{Level, LogPlugin}, prelude::*, }; use bevy_dev_console::{ - builtin_parser::{Environment, RunError, Spanned, Value}, + builtin_parser::{Environment, RunError, Spanned, Value, StrongRef}, prelude::*, register, }; @@ -47,16 +45,15 @@ fn increment_global_counter(world: &mut World) -> f64 { }) } -// Function with reference (Syntax subject to change very soon) -fn increment_number(number: Spanned>>) -> Result<(), RunError> { - let rc = number.value.upgrade().unwrap(); - let mut reference = rc.borrow_mut(); +// Function with reference (Syntax subject to change soon) +fn increment_number(number: Spanned>) -> Result<(), RunError> { + let mut reference = number.value.borrow_mut(); if let Value::Number(number) = &mut *reference { *number += 1.0; Ok(()) } else { Err(RunError::Custom { - text: "w".to_string(), + text: "Oh nooo".to_string(), span: number.span, }) } diff --git a/src/builtin_parser.rs b/src/builtin_parser.rs index 269f196..ed7e729 100644 --- a/src/builtin_parser.rs +++ b/src/builtin_parser.rs @@ -15,7 +15,7 @@ pub(crate) mod lexer; pub(crate) mod parser; pub(crate) mod runner; -pub use runner::{environment::Environment, Value, RunError}; +pub use runner::{environment::Environment, Value, RunError, unique_rc::*}; /// Wrapper around `T` that stores a [Span] (A location in the source code) #[derive(Debug, Clone)] diff --git a/src/builtin_parser/runner.rs b/src/builtin_parser/runner.rs index 5bbf5c4..4b26711 100644 --- a/src/builtin_parser/runner.rs +++ b/src/builtin_parser/runner.rs @@ -4,7 +4,10 @@ use std::{cell::RefCell, collections::HashMap, rc::Rc}; use environment::Environment; -use self::reflection::{object_to_dynamic_struct, IntoRegistration, IntoResource}; +use self::{ + reflection::{object_to_dynamic_struct, CreateRegistration, IntoResource}, + unique_rc::{UniqueRc, WeakRef}, +}; use super::{ parser::{Ast, Expression, Operator}, @@ -12,14 +15,14 @@ use super::{ }; use bevy::{ prelude::*, - reflect::{DynamicEnum, Enum, EnumInfo, ReflectMut, TypeInfo, TypeRegistration}, + reflect::{DynamicEnum, Enum, ReflectMut, TypeInfo, TypeRegistration}, }; use logos::Span; pub mod environment; pub mod reflection; pub mod stdlib; -pub mod value; pub mod unique_rc; +pub mod value; pub use value::Value; @@ -44,6 +47,7 @@ pub enum RunError { CannotBorrowValue(Span), IncompatibleReflectTypes { expected: String, actual: String }, EnumVariantNotFound { name: String }, + CannotMoveOutOfResource(Spanned), } pub fn run(ast: Ast, world: &mut World) { @@ -95,7 +99,7 @@ pub fn run(ast: Ast, world: &mut World) { match value { Ok(Value::None) => {} - Ok(value) => match value.try_format(span, &world, ®istrations) { + Ok(value) => match value.try_format(span, world, ®istrations) { Ok(value) => info!(name: "console_result", "> {value}"), Err(err) => error!("{err:?}"), }, @@ -123,7 +127,6 @@ fn eval_expression( value: value_expr, } => match eval_path(*name, environment, registrations)?.value { Path::Variable(variable) => { - let temp = variable.clone(); let value = eval_expression( *value_expr, EvalParams { @@ -132,9 +135,9 @@ fn eval_expression( registrations, }, )?; - *temp.borrow_mut() = value; + *variable.upgrade().unwrap().borrow_mut() = value; - Ok(Value::Reference(Rc::downgrade(&temp))) + Ok(Value::Reference(variable)) } Path::NewVariable(variable) => { let value = eval_expression( @@ -145,15 +148,15 @@ fn eval_expression( registrations, }, )?; - let rc = Rc::new(RefCell::new(value)); - let weak = Rc::downgrade(&rc); + let rc = UniqueRc::new(value); + let weak = rc.borrow(); environment.set(variable, rc); Ok(Value::Reference(weak)) } Path::Resource(resource) => { - let registeration = registrations.into_registration(resource.id); + let registeration = registrations.create_registration(resource.id); let mut dyn_reflect = resource.mut_dyn_reflect(world, registeration); let reflect = dyn_reflect @@ -248,11 +251,14 @@ fn eval_expression( Expression::String(string) => Ok(Value::String(string)), Expression::Number(number) => Ok(Value::Number(number)), Expression::Variable(variable) => { - if let Some(registration) = registrations + if registrations .iter() - .find(|v| v.type_info().type_path_table().short_path() == variable) + .any(|v| v.type_info().type_path_table().short_path() == variable) { - Ok(Value::Resource(IntoResource::new(registration.type_id()))) + Err(RunError::CannotMoveOutOfResource(Spanned { + span: expr.span, + value: variable, + })) } else { environment.move_var(&variable, expr.span) } @@ -353,7 +359,7 @@ fn eval_expression( Ok(Value::Resource(IntoResource::new(registration.type_id()))) } else { let rc = environment.get(&variable, inner.span)?; - let weak = Rc::downgrade(rc); + let weak = rc.borrow(); Ok(Value::Reference(weak)) } @@ -418,17 +424,17 @@ fn eval_member_expression( } } -enum Path<'a> { - Variable(&'a Rc>), +enum Path { + Variable(WeakRef), NewVariable(String), Resource(IntoResource), } -fn eval_path<'a>( +fn eval_path( expr: Spanned, - environment: &'a Environment, + environment: &Environment, registrations: &[&TypeRegistration], -) -> Result>, RunError> { +) -> Result, RunError> { match expr.value { Expression::Variable(variable) => { if let Some(registration) = registrations @@ -439,18 +445,16 @@ fn eval_path<'a>( span: expr.span, value: Path::Resource(IntoResource::new(registration.type_id())), }) + } else if let Ok(variable) = environment.get(&variable, expr.span.clone()) { + Ok(Spanned { + span: expr.span, + value: Path::Variable(variable.borrow()), + }) } else { - if let Ok(variable) = environment.get(&variable, expr.span.clone()) { - Ok(Spanned { - span: expr.span, - value: Path::Variable(variable), - }) - } else { - Ok(Spanned { - span: expr.span, - value: Path::NewVariable(variable), - }) - } + Ok(Spanned { + span: expr.span, + value: Path::NewVariable(variable), + }) } } Expression::Member { left, right } => { diff --git a/src/builtin_parser/runner/environment.rs b/src/builtin_parser/runner/environment.rs index abc099e..4cf3002 100644 --- a/src/builtin_parser/runner/environment.rs +++ b/src/builtin_parser/runner/environment.rs @@ -1,13 +1,15 @@ //! Environment and function registeration -use std::{cell::RefCell, collections::HashMap, rc::Rc}; +use std::collections::HashMap; use bevy::{ecs::world::World, log::warn, reflect::TypeRegistration}; use logos::Span; use super::{ super::{parser::Expression, Spanned}, - eval_expression, stdlib, EvalParams, RunError, Value, + eval_expression, stdlib, + unique_rc::UniqueRc, + EvalParams, RunError, Value, }; /// Macro for mass registering functions. @@ -130,6 +132,7 @@ macro_rules! impl_into_function { let world = &mut Some(world); let environment = &mut Some(environment); + #[allow(clippy::too_many_arguments)] fn call_inner>, $($($params),*)?>( mut f: impl FnMut($($($params),*)?) -> R, $($($params: $params),*)? @@ -144,7 +147,7 @@ macro_rules! impl_into_function { } else { None }; - + let res = $params::get( arg, world, @@ -180,7 +183,7 @@ impl_into_function!(T1, T2, T3, T4, T5, T6, T7, T8); /// A variable inside the [`Environment`]. pub enum Variable { - Unmoved(Rc>), + Unmoved(UniqueRc), Moved, Function(Function), } @@ -206,7 +209,7 @@ impl Default for Environment { impl Environment { /// Set a variable. - pub fn set(&mut self, name: impl Into, value: Rc>) { + pub fn set(&mut self, name: impl Into, value: UniqueRc) { self.variables.insert(name.into(), Variable::Unmoved(value)); } @@ -250,7 +253,7 @@ impl Environment { return_result } /// Returns a reference to a variable. - pub fn get(&self, name: &str, span: Span) -> Result<&Rc>, RunError> { + pub fn get(&self, name: &str, span: Span) -> Result<&UniqueRc, RunError> { let (env, span) = self.resolve(name, span)?; match env.variables.get(name) { @@ -261,33 +264,33 @@ impl Environment { } } - /// "moves" a variable, giving you ownership over it. However it will no longer be able to be used unless - /// it's a [`Value::None`], [`Value::Boolean`], or [`Value::Number`] in which case it will be copied. + /// "Moves" a variable, giving you ownership over it. + /// + /// However it will no longer be able to be used unless it's a [`Value::None`], + /// [`Value::Boolean`], or [`Value::Number`] in which case it will be copied. pub fn move_var(&mut self, name: &str, span: Span) -> Result { let (env, span) = self.resolve_mut(name, span)?; match env.variables.get_mut(name) { Some(Variable::Moved) => Err(RunError::VariableMoved(span)), Some(Variable::Function(_)) => todo!(), - Some(reference) => { - let Variable::Unmoved(value) = std::mem::replace(reference, Variable::Moved) else { + Some(variable_reference) => { + let Variable::Unmoved(reference) = variable_reference else { unreachable!() }; - // SAFETY: The reference goes out of scope before it can get borrowed again. - let reference = unsafe { value.try_borrow_unguarded().unwrap() }; - // This is a pretty bad way of handling - match reference { - Value::None => Ok(Value::None), - Value::Boolean(bool) => Ok(Value::Boolean(*bool)), - Value::Number(number) => Ok(Value::Number(*number)), - _ => { - // Unwrapping will always succeed due to only the owner of the variable having - // a strong reference. All other references are weak. - let value = Rc::try_unwrap(value).unwrap(); - - Ok(value.into_inner()) - } - } + // This is a pretty bad way of handling something similar to rust's [`Copy`] trait but whatever. + match &*reference.borrow_inner().borrow() { + Value::None => return Ok(Value::None), + Value::Boolean(bool) => return Ok(Value::Boolean(*bool)), + Value::Number(number) => return Ok(Value::Number(*number)), + _ => {} + }; + let Variable::Unmoved(value) = + std::mem::replace(variable_reference, Variable::Moved) + else { + unreachable!() + }; + Ok(value.into_inner()) } None => Err(RunError::VariableNotFound(span)), } diff --git a/src/builtin_parser/runner/reflection.rs b/src/builtin_parser/runner/reflection.rs index a0814e6..b52c718 100644 --- a/src/builtin_parser/runner/reflection.rs +++ b/src/builtin_parser/runner/reflection.rs @@ -22,9 +22,9 @@ impl IntoResource { pub fn ref_dyn_reflect<'a>( &self, world: &'a World, - registration: impl IntoRegistration, + registration: impl CreateRegistration, ) -> &'a dyn Reflect { - let registration = registration.into_registration(self.id); + let registration = registration.create_registration(self.id); let ref_dyn_reflect = ref_dyn_reflect(world, registration).unwrap(); ref_dyn_reflect @@ -32,9 +32,9 @@ impl IntoResource { pub fn mut_dyn_reflect<'a>( &self, world: &'a mut World, - registration: impl IntoRegistration, + registration: impl CreateRegistration, ) -> Mut<'a, dyn Reflect> { - let registration = registration.into_registration(self.id); + let registration = registration.create_registration(self.id); let ref_dyn_reflect = mut_dyn_reflect(world, registration).unwrap(); ref_dyn_reflect @@ -82,22 +82,22 @@ pub fn ref_dyn_reflect<'a>( let resource = world.get_resource_by_id(component_id).unwrap(); let reflect_from_ptr = registration.data::().unwrap(); // SAFETY: from the context it is known that `ReflectFromPtr` was made for the type of the `MutUntyped` - let val: &'a dyn Reflect = unsafe { reflect_from_ptr.as_reflect(resource) }; + let val: &dyn Reflect = unsafe { reflect_from_ptr.as_reflect(resource) }; Some(val) } -pub trait IntoRegistration { - fn into_registration<'a>(&'a self, type_id: TypeId) -> &'a TypeRegistration; +pub trait CreateRegistration { + fn create_registration(&self, type_id: TypeId) -> &TypeRegistration; } -impl IntoRegistration for &TypeRegistration { - fn into_registration<'a>(&'a self, type_id: TypeId) -> &'a TypeRegistration { +impl CreateRegistration for &TypeRegistration { + fn create_registration(&self, type_id: TypeId) -> &TypeRegistration { assert!(self.type_id() == type_id); - &self + self } } -impl IntoRegistration for &[&TypeRegistration] { - fn into_registration<'a>(&'a self, type_id: TypeId) -> &'a TypeRegistration { +impl CreateRegistration for &[&TypeRegistration] { + fn create_registration(&self, type_id: TypeId) -> &TypeRegistration { self.iter() .find(|reg| reg.type_id() == type_id) .expect("registration no longer exists") diff --git a/src/builtin_parser/runner/unique_rc.rs b/src/builtin_parser/runner/unique_rc.rs index 61edc33..cdc794d 100644 --- a/src/builtin_parser/runner/unique_rc.rs +++ b/src/builtin_parser/runner/unique_rc.rs @@ -1,27 +1,133 @@ -use std::{rc::Rc, ops::{DerefMut, Deref}, cell::RefCell}; +use std::{ + cell::{Ref, RefCell, RefMut, Cell}, + fmt::Debug, + ops::{Deref, DerefMut}, + rc::{Rc, Weak}, +}; -struct UniqueRc(Rc); + +/// A uniquely owned [`Rc`] with interior mutability. Interior mutability is abstracted away with [`WeakRef`]. +/// +/// This represents an [`Rc`] that is known to be uniquely owned -- that is, have exactly one strong +/// reference. +/// +/// **TODO:** This is actually going to be a standard library feature. Use [`alloc::rc::UniqueRc`] when it is stabilized. +pub struct UniqueRc(Rc>); impl UniqueRc { - pub fn into_rc(self) -> Rc { - self.0 - } + /// Get a reference to the inner [`Rc`] of [`UniqueRc`]. + /// + /// # Safety + /// + /// This function is unsafe because it allows direct access to the [`Rc`]. + /// If cloned then the gurantee that there is only ever one strong reference is no longer satisfied. + unsafe fn get_rc(&self) -> &Rc> { + &self.0 + } + pub fn borrow_inner(&self) -> &RefCell { + &self.0 + } + pub fn borrow(&self) -> WeakRef { + WeakRef::new(&self) + } } impl UniqueRc { - pub fn into_inner(self) -> T { - // SAFETY: There is only ever one owner of Rc - unsafe { Rc::try_unwrap(self.0).unwrap_unchecked() } - } + /// Create a new [`UniqueRc`]. + pub fn new(value: T) -> UniqueRc { + UniqueRc(Rc::new(RefCell::new(value))) + } + pub fn into_inner(self) -> T { + Rc::try_unwrap(self.0) + .unwrap_or_else(|rc| panic!( + "There are {} strong pointers to a UniqueRc!", + Rc::strong_count(&rc) + )) + .into_inner() + } } impl Deref for UniqueRc { - type Target = T; + type Target = RefCell; fn deref(&self) -> &Self::Target { self.0.as_ref() } } impl DerefMut for UniqueRc { fn deref_mut(&mut self) -> &mut Self::Target { - // SAFETY: There is only ever one owner of Rc - unsafe { Rc::get_mut(&mut self.0).unwrap_unchecked() } + Rc::get_mut(&mut self.0).unwrap() + } +} + +#[derive(Debug)] +pub struct WeakRef { + reference: Weak>, + upgraded: Cell +} +impl WeakRef { + fn new(unique_rc: &UniqueRc) -> Self { + // SAFETY: We are not cloning the `Rc`, so this is fine. + let rc = unsafe { unique_rc.get_rc() }; + Self { reference: Rc::downgrade(rc), upgraded: Cell::new(false) } + } + pub fn upgrade(&self) -> Option> { + if !self.upgraded.get() { + self.upgraded.set(true); + self.upgrade_unchecked() + } else { + None + } + } + fn upgrade_unchecked(&self) -> Option> { + Some(StrongRef(self.reference.upgrade()?)) + } +} + +/// A reference to value `T`. +/// +/// This value is *technically* unsafe, but in practice the only way +/// you could obtain it is by having it passed into a custom function. +/// +/// ``` +/// fn add_to_reference() +/// ``` +#[derive(Debug)] +pub struct StrongRef(Rc>); +impl StrongRef { + /// Immutably borrows the wrapped value. + pub fn borrow(&self) -> Ref { + self.0.borrow() } -} \ No newline at end of file + /// Mutably borrows the wrapped value. + pub fn borrow_mut(&self) -> RefMut { + self.0.borrow_mut() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn weak_ref_upgrade_once() { + let rc = UniqueRc::new(0); + + let weak = rc.borrow(); + + assert!(weak.upgrade().is_some()); + assert!(weak.upgrade().is_none()); + assert!(weak.upgrade().is_none()); + } + + #[test] + #[should_panic] + fn strong_ref_panic() { + let rc = UniqueRc::new(0); + + let weak = rc.borrow(); + + let strong = weak.upgrade().unwrap(); + + println!("{}", rc.into_inner()); // Panic! + + println!("{}", strong.borrow()); + } +} diff --git a/src/builtin_parser/runner/value.rs b/src/builtin_parser/runner/value.rs index 5cc1f4a..becb7fb 100644 --- a/src/builtin_parser/runner/value.rs +++ b/src/builtin_parser/runner/value.rs @@ -1,17 +1,14 @@ -use std::cell::RefMut; use std::collections::HashMap; use std::fmt::Debug; -use std::rc::Weak; use std::{cell::RefCell, rc::Rc}; -use crate::builtin_parser::Environment; +use crate::builtin_parser::{Environment, StrongRef}; -use super::environment::{FunctionParam, ResultContainer}; -use super::reflection::{mut_dyn_reflect, IntoRegistration, IntoResource}; -use super::EvalParams; +use super::environment::FunctionParam; +use super::reflection::{CreateRegistration, IntoResource}; +use super::unique_rc::WeakRef; use super::{super::Spanned, RunError}; -use bevy::ecs::system::Resource; use bevy::ecs::world::World; use bevy::reflect::{ DynamicStruct, GetPath, Reflect, ReflectRef, TypeInfo, TypeRegistration, VariantInfo, @@ -22,6 +19,7 @@ use logos::Span; /// A runtime value #[derive(Debug)] +#[non_exhaustive] pub enum Value { /// Nothing at all None, @@ -38,10 +36,7 @@ pub enum Value { /// and having only the owner of the value have a strong reference, /// while every other value has a weak reference. This causes /// [`Rc::try_unwrap`] to succeed every time. - /// - /// This isn't partically efficent, so: - /// TODO: Create a custom type this! - Reference(Weak>), + Reference(WeakRef), /// A dynamic [`HashMap`]. Object(HashMap>>), /// An [`Object`](Value::Object) with a name attached to it. @@ -144,7 +139,7 @@ fn fancy_debug_print( world: &World, registrations: &[&TypeRegistration], ) -> String { - let registration = registrations.into_registration(resource.id); + let registration = registrations.create_registration(resource.id); let dyn_reflect = resource.ref_dyn_reflect(world, registration); let reflect = dyn_reflect.reflect_path(resource.path.as_str()).unwrap(); @@ -349,7 +344,7 @@ impl_function_param_for_value!(impl HashMap>>: Value:: impl_function_param_for_value!(impl HashMap: Value::Object(object) => { object.into_iter().map(|(k, v)| (k, Rc::try_unwrap(v).unwrap().into_inner())).collect() }); -impl_function_param_for_value!(impl Weak>: Value::Reference(reference) => reference); +impl_function_param_for_value!(impl StrongRef: Value::Reference(reference) => reference.upgrade().unwrap()); impl FunctionParam for &mut World { type Item<'world, 'env, 'reg> = &'world mut World;