Skip to content

Commit

Permalink
refactor internal vector representation
Browse files Browse the repository at this point in the history
  • Loading branch information
sebffischer authored Oct 9, 2024
1 parent fcbafb9 commit ebf4bc4
Show file tree
Hide file tree
Showing 24 changed files with 1,892 additions and 692 deletions.
22 changes: 22 additions & 0 deletions src/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,28 @@

## Changes

* Named vectors were added and can e.g. be constructed via `[a = 1, b = 2]`
* The `is_null()` primitive was added
* Setting a list value to `null` actually sets it to `null` and does not remove it.

## Internals

* The `List` is now represented as a `Rep<Obj>`, unifying heterogenous and atomic vectors.
This included a considerable refactor.
* Iterating over references of a `Rep<T>` was made much simpler and new methods were added
and unused ones removed.

## Notable Bugs Addressed

* Concatenating `list`s now works as expected (#177).
* `names()` now works correctly when the vector is subset, e.g.
`names(list(a = 1, b = 2)[1])` (#181).
* `[[.<-` assignment has been fixed for lists (#179)

# 0.4.0

## Changes

* Added German (`ger`) localization.

* Digits can now be separated by an underscore to improve readability.
Expand Down
1 change: 1 addition & 0 deletions src/callable/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub static BUILTIN: LazyLock<HashMap<&'static str, Box<dyn Builtin>>> = LazyLock
("callstack", Box::new(PrimitiveCallstack) as Box<dyn Builtin>),
("environment", Box::new(PrimitiveEnvironment) as Box<dyn Builtin>),
("eval", Box::new(PrimitiveEval) as Box<dyn Builtin>),
("is_null", Box::new(PrimitiveIsNull) as Box<dyn Builtin>),
("length", Box::new(PrimitiveLength) as Box<dyn Builtin>),
("list", Box::new(PrimitiveList) as Box<dyn Builtin>),
("names", Box::new(PrimitiveNames) as Box<dyn Builtin>),
Expand Down
85 changes: 33 additions & 52 deletions src/callable/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ extern crate r_derive;
use crate::callable::dyncompare::*;
use crate::cli::Experiment;
use crate::context::Context;
use crate::object::List;
use crate::object::{Expr, ExprList, Obj};
use crate::object::types::{Character, Integer};
use crate::object::{Expr, ExprList, List, Obj, Subset};
use crate::{internal_err, lang::*};

impl std::fmt::Debug for Box<dyn Callable> {
Expand Down Expand Up @@ -38,81 +38,66 @@ pub trait CallableFormals {
pub trait Callable: CallableFormals {
fn match_args(&self, args: List, stack: &mut CallStack) -> Result<(List, List), Signal> {
let mut formals = self.formals();
let ellipsis: List = vec![].into();
let matched_args: List = vec![].into();
let ellipsis: List = List::new();
let matched_args: List = List::new();

// assign named args to corresponding formals
let mut i: usize = 0;
'outer: while i < args.values.len() {
'inner: {
// check argname with immutable borrow, but drop scope. If
// found, drop borrow so we can mutably assign it
if let (Some(argname), _) = &args.values.borrow()[i] {
if let Some((Some(_), _)) = formals.remove_named(argname) {
break 'inner;
}
}

i += 1;
continue 'outer;
}
let mut indices: Vec<i32> = Vec::new();

matched_args
.values
.with_inner_mut(|v| v.push(args.values.with_inner_mut(|x| x.remove(i))))
for (i, (maybe_name, value)) in args.pairs_ref().iter().enumerate() {
if let Character::Some(name) = maybe_name {
if let Some((Some(_), _)) = formals.remove_named(name) {
matched_args.push_named(Character::Some(name.clone()), value.clone());
continue;
}
}
indices.push(i as i32);
}

let indices: Vec<Integer> = indices.into_iter().map(Integer::Some).collect();
let subset = Subset::Indices(indices.into());
let args = args.subset(subset).materialize();

// TODO(bug): need to evaluate trailing unassigned params that have
// a default value before popping off remaining trailing params

// remove any Ellipsis param, and any trailing unassigned params
let remainder = formals.pop_trailing();

// backfill unnamed args, populating ellipsis with overflow
let argsiter = args.values.into_iter();
for (key, value) in argsiter {
for (key, value) in args.iter_pairs() {
match key {
// named args go directly to ellipsis, they did not match a formal
Some(arg) => {
ellipsis
.values
.with_inner_mut(|v| v.push((Some(arg), value)));
}
Character::Some(arg) => ellipsis.push_named(Character::Some(arg), value),

// unnamed args populate next formal, or ellipsis if formals exhausted
None => {
Character::NA => {
let next_unassigned_formal = formals.remove(0);
if let Some((Some(param), _)) = next_unassigned_formal {
matched_args
.values
.with_inner_mut(|vals| vals.push((Some(param), value)));
matched_args.push_named(Character::Some(param), value);
} else {
ellipsis
.values
.with_inner_mut(|vals| vals.push((None, value)));
ellipsis.push_named(Character::NA, value);
}
}
}
}

// add back in parameter defaults that weren't filled with args
for (param, default) in formals.into_iter() {
matched_args.values.with_inner_mut(|v| {
v.push((
param,
Obj::Promise(None, default, stack.last_frame().env().clone()),
));
})
matched_args.push_named(
param.into(),
Obj::Promise(None, default, stack.last_frame().env().clone()),
)
}

if let Some(Expr::Ellipsis(Some(name))) = remainder.get(0) {
matched_args
.values
.with_inner_mut(|v| v.push((Some(name), Obj::List(ellipsis.clone()))))
matched_args.push_named(Character::Some(name), Obj::List(ellipsis.clone()));
} else if !remainder.is_empty() {
matched_args
.values
.with_inner_mut(|v| v.push((Some("...".to_string()), Obj::List(ellipsis.clone()))))
matched_args.push_named(
Character::Some("...".to_string()),
Obj::List(ellipsis.clone()),
);
}

Ok((matched_args, ellipsis))
Expand Down Expand Up @@ -276,14 +261,10 @@ where
impl CallableFormals for String {}
impl Builtin for String {}

pub fn force_promises(
vals: List,
stack: &mut CallStack,
) -> Result<Vec<(Option<String>, Obj)>, Signal> {
pub fn force_promises(vals: List, stack: &mut CallStack) -> Result<Vec<(Character, Obj)>, Signal> {
// Force any closures that were created during call. This helps with using
// variables as argument for sep and collapse parameters.
vals.values
.into_iter()
vals.iter_pairs()
.map(|(k, v)| match (k, v.force(stack)) {
(k, Ok(v)) => Ok((k, v)),
(_, Err(e)) => Err(e),
Expand Down
60 changes: 53 additions & 7 deletions src/callable/operators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ impl Callable for InfixDollar {
let mut what = stack.eval(what)?;

match index {
Expr::String(s) | Expr::Symbol(s) => what.try_get_named(s.as_str()),
Expr::String(s) | Expr::Symbol(s) => what.try_get_named(&s),
_ => Ok(Obj::Null),
}
}
Expand All @@ -381,7 +381,7 @@ impl Callable for InfixDollar {
let mut what = stack.eval_mut(what)?;

match index {
Expr::String(s) | Expr::Symbol(s) => what.try_get_named(s.as_str()),
Expr::String(s) | Expr::Symbol(s) => what.try_get_named_mut(s.as_str()),
_ => Ok(Obj::Null),
}
}
Expand All @@ -401,10 +401,7 @@ impl Callable for InfixDollar {
let mut what = stack.eval_mut(what)?;

match name {
Expr::String(s) | Expr::Symbol(s) => {
what.set_named(s.as_str(), value)?;
Ok(what)
}
Expr::String(s) | Expr::Symbol(s) => what.try_set_named(&s, value),
_ => unimplemented!(),
}
}
Expand Down Expand Up @@ -436,6 +433,30 @@ impl Callable for PostfixIndex {
let index = stack.eval(x.1)?;
what.try_get_inner_mut(index)
}

fn call_assign(&self, value: Expr, args: ExprList, stack: &mut CallStack) -> EvalResult {
let mut argstream = args.into_iter();

let Some((_, what)) = argstream.next() else {
unreachable!();
};

let Some((_, index)) = argstream.next() else {
unreachable!();
};

let value = stack.eval(value)?;
let what = stack.eval_mut(what)?;
let index = stack.eval(index)?;

let subset = index.try_into()?;

Ok(match what {
Obj::List(mut v) => v.set_subset(subset, value)?,
Obj::Vector(mut v) => v.set_subset(subset, value).map(Obj::Vector)?,
_ => unimplemented!(),
})
}
}

#[derive(Debug, Clone, PartialEq)]
Expand All @@ -460,12 +481,37 @@ impl Callable for PostfixVecIndex {
mod tests {
use crate::error::Error;
use crate::lang::{EvalResult, Signal};
use crate::r;
use crate::{r, r_expect};
#[test]
fn colon_operator() {
assert_eq!(EvalResult::Err(Signal::Error(Error::InvalidRange)), r!(1:0));
assert_eq!(r!([1, 2]), r!(1:2));
assert_eq!(r!([1]), r!(1:1));
assert_eq!(r!(1:-2:-3), r!([1, -1, -3]));
}

#[test]
fn dollar_assign() {
r_expect! {{"
l = (a = 1, )
x = (l$a = 2)
l$a == 2 & x == 2
"}}
}
#[test]
fn dollar_assign_nested() {
r_expect! {{"
l = (a = (b = 1,),)
x = (l$a$b = 2)
l$a$b == 2 & x == 2
"}}
}

#[test]
fn dollar_access() {
r_expect! {{"
l = (a = 1, )
l$a == 1
"}}
}
}
Loading

0 comments on commit ebf4bc4

Please sign in to comment.