Skip to content

Commit

Permalink
rune: Make formatter reference a local value
Browse files Browse the repository at this point in the history
  • Loading branch information
udoprog committed Oct 30, 2024
1 parent 2210843 commit a122ec7
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 97 deletions.
9 changes: 7 additions & 2 deletions crates/rune/src/modules/option.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -83,9 +86,11 @@ fn expect(option: Option<Value>, message: Value) -> VmResult<Value> {
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))
}
}
}
Expand Down
8 changes: 6 additions & 2 deletions crates/rune/src/modules/result.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! The [`Result`] type.

use core::ptr::NonNull;

use crate as rune;
use crate::alloc::fmt::TryWrite;
use crate::alloc::prelude::*;
Expand Down Expand Up @@ -201,11 +203,13 @@ fn expect(result: Result<Value, Value>, message: Value) -> VmResult<Value> {
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))
}
}
}
Expand Down
65 changes: 13 additions & 52 deletions crates/rune/src/runtime/fmt.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate as rune;
use core::ptr::NonNull;

use crate::alloc::fmt::TryWrite;
use crate::alloc::{self, String};
Expand All @@ -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<dyn TryWrite>,
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<dyn TryWrite>) -> Self {
Self {
string: String::new(),
out,
buf: String::new(),
}
}

#[inline]
pub(crate) fn with_capacity(capacity: usize) -> alloc::Result<Self> {
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) }
}
}
20 changes: 10 additions & 10 deletions crates/rune/src/runtime/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(());
}

Expand All @@ -164,37 +164,37 @@ 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(());
}

let mut filler = iter::repeat(fill).take(w);

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));
}
}

Expand Down
14 changes: 8 additions & 6 deletions crates/rune/src/runtime/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"));
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1796,7 +1798,7 @@ impl fmt::Debug for Value {
return Ok(());
}

f.write_str(o.as_str())?;
f.write_str(s.as_str())?;
Ok(())
}
}
Expand Down
35 changes: 10 additions & 25 deletions crates/rune/src/runtime/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<alloc::Vec<_>>());

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(())
}

Expand Down Expand Up @@ -3635,30 +3637,13 @@ impl Vm {
/// [Value::string_display] which requires access to a virtual machine.
///
/// ```no_run
/// use rune::{Context, Unit};
/// use rune::runtime::Formatter;
/// use std::sync::Arc;
/// use rune::{Value, Vm};
/// use rune::runtime::{Formatter, VmError};
///
/// let context = Context::with_default_modules()?;
/// let context = Arc::new(context.runtime()?);
///
/// // Normally the unit would be created by compiling some source,
/// // and since this one is empty it'll just error.
/// let unit = Arc::new(Unit::default());
///
/// let mut vm = rune::Vm::new(context, unit);
///
/// let output = vm.call(["main"], ())?;
///
/// // Call the string_display protocol on `output`. This requires
/// // access to a virtual machine since it might use functions
/// // registered in the unit associated with it.
/// let mut f = Formatter::new();
///
/// // Note: We do an extra unwrap because the return value is
/// // `fmt::Result`.
/// vm.with(|| output.string_display(&mut f)).into_result()?;
/// # Ok::<_, rune::support::Error>(())
/// fn use_with(vm: &Vm, output: &Value, f: &mut Formatter) -> Result<(), VmError> {
/// vm.with(|| output.string_display(f)).into_result()?;
/// Ok(())
/// }
/// ```
pub fn with<F, T>(&self, f: F) -> T
where
Expand Down

0 comments on commit a122ec7

Please sign in to comment.