From 95e790d8e85a31e3148e64c54c73771e589e46be Mon Sep 17 00:00:00 2001 From: Erin Date: Fri, 22 Apr 2022 21:06:34 +0200 Subject: [PATCH 1/3] Reimplemented Melo - Variable made into enum of `ValueRef or Melo` - Melo-ing variable causes dropping of the Rc, decreasing reference count --- ablescript/src/interpret.rs | 52 ++++++++++++++++--------------------- ablescript/src/variables.rs | 14 +++------- 2 files changed, 27 insertions(+), 39 deletions(-) diff --git a/ablescript/src/interpret.rs b/ablescript/src/interpret.rs index b15967d8..febd1c7d 100644 --- a/ablescript/src/interpret.rs +++ b/ablescript/src/interpret.rs @@ -198,7 +198,9 @@ impl ExecEnv { // TODO: not too happy with constructing an artificial // Ident here. - Variable(name) => self.get_var(&Spanned::new(name.to_owned(), expr.span.clone()))?, + Variable(name) => { + self.get_var_value(&Spanned::new(name.to_owned(), expr.span.clone()))? + } }) } @@ -275,7 +277,7 @@ impl ExecEnv { return Ok(HaltStatus::Hopback(stmt.span.clone())); } Stmt::Melo(ident) => { - self.get_var_mut(ident)?.melo = true; + *self.get_var_mut(ident)? = Variable::Melo; } Stmt::Rlyeh => { // Maybe print a creepy error message or something @@ -305,7 +307,7 @@ impl ExecEnv { fn assign(&mut self, dest: &Assignable, value: Value) -> Result<(), Error> { match dest.kind { AssignableKind::Variable => { - self.get_var_mut(&dest.ident)?.value.replace(value); + self.get_var_rc(&dest.ident)?.replace(value); } AssignableKind::Index { ref indices } => { let mut cell = self.get_var_rc(&dest.ident)?; @@ -514,7 +516,7 @@ impl ExecEnv { /// Get the value of a variable. Throw an error if the variable is /// inaccessible or banned. - fn get_var(&self, name: &Spanned) -> Result { + fn get_var_value(&self, name: &Spanned) -> Result { // Search for the name in the stack from top to bottom. match self .stack @@ -522,16 +524,11 @@ impl ExecEnv { .rev() .find_map(|scope| scope.variables.get(&name.item)) { - Some(var) => { - if !var.melo { - Ok(var.value.borrow().clone()) - } else { - Err(Error { - kind: ErrorKind::MeloVariable(name.item.to_owned()), - span: name.span.clone(), - }) - } - } + Some(Variable::Ref(r)) => Ok(r.borrow().clone()), + Some(Variable::Melo) => Err(Error { + kind: ErrorKind::MeloVariable(name.item.to_owned()), + span: name.span.clone(), + }), None => Err(Error { kind: ErrorKind::UnknownVariable(name.item.to_owned()), span: name.span.clone(), @@ -550,16 +547,7 @@ impl ExecEnv { .rev() .find_map(|scope| scope.variables.get_mut(&name.item)) { - Some(var) => { - if !var.melo { - Ok(var) - } else { - Err(Error { - kind: ErrorKind::MeloVariable(name.item.to_owned()), - span: name.span.clone(), - }) - } - } + Some(var) => Ok(var), None => Err(Error { kind: ErrorKind::UnknownVariable(name.item.to_owned()), span: name.span.clone(), @@ -570,7 +558,13 @@ impl ExecEnv { /// Get an Rc'd pointer to the value of a variable. Throw an error /// if the variable is inaccessible or banned. fn get_var_rc(&mut self, name: &Spanned) -> Result { - Ok(self.get_var_mut(name)?.value.clone()) + match self.get_var_mut(name)? { + Variable::Ref(r) => Ok(r.clone()), + Variable::Melo => Err(Error { + kind: ErrorKind::MeloVariable(name.item.to_owned()), + span: name.span.clone(), + }), + } } /// Declare a new variable, with the given initial value. @@ -585,7 +579,7 @@ impl ExecEnv { .last() .expect("Declaring variable on empty stack") .variables - .insert(name.to_owned(), Variable { melo: false, value }); + .insert(name.to_owned(), Variable::Ref(value)); } } @@ -709,7 +703,7 @@ mod tests { // Declaring and reading from a variable. eval(&mut env, "dim foo 32; dim bar foo + 1;").unwrap(); assert_eq!( - env.get_var(&Spanned { + env.get_var_value(&Spanned { item: "bar".to_owned(), span: 1..1, }) @@ -720,7 +714,7 @@ mod tests { // Assigning an existing variable. eval(&mut env, "/*hi*/ =: foo;").unwrap(); assert_eq!( - env.get_var(&Spanned { + env.get_var_value(&Spanned { item: "foo".to_owned(), span: 1..1, }) @@ -746,7 +740,7 @@ mod tests { .unwrap(); assert_eq!( - env.get_var(&Spanned { + env.get_var_value(&Spanned { item: "foo".to_owned(), span: 1..1, }) diff --git a/ablescript/src/variables.rs b/ablescript/src/variables.rs index 17fa2c81..2879da0a 100644 --- a/ablescript/src/variables.rs +++ b/ablescript/src/variables.rs @@ -971,19 +971,13 @@ impl ValueRef { } #[derive(Debug)] -pub struct Variable { - pub melo: bool, - - // Multiple Variables can reference the same underlying Value when - // pass-by-reference is used, therefore we use Rc here. - pub value: ValueRef, +pub enum Variable { + Ref(ValueRef), + Melo, } impl Variable { pub fn from_value(value: Value) -> Self { - Self { - melo: false, - value: ValueRef::new(value), - } + Self::Ref(ValueRef::new(value)) } } From 5ccc181f476eb263170f8d1193b6db05f4f3b619 Mon Sep 17 00:00:00 2001 From: Erin Date: Fri, 22 Apr 2022 21:09:03 +0200 Subject: [PATCH 2/3] Rc is got by reference by default --- ablescript/src/interpret.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ablescript/src/interpret.rs b/ablescript/src/interpret.rs index febd1c7d..d0885d05 100644 --- a/ablescript/src/interpret.rs +++ b/ablescript/src/interpret.rs @@ -307,10 +307,10 @@ impl ExecEnv { fn assign(&mut self, dest: &Assignable, value: Value) -> Result<(), Error> { match dest.kind { AssignableKind::Variable => { - self.get_var_rc(&dest.ident)?.replace(value); + self.get_var_rc_mut(&dest.ident)?.replace(value); } AssignableKind::Index { ref indices } => { - let mut cell = self.get_var_rc(&dest.ident)?; + let mut cell = self.get_var_rc_mut(&dest.ident)?.clone(); for index in indices { let index = self.eval_expr(index)?; @@ -366,7 +366,7 @@ impl ExecEnv { .iter() .map(|arg| { if let Expr::Variable(name) = &arg.item { - self.get_var_rc(&Spanned::new(name.to_owned(), arg.span.clone())) + self.get_var_rc_mut(&Spanned::new(name.to_owned(), arg.span.clone())).cloned() } else { self.eval_expr(arg).map(ValueRef::new) } @@ -555,11 +555,11 @@ impl ExecEnv { } } - /// Get an Rc'd pointer to the value of a variable. Throw an error + /// Get an reference to an Rc'd pointer to the value of a variable. Throw an error /// if the variable is inaccessible or banned. - fn get_var_rc(&mut self, name: &Spanned) -> Result { + fn get_var_rc_mut(&mut self, name: &Spanned) -> Result<&mut ValueRef, Error> { match self.get_var_mut(name)? { - Variable::Ref(r) => Ok(r.clone()), + Variable::Ref(r) => Ok(r), Variable::Melo => Err(Error { kind: ErrorKind::MeloVariable(name.item.to_owned()), span: name.span.clone(), From db3b56d79876df9b3b214a8d50392464b8929db3 Mon Sep 17 00:00:00 2001 From: Erin Date: Fri, 22 Apr 2022 21:14:53 +0200 Subject: [PATCH 3/3] Double Melo causes variable deletion --- ablescript/src/interpret.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/ablescript/src/interpret.rs b/ablescript/src/interpret.rs index d0885d05..f666a55e 100644 --- a/ablescript/src/interpret.rs +++ b/ablescript/src/interpret.rs @@ -276,9 +276,16 @@ impl ExecEnv { Stmt::HopBack => { return Ok(HaltStatus::Hopback(stmt.span.clone())); } - Stmt::Melo(ident) => { - *self.get_var_mut(ident)? = Variable::Melo; - } + Stmt::Melo(ident) => match self.get_var_mut(ident)? { + var @ Variable::Ref(_) => *var = Variable::Melo, + Variable::Melo => { + for s in &mut self.stack { + if s.variables.remove(&ident.item).is_some() { + break; + } + } + } + }, Stmt::Rlyeh => { // Maybe print a creepy error message or something // here at some point. ~~Alex @@ -366,7 +373,8 @@ impl ExecEnv { .iter() .map(|arg| { if let Expr::Variable(name) = &arg.item { - self.get_var_rc_mut(&Spanned::new(name.to_owned(), arg.span.clone())).cloned() + self.get_var_rc_mut(&Spanned::new(name.to_owned(), arg.span.clone())) + .cloned() } else { self.eval_expr(arg).map(ValueRef::new) } @@ -536,8 +544,7 @@ impl ExecEnv { } } - /// Get a mutable reference to a variable. Throw an error if the - /// variable is inaccessible or banned. + /// Get a mutable reference to a variable. fn get_var_mut(&mut self, name: &Spanned) -> Result<&mut Variable, Error> { // This function has a lot of duplicated code with `get_var`, // which I feel like is a bad sign...