Skip to content

Commit

Permalink
Add f64::sqrt, avoid a bit of cleaning during assembly, parse numeric…
Browse files Browse the repository at this point in the history
…al values correctly (#664)

* Add f64::sqrt, avoid a bit of cleaning during assembly, parse numerical values correctly
* Add ability to show time when running scripts
  • Loading branch information
udoprog authored Feb 17, 2024
1 parent 6c575a0 commit b577618
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 90 deletions.
13 changes: 7 additions & 6 deletions crates/rune/src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ pub(super) struct Flags {
/// Provide detailed tracing for each instruction executed.
#[arg(short, long)]
trace: bool,
/// Time how long the script took to execute.
#[arg(long)]
time: bool,
/// Perform a default dump.
#[arg(short, long)]
dump: bool,
Expand Down Expand Up @@ -228,18 +231,16 @@ pub(super) async fn run(

let errored = match result {
VmResult::Ok(result) => {
let duration = Instant::now().duration_since(last);

if c.verbose {
if c.verbose || args.time {
let duration = Instant::now().saturating_duration_since(last);
writeln!(io.stderr, "== {:?} ({:?})", result, duration)?;
}

None
}
VmResult::Err(error) => {
let duration = Instant::now().duration_since(last);

if c.verbose {
if c.verbose || args.time {
let duration = Instant::now().saturating_duration_since(last);
writeln!(io.stderr, "== ! ({}) ({:?})", error, duration)?;
}

Expand Down
16 changes: 1 addition & 15 deletions crates/rune/src/compile/v1/assemble.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,6 @@ impl<'a, 'hir, 'arena> Ctxt<'a, 'hir, 'arena> {

/// Clean up local variables by preserving the value that is on top and
/// popping the rest.
///
/// The clean operation will preserve the value that is on top of the stack,
/// and pop the values under it.
pub(crate) fn locals_clean(
&mut self,
total_var_count: usize,
Expand Down Expand Up @@ -337,10 +334,8 @@ fn return_<'hir, T>(
hir: T,
asm: impl FnOnce(&mut Ctxt<'_, 'hir, '_>, T, Needs) -> compile::Result<Asm<'hir>>,
) -> compile::Result<()> {
let clean = cx.scopes.total(span)?;

let address = asm(cx, hir, Needs::Value)?.apply_targeted(cx)?;
cx.asm.push(Inst::Return { address, clean }, span)?;
cx.asm.push(Inst::Return { address }, span)?;

// Top address produces an anonymous variable, which is consumed by the
// return statement.
Expand Down Expand Up @@ -390,9 +385,6 @@ fn pat_with_offset<'hir>(

/// Encode a pattern.
///
/// Patterns will clean up their own locals and execute a jump to `false_label`
/// in case the pattern does not match.
///
/// Returns a boolean indicating if the label was used.
#[instrument(span = hir)]
fn pat<'hir>(
Expand Down Expand Up @@ -2124,10 +2116,6 @@ fn expr_return<'hir>(
if let Some(e) = hir {
return_(cx, span, e, expr)?;
} else {
// NB: we actually want total_var_count here since we need to clean up
// _every_ variable declared until we reached the current return.
let clean = cx.scopes.total(span)?;
cx.locals_pop(clean, span)?;
cx.asm.push(Inst::ReturnUnit, span)?;
}

Expand Down Expand Up @@ -2237,13 +2225,11 @@ fn expr_try<'hir>(
span: &dyn Spanned,
needs: Needs,
) -> compile::Result<Asm<'hir>> {
let clean = cx.scopes.total(span)?;
let address = expr(cx, hir, Needs::Value)?.apply_targeted(cx)?;

cx.asm.push(
Inst::Try {
address,
clean,
preserve: needs.value(),
},
span,
Expand Down
25 changes: 25 additions & 0 deletions crates/rune/src/modules/f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub fn module() -> Result<Module, ContextError> {
m.function_meta(max)?;
m.function_meta(min)?;
#[cfg(feature = "std")]
m.function_meta(sqrt)?;
#[cfg(feature = "std")]
m.function_meta(abs)?;
#[cfg(feature = "std")]
m.function_meta(powf)?;
Expand Down Expand Up @@ -216,6 +218,29 @@ fn min(this: f64, other: f64) -> f64 {
this.min(other)
}

/// Returns the square root of a number.
///
/// Returns NaN if `self` is a negative number other than `-0.0`.
///
/// # Examples
///
/// ```rune
/// let positive = 4.0_f64;
/// let negative = -4.0_f64;
/// let negative_zero = -0.0_f64;
///
/// let abs_difference = (positive.sqrt() - 2.0).abs();
///
/// assert!(abs_difference < 1e-10);
/// assert!(negative.sqrt().is_nan());
/// assert!(negative_zero.sqrt() == negative_zero);
/// ```
#[rune::function(instance)]
#[cfg(feature = "std")]
fn sqrt(this: f64) -> f64 {
this.sqrt()
}

/// Computes the absolute value of `self`.
///
/// # Examples
Expand Down
4 changes: 2 additions & 2 deletions crates/rune/src/parse/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ impl<'a> Lexer<'a> {
has_exponent = true;
is_fractional = true;

// Negative exponent.
if matches!(self.iter.peek(), Some('-')) {
// Negative or explicitly positive exponent.
if matches!(self.iter.peek(), Some('-') | Some('+')) {
self.iter.next();
}
}
Expand Down
7 changes: 0 additions & 7 deletions crates/rune/src/runtime/inst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,6 @@ pub enum Inst {
Return {
/// The address of the value to return.
address: InstAddress,
/// Number of variables to clean. If address is top, this should only
/// specify variables in excess of the top variable. Otherwise, this
/// includes the return value.
clean: usize,
},
/// Pop the current stack frame and restore the instruction pointer from it.
///
Expand Down Expand Up @@ -826,9 +822,6 @@ pub enum Inst {
Try {
/// Address to test if value.
address: InstAddress,
/// Variable count that needs to be cleaned in case the operation
/// results in a return.
clean: usize,
/// If the value on top of the stack should be preserved.
preserve: bool,
},
Expand Down
38 changes: 20 additions & 18 deletions crates/rune/src/runtime/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ use core::fmt;
use core::hash;
use core::iter;

use crate::alloc::hashbrown::raw::RawIter;
use crate::alloc::prelude::*;
use crate::alloc::{self, String};
use crate::alloc::{btree_map, BTreeMap};
use crate::alloc::{hash_map, HashMap};

use crate as rune;
use crate::compile::ItemBuf;
Expand All @@ -21,7 +22,7 @@ use crate::Any;
///
/// [`into_iter`]: struct.Object.html#method.into_iter
/// [`Object`]: struct.Object.html
pub type IntoIter = btree_map::IntoIter<String, Value>;
pub type IntoIter = hash_map::IntoIter<String, Value>;

/// A mutable iterator over the entries of a `Object`.
///
Expand All @@ -30,7 +31,7 @@ pub type IntoIter = btree_map::IntoIter<String, Value>;
///
/// [`iter_mut`]: struct.Object.html#method.iter_mut
/// [`Object`]: struct.Object.html
pub type IterMut<'a> = btree_map::IterMut<'a, String, Value>;
pub type IterMut<'a> = hash_map::IterMut<'a, String, Value>;

/// An iterator over the entries of a `Object`.
///
Expand All @@ -39,7 +40,7 @@ pub type IterMut<'a> = btree_map::IterMut<'a, String, Value>;
///
/// [`iter`]: struct.Object.html#method.iter
/// [`Object`]: struct.Object.html
pub type Iter<'a> = btree_map::Iter<'a, String, Value>;
pub type Iter<'a> = hash_map::Iter<'a, String, Value>;

/// An iterator over the keys of a `HashMap`.
///
Expand All @@ -48,7 +49,7 @@ pub type Iter<'a> = btree_map::Iter<'a, String, Value>;
///
/// [`keys`]: struct.Object.html#method.keys
/// [`Object`]: struct.Object.html
pub type Keys<'a> = btree_map::Keys<'a, String, Value>;
pub type Keys<'a> = hash_map::Keys<'a, String, Value>;

/// An iterator over the values of a `HashMap`.
///
Expand All @@ -57,7 +58,7 @@ pub type Keys<'a> = btree_map::Keys<'a, String, Value>;
///
/// [`values`]: struct.Object.html#method.values
/// [`Object`]: struct.Object.html
pub type Values<'a> = btree_map::Values<'a, String, Value>;
pub type Values<'a> = hash_map::Values<'a, String, Value>;

/// Struct representing a dynamic anonymous object.
///
Expand All @@ -82,7 +83,7 @@ pub type Values<'a> = btree_map::Values<'a, String, Value>;
#[repr(transparent)]
#[rune(builtin, static_type = OBJECT_TYPE)]
pub struct Object {
inner: BTreeMap<String, Value>,
inner: HashMap<String, Value>,
}

impl Object {
Expand All @@ -98,7 +99,7 @@ impl Object {
#[rune::function(keep, path = Self::new)]
pub fn new() -> Self {
Self {
inner: BTreeMap::new(),
inner: HashMap::new(),
}
}

Expand All @@ -117,11 +118,11 @@ impl Object {
}

/// Construct a new object with the given capacity.
pub fn with_capacity(#[allow(unused)] capacity: usize) -> alloc::Result<Self> {
pub fn with_capacity(capacity: usize) -> alloc::Result<Self> {
// BTreeMap doesn't support setting capacity on creation but we keep
// this here in case we want to switch store later.
Ok(Self {
inner: BTreeMap::new(),
inner: HashMap::try_with_capacity(capacity)?,
})
}

Expand Down Expand Up @@ -248,7 +249,7 @@ impl Object {
///
/// If the map did not have this key present, `None` is returned.
pub fn insert(&mut self, k: String, v: Value) -> alloc::Result<Option<Value>> {
Ok(self.inner.try_insert(k, v)?)
self.inner.try_insert(k, v)
}

/// Clears the object, removing all key-value pairs. Keeps the allocated
Expand All @@ -260,7 +261,7 @@ impl Object {
}

/// Convert into inner.
pub fn into_inner(self) -> BTreeMap<String, Value> {
pub fn into_inner(self) -> HashMap<String, Value> {
self.inner
}

Expand Down Expand Up @@ -308,30 +309,31 @@ impl Object {
#[rune::function(keep, path = Self::iter)]
pub fn rune_iter(this: Ref<Self>) -> Iterator {
struct Iter {
iter: btree_map::IterRaw<String, Value>,
iter: RawIter<(String, Value)>,
_guard: RawRef,
}

impl iter::Iterator for Iter {
type Item = VmResult<(String, Value)>;

fn next(&mut self) -> Option<Self::Item> {
let (key, value) = self.iter.next()?;

unsafe {
let key = match (*key).try_clone() {
let bucket = self.iter.next()?;
let (key, value) = bucket.as_ref();

let key = match key.try_clone() {
Ok(key) => key,
Err(err) => return Some(VmResult::err(err)),
};

Some(VmResult::Ok((key, (*value).clone())))
Some(VmResult::Ok((key, value.clone())))
}
}
}

// SAFETY: we're holding onto the related reference guard, and making
// sure that it's dropped after the iterator.
let iter = unsafe { this.inner.iter_raw() };
let iter = unsafe { this.inner.raw_table().iter() };
let (_, _guard) = Ref::into_raw(this);

let iter = Iter { iter, _guard };
Expand Down
19 changes: 3 additions & 16 deletions crates/rune/src/runtime/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,28 +397,15 @@ impl Stack {
}
}

// Assert that the stack frame has been restored to the previous top
// at the point of return.
#[tracing::instrument(skip_all)]
pub(crate) fn check_stack_top(&self) -> Result<(), StackError> {
tracing::trace!(stack = self.stack.len(), self.stack_bottom,);

if self.stack.len() == self.stack_bottom {
return Ok(());
}

Err(StackError)
}

/// Pop the current stack top and modify it to a different one.
///
/// This asserts that the size of the current stack frame is exactly zero
/// before restoring it.
#[tracing::instrument(skip_all)]
pub(crate) fn pop_stack_top(&mut self, stack_bottom: usize) -> Result<(), StackError> {
self.check_stack_top()?;
pub(crate) fn pop_stack_top(&mut self, stack_bottom: usize) {
tracing::trace!(stack = self.stack.len(), self.stack_bottom);
self.stack.truncate(self.stack_bottom);
self.stack_bottom = stack_bottom;
Ok(())
}
}

Expand Down
Loading

0 comments on commit b577618

Please sign in to comment.