Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rune: Make formatter reference a local value #864

Merged
merged 1 commit into from
Oct 30, 2024
Merged

rune: Make formatter reference a local value #864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions crates/rune/src/modules/option.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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