From 17b87896a913225ce4f56ef1a2a0d3522b8258c7 Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Wed, 30 Oct 2024 20:10:46 +0100 Subject: [PATCH] rune: Make formatter reference a local value --- crates/rune/src/modules/option.rs | 9 ++++- crates/rune/src/modules/result.rs | 8 +++- crates/rune/src/runtime/fmt.rs | 65 +++++++------------------------ crates/rune/src/runtime/format.rs | 20 +++++----- crates/rune/src/runtime/value.rs | 14 ++++--- crates/rune/src/runtime/vm.rs | 6 ++- 6 files changed, 48 insertions(+), 74 deletions(-) diff --git a/crates/rune/src/modules/option.rs b/crates/rune/src/modules/option.rs index 528b9254a..a8c723015 100644 --- a/crates/rune/src/modules/option.rs +++ b/crates/rune/src/modules/option.rs @@ -1,6 +1,9 @@ //! The [`Option`] type. +use core::ptr::NonNull; + use crate as rune; +use crate::alloc::String; use crate::runtime::{ControlFlow, Formatter, Function, Panic, Value, VmResult}; use crate::Any; use crate::{ContextError, Module}; @@ -83,9 +86,11 @@ fn expect(option: Option, message: Value) -> VmResult { match option { Some(some) => VmResult::Ok(some), None => { - let mut f = Formatter::new(); + let mut s = String::new(); + // SAFETY: Formatter does not outlive the string it references. + let mut f = unsafe { Formatter::new(NonNull::from(&mut s)) }; vm_try!(message.string_display(&mut f)); - VmResult::err(Panic::custom(f.into_string())) + VmResult::err(Panic::custom(s)) } } } diff --git a/crates/rune/src/modules/result.rs b/crates/rune/src/modules/result.rs index 92d929a39..377a44261 100644 --- a/crates/rune/src/modules/result.rs +++ b/crates/rune/src/modules/result.rs @@ -1,5 +1,7 @@ //! The [`Result`] type. +use core::ptr::NonNull; + use crate as rune; use crate::alloc::fmt::TryWrite; use crate::alloc::prelude::*; @@ -201,11 +203,13 @@ fn expect(result: Result, message: Value) -> VmResult { match result { Ok(value) => VmResult::Ok(value), Err(err) => { - let mut f = Formatter::new(); + let mut s = String::new(); + // SAFETY: Formatter does not outlive the string it references. + let mut f = unsafe { Formatter::new(NonNull::from(&mut s)) }; vm_try!(message.string_display(&mut f)); vm_try!(f.try_write_str(": ")); vm_try!(err.string_debug(&mut f)); - VmResult::err(Panic::custom(f.into_string())) + VmResult::err(Panic::custom(s)) } } } diff --git a/crates/rune/src/runtime/fmt.rs b/crates/rune/src/runtime/fmt.rs index 1b632d733..d5553cddf 100644 --- a/crates/rune/src/runtime/fmt.rs +++ b/crates/rune/src/runtime/fmt.rs @@ -1,4 +1,4 @@ -use crate as rune; +use core::ptr::NonNull; use crate::alloc::fmt::TryWrite; use crate::alloc::{self, String}; @@ -12,83 +12,44 @@ use crate::Any; /// [`STRING_DEBUG`]: crate::runtime::Protocol::STRING_DEBUG /// [`STRING_DISPLAY`]: crate::runtime::Protocol::STRING_DISPLAY #[derive(Any)] -#[rune(item = ::std::fmt)] +#[rune(crate, item = ::std::fmt)] pub struct Formatter { - pub(crate) string: String, + pub(crate) out: NonNull, pub(crate) buf: String, } impl Formatter { - /// Construct a new empty formatter. - /// - /// # Examples - /// - /// ``` - /// use rune::runtime::Formatter; - /// - /// let mut f = Formatter::new(); - /// ``` + /// Construct a new formatter wrapping a [`TryWrite`]. #[inline] - pub fn new() -> Self { + pub(crate) unsafe fn new(out: NonNull) -> Self { Self { - string: String::new(), + out, buf: String::new(), } } #[inline] - pub(crate) fn with_capacity(capacity: usize) -> alloc::Result { - Ok(Self { - string: String::try_with_capacity(capacity)?, - buf: String::new(), - }) - } - - #[inline] - pub(crate) fn parts_mut(&mut self) -> (&mut String, &str) { - (&mut self.string, &self.buf) + pub(crate) fn parts_mut(&mut self) -> (&mut dyn TryWrite, &str) { + // SAFETY: Formatter constrution requires `out` to be valid. + (unsafe { self.out.as_mut() }, &self.buf) } #[inline] pub(crate) fn buf_mut(&mut self) -> &mut String { &mut self.buf } - - #[inline] - pub(crate) fn push(&mut self, c: char) -> alloc::Result<()> { - self.string.try_push(c) - } - - #[inline] - pub(crate) fn push_str(&mut self, s: &str) -> alloc::Result<()> { - self.string.try_push_str(s) - } - - #[inline] - pub(crate) fn into_string(self) -> String { - self.string - } - - #[inline] - pub(crate) fn as_str(&self) -> &str { - &self.string - } -} - -impl Default for Formatter { - fn default() -> Self { - Self::new() - } } impl TryWrite for Formatter { #[inline] fn try_write_str(&mut self, s: &str) -> alloc::Result<()> { - self.string.try_push_str(s) + // SAFETY: Formatter constrution requires `out` to be valid. + unsafe { self.out.as_mut().try_write_str(s) } } #[inline] fn try_write_char(&mut self, c: char) -> alloc::Result<()> { - self.string.try_push(c) + // SAFETY: Formatter constrution requires `out` to be valid. + unsafe { self.out.as_mut().try_write_char(c) } } } diff --git a/crates/rune/src/runtime/format.rs b/crates/rune/src/runtime/format.rs index 319942646..15bce69d2 100644 --- a/crates/rune/src/runtime/format.rs +++ b/crates/rune/src/runtime/format.rs @@ -149,13 +149,13 @@ impl FormatSpec { let (f, buf) = f.parts_mut(); if let Some(sign) = sign { - vm_try!(f.try_push(sign)); + vm_try!(f.try_write_char(sign)); } let mut w = self.width.map(|n| n.get()).unwrap_or_default(); if w == 0 { - vm_try!(f.try_push_str(buf)); + vm_try!(f.try_write_str(buf)); return VmResult::Ok(()); } @@ -164,7 +164,7 @@ impl FormatSpec { .saturating_sub(sign.map(|_| 1).unwrap_or_default()); if w == 0 { - vm_try!(f.try_push_str(buf)); + vm_try!(f.try_write_str(buf)); return VmResult::Ok(()); } @@ -172,29 +172,29 @@ impl FormatSpec { match align { Alignment::Left => { - vm_try!(f.try_push_str(buf)); + vm_try!(f.try_write_str(buf)); for c in filler { - vm_try!(f.try_push(c)); + vm_try!(f.try_write_char(c)); } } Alignment::Center => { for c in (&mut filler).take(w / 2) { - vm_try!(f.try_push(c)); + vm_try!(f.try_write_char(c)); } - vm_try!(f.try_push_str(buf)); + vm_try!(f.try_write_str(buf)); for c in filler { - vm_try!(f.try_push(c)); + vm_try!(f.try_write_char(c)); } } Alignment::Right => { for c in filler { - vm_try!(f.try_push(c)); + vm_try!(f.try_write_char(c)); } - vm_try!(f.try_push_str(buf)); + vm_try!(f.try_write_str(buf)); } } diff --git a/crates/rune/src/runtime/value.rs b/crates/rune/src/runtime/value.rs index 0def50da3..75532aa85 100644 --- a/crates/rune/src/runtime/value.rs +++ b/crates/rune/src/runtime/value.rs @@ -361,19 +361,19 @@ impl Value { match vm_try!(self.borrow_ref_repr()) { BorrowRefRepr::Inline(value) => match value { Inline::Char(c) => { - vm_try!(f.push(*c)); + vm_try!(f.try_write_char(*c)); } Inline::Unsigned(byte) => { let mut buffer = itoa::Buffer::new(); - vm_try!(f.push_str(buffer.format(*byte))); + vm_try!(f.try_write_str(buffer.format(*byte))); } Inline::Signed(integer) => { let mut buffer = itoa::Buffer::new(); - vm_try!(f.push_str(buffer.format(*integer))); + vm_try!(f.try_write_str(buffer.format(*integer))); } Inline::Float(float) => { let mut buffer = ryu::Buffer::new(); - vm_try!(f.push_str(buffer.format(*float))); + vm_try!(f.try_write_str(buffer.format(*float))); } Inline::Bool(bool) => { vm_try!(vm_write!(f, "{bool}")); @@ -1768,7 +1768,9 @@ impl fmt::Debug for Value { return Ok(()); } - let mut o = Formatter::new(); + let mut s = String::new(); + // SAFETY: Formatter does not outlive the string it references. + let mut o = unsafe { Formatter::new(NonNull::from(&mut s)) }; if let Err(e) = self.string_debug(&mut o).into_result() { match &self.repr { @@ -1796,7 +1798,7 @@ impl fmt::Debug for Value { return Ok(()); } - f.write_str(o.as_str())?; + f.write_str(s.as_str())?; Ok(()) } } diff --git a/crates/rune/src/runtime/vm.rs b/crates/rune/src/runtime/vm.rs index 4c772df61..07b4d2df6 100644 --- a/crates/rune/src/runtime/vm.rs +++ b/crates/rune/src/runtime/vm.rs @@ -3024,13 +3024,15 @@ impl Vm { let values = vm_try!(self.stack.slice_at(addr, len)); let values = vm_try!(values.iter().cloned().try_collect::>()); - let mut f = vm_try!(Formatter::with_capacity(size_hint)); + let mut s = vm_try!(String::try_with_capacity(size_hint)); + // SAFETY: Formatter does not outlive the string it references. + let mut f = unsafe { Formatter::new(NonNull::from(&mut s)) }; for value in values { vm_try!(value.string_display_with(&mut f, &mut *self)); } - vm_try!(out.store(&mut self.stack, f.string)); + vm_try!(out.store(&mut self.stack, s)); VmResult::Ok(()) }