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.

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

Add f64::sqrt, avoid a bit of cleaning during assembly, parse numerical values correctly #664

Merged
merged 2 commits into from
Feb 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading