rune: Store String in AnyObj instead of Mutable (relates #844)

crates/rune/src/any.rs

@@ -1,5 +1,6 @@
 use core::any;
 
+use crate::alloc::String;
 use crate::compile::Named;
 use crate::runtime::{AnyTypeInfo, TypeHash};
 
@@ -124,3 +125,5 @@ cfg_std! {
 }
 
 crate::__internal_impl_any!(::std::error, anyhow::Error);
+
+impl Any for String {}

crates/rune/src/compile/error.rs

@@ -20,7 +20,7 @@ use crate::macros::{SyntheticId, SyntheticKind};
 use crate::parse::{Expectation, IntoExpectation, LexerMode};
 use crate::runtime::debug::DebugSignature;
 use crate::runtime::unit::EncodeError;
-use crate::runtime::{AccessError, RuntimeError, TypeInfo, TypeOf, VmError};
+use crate::runtime::{AccessError, AnyObjError, RuntimeError, TypeInfo, TypeOf, VmError};
 #[cfg(feature = "std")]
 use crate::source;
 use crate::{Hash, Item, ItemBuf, SourceId};
@@ -1307,6 +1307,13 @@ impl From<RuntimeError> for ErrorKind {
     }
 }
 
+impl From<AnyObjError> for ErrorKind {
+    #[inline]
+    fn from(error: AnyObjError) -> Self {
+        Self::from(RuntimeError::from(error))
+    }
+}
+
 impl From<EncodeError> for ErrorKind {
     #[inline]
     fn from(error: EncodeError) -> Self {

crates/rune/src/compile/ir/eval.rs

@@ -7,7 +7,8 @@ use crate::ast::{Span, Spanned};
 use crate::compile::ir::{self};
 use crate::compile::{self, WithSpan};
 use crate::query::Used;
-use crate::runtime::{Inline, Mutable, Object, OwnedTuple, Value, ValueBorrowRef};
+use crate::runtime::{Inline, Object, OwnedTuple, Value, ValueBorrowRef};
+use crate::TypeHash;
 
 /// The outcome of a constant evaluation.
 pub enum EvalOutcome {
@@ -139,18 +140,25 @@ fn eval_ir_binary(
 
             return Ok(Value::from(out));
         }
-        (ValueBorrowRef::Mutable(a), ValueBorrowRef::Mutable(b)) => {
-            let out = 'out: {
-                if let (Mutable::String(a), Mutable::String(b)) = (&*a, &*b) {
-                    if let ir::IrBinaryOp::Add = ir.op {
-                        break 'out Mutable::String(add_strings(a, b).with_span(span)?);
+        (ValueBorrowRef::Any(a), ValueBorrowRef::Any(b)) => {
+            let value = 'out: {
+                match (a.type_hash(), b.type_hash()) {
+                    (String::HASH, String::HASH) => {
+                        let a = a.borrow_ref::<String>().with_span(span)?;
+                        let b = b.borrow_ref::<String>().with_span(span)?;
+
+                        if let ir::IrBinaryOp::Add = ir.op {
+                            let string = add_strings(&*a, &*b).with_span(span)?;
+                            break 'out Value::new(string).with_span(span)?;
+                        }
                     }
+                    _ => {}
                 }
 
                 return Err(EvalOutcome::not_const(span));
             };
 
-            return Ok(Value::try_from(out).with_span(span)?);
+            return Ok(value);
         }
         _ => (),
     }
@@ -366,15 +374,16 @@ fn eval_ir_template(
                             return Err(EvalOutcome::not_const(ir));
                         }
                     },
-                    ValueBorrowRef::Mutable(value) => match &*value {
-                        Mutable::String(s) => {
-                            buf.try_push_str(s)?;
+                    ValueBorrowRef::Any(value) => match value.type_hash() {
+                        String::HASH => {
+                            let s = value.borrow_ref::<String>().with_span(ir)?;
+                            buf.try_push_str(&s)?;
                         }
                         _ => {
                             return Err(EvalOutcome::not_const(ir));
                         }
                     },
-                    ValueBorrowRef::Any(..) => {
+                    ValueBorrowRef::Mutable(..) => {
                         return Err(EvalOutcome::not_const(ir));
                     }
                 }

crates/rune/src/macros/format_args.rs

@@ -7,8 +7,7 @@ use crate::ast::{self, Span, Spanned};
 use crate::compile::{self, WithSpan};
 use crate::macros::{quote, MacroContext, Quote, ToTokens, TokenStream};
 use crate::parse::{Parse, Parser, Peek, Peeker};
-use crate::runtime::format;
-use crate::runtime::{Inline, Mutable, OwnedValue};
+use crate::runtime::{format, Inline};
 
 /// A format specification: A format string followed by arguments to be
 /// formatted in accordance with that string.
@@ -49,14 +48,7 @@ impl FormatArgs {
             }
         }
 
-        let format = format.take_value().with_span(&self.format)?;
-
-        let OwnedValue::Mutable(Mutable::String(format)) = format else {
-            return Err(compile::Error::msg(
-                &self.format,
-                "format argument must be a string",
-            ));
-        };
+        let format = format.into_any::<String>().with_span(&self.format)?;
 
         let mut unused_pos = (0..pos.len()).try_collect::<BTreeSet<_>>()?;
         let mut unused_named = named

crates/rune/src/modules/iter.rs

@@ -8,8 +8,8 @@ use crate::modules::collections::VecDeque;
 use crate::modules::collections::{HashMap, HashSet};
 use crate::runtime::range::RangeIter;
 use crate::runtime::{
-    FromValue, Function, Inline, InstAddress, Mutable, Object, Output, OwnedTuple, Protocol,
-    TypeHash, Value, ValueBorrowRef, Vec, VmErrorKind, VmResult,
+    FromValue, Function, Inline, InstAddress, Object, Output, OwnedTuple, Protocol, TypeHash,
+    Value, ValueBorrowRef, Vec, VmErrorKind, VmResult,
 };
 use crate::shared::Caller;
 use crate::{Any, ContextError, Module, Params};
@@ -489,17 +489,21 @@ pub fn module() -> Result<Module, ContextError> {
                                 ValueBorrowRef::Inline(Inline::Char(c)) => {
                                     vm_try!(string.try_push(*c));
                                 }
-                                ValueBorrowRef::Mutable(value) => match &*value {
-                                    Mutable::String(s) => {
-                                        vm_try!(string.try_push_str(s));
+                                ValueBorrowRef::Inline(value) => {
+                                    return VmResult::expected::<String>(value.type_info());
+                                }
+                                ValueBorrowRef::Mutable(value) => {
+                                    return VmResult::expected::<String>(value.type_info());
+                                }
+                                ValueBorrowRef::Any(value) => match value.type_hash() {
+                                    String::HASH => {
+                                        let s = vm_try!(value.borrow_ref::<String>());
+                                        vm_try!(string.try_push_str(&*s));
                                     }
-                                    actual => {
-                                        return VmResult::expected::<String>(actual.type_info());
+                                    _ => {
+                                        return VmResult::expected::<String>(value.type_info());
                                     }
                                 },
-                                actual => {
-                                    return VmResult::expected::<String>(actual.type_info());
-                                }
                             }
                         }
 

crates/rune/src/modules/string.rs

@@ -762,12 +762,6 @@ fn split(this: Ref<str>, value: Value) -> VmResult<Value> {
             vm_try!(rune::to_value(Split::new(this, *c)))
         }
         ValueBorrowRef::Mutable(value) => match &*value {
-            Mutable::String(ref s) => {
-                vm_try!(rune::to_value(Split::new(
-                    this,
-                    vm_try!(Box::try_from(s.as_str()))
-                )))
-            }
             Mutable::Function(ref f) => {
                 vm_try!(rune::to_value(Split::new(this, vm_try!(f.try_clone()))))
             }
@@ -778,11 +772,27 @@ fn split(this: Ref<str>, value: Value) -> VmResult<Value> {
                 ])
             }
         },
-        actual => {
+        ValueBorrowRef::Any(value) => match value.type_hash() {
+            String::HASH => {
+                let s = vm_try!(value.borrow_ref::<String>());
+
+                vm_try!(rune::to_value(Split::new(
+                    this,
+                    vm_try!(Box::try_from(s.as_str()))
+                )))
+            }
+            _ => {
+                return VmResult::err([
+                    VmErrorKind::expected::<String>(value.type_info()),
+                    VmErrorKind::bad_argument(0),
+                ]);
+            }
+        },
+        value => {
             return VmResult::err([
-                VmErrorKind::expected::<String>(actual.type_info()),
+                VmErrorKind::expected::<String>(value.type_info()),
                 VmErrorKind::bad_argument(0),
-            ])
+            ]);
         }
     };
 
@@ -805,7 +815,6 @@ fn split_once(this: &str, value: Value) -> VmResult<Option<(String, String)>> {
     let outcome = match vm_try!(value.borrow_ref()) {
         ValueBorrowRef::Inline(Inline::Char(pat)) => this.split_once(*pat),
         ValueBorrowRef::Mutable(value) => match &*value {
-            Mutable::String(s) => this.split_once(s.as_str()),
             Mutable::Function(f) => {
                 let mut err = None;
 
@@ -833,6 +842,18 @@ fn split_once(this: &str, value: Value) -> VmResult<Option<(String, String)>> {
                 ])
             }
         },
+        ValueBorrowRef::Any(value) => match value.type_hash() {
+            String::HASH => {
+                let s = vm_try!(value.borrow_ref::<String>());
+                this.split_once(s.as_str())
+            }
+            _ => {
+                return VmResult::err([
+                    VmErrorKind::expected::<String>(value.type_info()),
+                    VmErrorKind::bad_argument(0),
+                ]);
+            }
+        },
         ref actual => {
             return VmResult::err([
                 VmErrorKind::expected::<String>(actual.type_info()),

crates/rune/src/runtime/access.rs

@@ -12,11 +12,17 @@ const MOVED: usize = 1usize.rotate_right(1);
 /// Mask indicating if the value is exclusively set or moved.
 const MASK: usize = EXCLUSIVE | MOVED;
 
-/// An error raised while downcasting.
-#[derive(Debug, PartialEq)]
-#[allow(missing_docs)]
+/// An error raised when failing to access a value.
+///
+/// Access errors can be raised for various reasons, such as:
+/// * The value you are trying to access is an empty placeholder.
+/// * The value is already being accessed in an incompatible way, such as trying
+///   to access a value exclusively twice.
+/// * The value has been taken and is no longer present.
+#[derive(Debug)]
+#[cfg_attr(test, derive(PartialEq))]
 #[non_exhaustive]
-pub(crate) struct AccessError {
+pub struct AccessError {
     kind: AccessErrorKind,
 }
 
@@ -64,7 +70,8 @@ impl From<AccessErrorKind> for AccessError {
     }
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug)]
+#[cfg_attr(test, derive(PartialEq))]
 pub(crate) enum AccessErrorKind {
     Empty,
     NotAccessibleRef(Snapshot),

crates/rune/src/runtime/any_obj.rs

@@ -13,6 +13,13 @@ use super::{
     RefVtable, Snapshot, TypeInfo, VmErrorKind,
 };
 
+#[derive(Debug)]
+#[cfg_attr(test, derive(PartialEq))]
+pub(super) enum AnyObjErrorKind {
+    Cast(AnyTypeInfo, TypeInfo),
+    AccessError(AccessError),
+}
+
 /// Errors caused when accessing or coercing an [`AnyObj`].
 #[cfg_attr(test, derive(PartialEq))]
 pub struct AnyObjError {
@@ -30,11 +37,17 @@ impl AnyObjError {
     }
 }
 
-#[derive(Debug)]
-#[cfg_attr(test, derive(PartialEq))]
-pub(super) enum AnyObjErrorKind {
-    Cast(AnyTypeInfo, TypeInfo),
-    AccessError(AccessError),
+impl core::error::Error for AnyObjError {}
+
+impl fmt::Display for AnyObjError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match &self.kind {
+            AnyObjErrorKind::Cast(expected, actual) => {
+                write!(f, "Expected type `{expected}` but found `{actual}`")
+            }
+            AnyObjErrorKind::AccessError(error) => error.fmt(f),
+        }
+    }
 }
 
 impl fmt::Debug for AnyObjError {
@@ -370,6 +383,31 @@ impl AnyObj {
         }
     }
 
+    /// Try to borrow a reference to the interior value while checking for
+    /// shared access.
+    ///
+    /// Returns `None` if the interior type is not `T`.
+    ///
+    /// This prevents other exclusive accesses from being performed while the
+    /// guard returned from this function is live.
+    pub fn try_borrow_ref<T>(&self) -> Result<Option<BorrowRef<'_, T>>, AccessError>
+    where
+        T: Any,
+    {
+        let vtable = vtable(self);
+
+        if (vtable.type_id)() != TypeId::of::<T>() {
+            return Ok(None);
+        }
+
+        // SAFETY: We've checked for the appropriate type just above.
+        unsafe {
+            let guard = self.shared.as_ref().access.shared()?;
+            let data = vtable.as_ptr(self.shared);
+            Ok(Some(BorrowRef::new(data, guard)))
+        }
+    }
+
     /// Returns some mutable reference to the boxed value if it is of type `T`.
     pub fn borrow_mut<T>(&self) -> Result<BorrowMut<'_, T>, AnyObjError>
     where

crates/rune/src/runtime/const_value.rs

@@ -6,6 +6,7 @@ use crate::runtime::{
     self, Bytes, FromValue, Inline, Mutable, Object, OwnedTuple, ToValue, TypeInfo, Value,
     ValueBorrowRef, VmErrorKind, VmResult,
 };
+use crate::TypeHash;
 
 /// A constant value.
 #[derive(Debug, Deserialize, Serialize)]
@@ -54,7 +55,6 @@ impl ConstValue {
                 }
             },
             ValueBorrowRef::Mutable(value) => match &*value {
-                Mutable::String(s) => Self::String(vm_try!(s.try_to_owned())),
                 Mutable::Option(option) => Self::Option(match option {
                     Some(some) => Some(vm_try!(Box::try_new(vm_try!(Self::from_value_ref(some))))),
                     None => None,
@@ -95,11 +95,17 @@ impl ConstValue {
                     })
                 }
             },
-            value => {
-                return VmResult::err(VmErrorKind::ConstNotSupported {
-                    actual: value.type_info(),
-                })
-            }
+            ValueBorrowRef::Any(value) => match value.type_hash() {
+                String::HASH => {
+                    let s = vm_try!(value.borrow_ref::<String>());
+                    Self::String(vm_try!(s.try_to_owned()))
+                }
+                _ => {
+                    return VmResult::err(VmErrorKind::ConstNotSupported {
+                        actual: value.type_info(),
+                    });
+                }
+            },
         })
     }