Merge pull request 'Changed the way how Melo-ing variables works' (#8) from improvements/melo into master

Reviewed-on: https://git.ablecorp.us:443/AbleScript/able-script/pulls/8

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()))?
+            }
         })
     }
 
@@ -274,9 +276,16 @@ impl ExecEnv {
             Stmt::HopBack => {
                 return Ok(HaltStatus::Hopback(stmt.span.clone()));
             }
-            Stmt::Melo(ident) => {
+            Stmt::Melo(ident) => match self.get_var_mut(ident)? {
-                self.get_var_mut(ident)?.melo = true;
+                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
@@ -305,10 +314,10 @@ 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_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)?;
 
@@ -364,7 +373,8 @@ 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)
                 }
@@ -514,7 +524,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<String>) -> Result<Value, Error> {
+    fn get_var_value(&self, name: &Spanned<String>) -> Result<Value, Error> {
         // Search for the name in the stack from top to bottom.
         match self
             .stack
@@ -522,16 +532,11 @@ impl ExecEnv {
             .rev()
             .find_map(|scope| scope.variables.get(&name.item))
         {
-            Some(var) => {
+            Some(Variable::Ref(r)) => Ok(r.borrow().clone()),
-                if !var.melo {
+            Some(Variable::Melo) => Err(Error {
-                    Ok(var.value.borrow().clone())
+                kind: ErrorKind::MeloVariable(name.item.to_owned()),
-                } else {
+                span: name.span.clone(),
-                    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(),
@@ -539,8 +544,7 @@ impl ExecEnv {
         }
     }
 
-    /// Get a mutable reference to a variable. Throw an error if the
+    /// Get a mutable reference to a variable.
-    /// variable is inaccessible or banned.
     fn get_var_mut(&mut self, name: &Spanned<String>) -> Result<&mut Variable, Error> {
         // This function has a lot of duplicated code with `get_var`,
         // which I feel like is a bad sign...
@@ -550,16 +554,7 @@ impl ExecEnv {
             .rev()
             .find_map(|scope| scope.variables.get_mut(&name.item))
         {
-            Some(var) => {
+            Some(var) => Ok(var),
-                if !var.melo {
-                    Ok(var)
-                } else {
-                    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(),
@@ -567,10 +562,16 @@ 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<String>) -> Result<ValueRef, Error> {
+    fn get_var_rc_mut(&mut self, name: &Spanned<String>) -> Result<&mut ValueRef, Error> {
-        Ok(self.get_var_mut(name)?.value.clone())
+        match self.get_var_mut(name)? {
+            Variable::Ref(r) => Ok(r),
+            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 +586,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 +710,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 +721,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 +747,7 @@ mod tests {
         .unwrap();
 
         assert_eq!(
-            env.get_var(&Spanned {
+            env.get_var_value(&Spanned {
                 item: "foo".to_owned(),
                 span: 1..1,
             })

ablescript/src/variables.rs

@@ -971,19 +971,13 @@ impl ValueRef {
 }
 
 #[derive(Debug)]
-pub struct Variable {
+pub enum Variable {
-    pub melo: bool,
+    Ref(ValueRef),
-
+    Melo,
-    // Multiple Variables can reference the same underlying Value when
-    // pass-by-reference is used, therefore we use Rc here.
-    pub value: ValueRef,
 }
 
 impl Variable {
     pub fn from_value(value: Value) -> Self {
-        Self {
+        Self::Ref(ValueRef::new(value))
-            melo: false,
-            value: ValueRef::new(value),
-        }
     }
 }