Clean up storage model (#31)

Properly implement the scheme storage model.
This adds a representation of set-able places, that can be mutated with the set! core form.
It implements vector, list (proper and improper) as having their elements represented as places.
In contrast to scheme, where strings are essentially sequences of settable chars, we left strings as immutable.

README.md

@@ -45,7 +45,7 @@ make repl
 - [x] basic code generator 
 - [ ] collect all errors in reader  
 - [ ] collect all errors in parser 
-- [ ] bytecode serialization
+- [ ] bytecode serialization (ahead of time compilation)
 - [ ] libraries  
 - [ ] optimization pass 
 - [x] identify tail calls
@@ -86,7 +86,7 @@ I probably should build one or maybe not.
 ### Problems
 There are plenty :D 
 
-* The storage model isn't as it must be. List/Vector/Byte-Vector cells are currently not locations. It's not possible to set! those currently. In fact the parser even rejects to set! anything other than an identifier. So this means currently only variables are locations. This can be changed and as a result will also simplify the model to set variables as no special handling will be needed anymore in the VM.
+* Equality isn't correctly implemented -> I will add a test file that verifies that the language implements as the r7rs specifies it, which will cover equality as well.
 * Runtime representation of values isn't at all optimised. It likely uses way too many allocations and holds the types wrong
 * The parser bails after the first error (you'd want to collect all)
 * I'm not sure if the custom ParseResult I use is really needed? (I will have to figure that out)

benches/vm_benchmark.rs

@@ -32,19 +32,19 @@ fn writer_benchmark(c: &mut Criterion) {
     ]);
 
     c.bench_function("Writer#write_list", |b| {
-        b.iter(|| black_box(writer.write(&list, &factory)))
+        b.iter(|| black_box(writer.write(&list)))
     });
 
     let vector_input = factory.vector(vec![value::Value::Bool(true), value::Value::Char('z')]);
     c.bench_function("Writer#write_vector", |b| {
-        b.iter(|| black_box(writer.write(&vector_input, &factory)))
+        b.iter(|| black_box(writer.write(&vector_input)))
     });
 
     let symbol_input = factory.symbol(" a weird symbol  ");
     c.bench_with_input(
         BenchmarkId::new("Writer#write_symbol", format!("{:?}", symbol_input)),
         &symbol_input,
-        |b, i| b.iter(|| black_box(writer.write(i, &factory))),
+        |b, i| b.iter(|| black_box(writer.write(i))),
     );
 }
 

src/compiler/backend/code_generator.rs

@@ -11,6 +11,7 @@
 /// computation of addresses.
 use thiserror::Error;
 
+use super::variables::{Variables, VariablesRef};
 use crate::compiler::frontend::parser::{
     apply::ApplicationExpression,
     assignment::SetExpression,
@@ -36,8 +37,6 @@ use crate::vm::value::closure::Closure;
 use crate::vm::value::procedure::Arity;
 use crate::vm::value::Value;
 
-use super::variables::{Variables, VariablesRef};
-
 #[derive(Error, Debug, Clone)]
 pub enum Error {
     #[error("Too many locals defined")]
@@ -223,7 +222,7 @@ impl<'a> CodeGenerator<'a> {
     fn emit_instructions(&mut self, ast: &Expression, context: &Context) -> Result<()> {
         match ast {
             Expression::Identifier(id) => self.emit_get_variable(id)?,
-            Expression::Assign(expr) => self.emit_set_variable(expr, context)?,
+            Expression::Assign(expr) => self.emit_set(expr, context)?,
             Expression::Literal(lit) => self.emit_lit(lit.datum())?,
             Expression::If(if_expr) => self.emit_if(if_expr, context)?,
             Expression::Define(definition) => self.emit_definition(definition)?,
@@ -307,9 +306,9 @@ impl<'a> CodeGenerator<'a> {
         }
 
         let instr = if context.is_tail_context() {
-            Instruction::TailCall(application.operands.len())
+            Instruction::ApplyTCO(application.operands.len())
         } else {
-            Instruction::Call(application.operands.len())
+            Instruction::Apply(application.operands.len())
         };
 
         self.emit_instruction(instr, application.source_location())?;
@@ -363,21 +362,10 @@ impl<'a> CodeGenerator<'a> {
         }
     }
 
-    fn emit_set_variable(&mut self, expr: &SetExpression, context: &Context) -> Result<()> {
-        // push the value of the expression
+    fn emit_set(&mut self, expr: &SetExpression, context: &Context) -> Result<()> {
+        self.emit_instructions(&expr.location, context)?;
         self.emit_instructions(&expr.value, context)?;
-
-        // is it local?
-        if let Some(addr) = self.resolve_local(&expr.name) {
-            self.emit_instruction(Instruction::SetLocal(addr), expr.source_location())
-        } else if let Some(addr) = self.resolve_up_value(&expr.name)? {
-            self.emit_instruction(Instruction::SetUpValue(addr), expr.source_location())
-        } else {
-            // top level variable
-            let id_sym = self.sym(&expr.name.string());
-            let const_addr = self.current_chunk().add_constant(id_sym);
-            self.emit_instruction(Instruction::SetGlobal(const_addr), expr.source_location())
-        }
+        self.emit_instruction(Instruction::Set, expr.source_location())
     }
 
     fn emit_body(&mut self, body: &BodyExpression) -> Result<()> {
@@ -518,7 +506,7 @@ mod tests {
                 Instruction::Pop,
                 Instruction::GetGlobal(_),
                 // (foo)
-                Instruction::TailCall(_),
+                Instruction::ApplyTCO(_),
                 Instruction::Jump(_),
                 Instruction::Pop,
                 Instruction::Const(_),
@@ -536,11 +524,11 @@ mod tests {
                 Instruction::Pop,
                 Instruction::GetGlobal(_),
                 // (foo)
-                Instruction::Call(_),
+                Instruction::Apply(_),
                 Instruction::Jump(_),
                 Instruction::Pop,
                 Instruction::GetGlobal(_),
-                Instruction::TailCall(_),
+                Instruction::ApplyTCO(_),
                 Instruction::Return
             ]
         )
@@ -555,7 +543,7 @@ mod tests {
             [
                 // (bar)
                 Instruction::GetGlobal(_),
-                Instruction::TailCall(_),
+                Instruction::ApplyTCO(_),
                 Instruction::Return
             ]
         );
@@ -567,9 +555,9 @@ mod tests {
             [
                 // (bar)
                 Instruction::GetGlobal(_),
-                Instruction::Call(_),
+                Instruction::Apply(_),
                 Instruction::GetGlobal(_),
-                Instruction::TailCall(_),
+                Instruction::ApplyTCO(_),
                 Instruction::Return
             ]
         );
@@ -589,9 +577,9 @@ mod tests {
                 Instruction::Closure(_),
                 Instruction::GetGlobal(_),
                 // (foo)
-                Instruction::Call(_),
+                Instruction::Apply(_),
                 // the body of the (let)
-                Instruction::TailCall(_),
+                Instruction::ApplyTCO(_),
                 Instruction::Return
             ]
         );
@@ -601,9 +589,9 @@ mod tests {
             &let_closure.code().code[..],
             [
                 Instruction::GetGlobal(_),
-                Instruction::Call(_),
+                Instruction::Apply(_),
                 Instruction::GetGlobal(_),
-                Instruction::TailCall(_),
+                Instruction::ApplyTCO(_),
                 Instruction::Return
             ]
         )
@@ -630,12 +618,12 @@ mod tests {
                 // create closure
                 Instruction::Closure(_), // define closure
                 Instruction::True,
-                Instruction::Call(_), // call closure
+                Instruction::Apply(_), // call closure
                 // define foo
                 Instruction::Define(_),
                 // apply foo
                 Instruction::GetGlobal(_),
-                Instruction::TailCall(_),
+                Instruction::ApplyTCO(_),
                 Instruction::Return
             ]
         );
@@ -646,7 +634,7 @@ mod tests {
             [
                 Instruction::GetGlobal(_), // get y
                 Instruction::Const(_),     // 'foo
-                Instruction::Call(_),      // apply 'foo to y
+                Instruction::Apply(_),     // apply 'foo to y
                 // setup the up-value for the x constant
                 Instruction::UpValue(0, true), //x #t
                 // build the closure

src/compiler/frontend/expander.rs

@@ -13,6 +13,7 @@ use crate::compiler::frontend::syntax;
 use crate::compiler::frontend::syntax::symbol::Symbol;
 use crate::compiler::source::{HasSourceLocation, Location};
 use crate::vm::scheme::ffi::{binary_procedure, unary_procedure};
+use crate::vm::value::access::Access;
 use crate::vm::value::procedure::{foreign, Arity, Procedure};
 use crate::vm::value::Value;
 use crate::vm::VM;
@@ -130,7 +131,7 @@ impl Expander {
     fn create_renamer(&mut self) -> Procedure {
         Procedure::foreign(foreign::Procedure::new(
             "rename",
-            |values| unary_procedure(&values).map(|v| v.clone()),
+            |values| unary_procedure(&values).map(|v| Access::ByVal(v.clone())),
             Arity::Exactly(1),
         ))
     }
@@ -139,7 +140,7 @@ impl Expander {
     fn create_comparator(&mut self) -> Procedure {
         Procedure::foreign(foreign::Procedure::new(
             "compare",
-            |values| binary_procedure(&values).map(|(l, r)| Value::Bool(l == r)),
+            |values| binary_procedure(&values).map(|(l, r)| Access::from(Value::Bool(l == r))),
             Arity::Exactly(2),
         ))
     }

src/compiler/frontend/expander/define.rs

@@ -76,10 +76,18 @@ impl Expander {
                 }
             }
             //(define <id> <expr>)
-            [identifier, expr] => Ok(Datum::list(
-                vec![operator.clone(), identifier.clone(), expr.clone()],
-                datum.source_location().clone(),
-            )),
+            [location, expr] => {
+                let expanded_location = self.expand_macros(location)?;
+                let expanded_expr = self.expand_macros(expr)?;
+                Ok(Datum::list(
+                    vec![
+                        operator.clone(),
+                        expanded_location.clone(),
+                        expanded_expr.clone(),
+                    ],
+                    datum.source_location().clone(),
+                ))
+            }
             _ => Err(Error::expansion_error(
                 "Expected definition or procedure definition",
                 &datum,

src/compiler/frontend/expander/letexp.rs

@@ -5,6 +5,7 @@ use crate::compiler::frontend::syntax::symbol::Symbol;
 use crate::compiler::frontend::syntax::Transformer;
 use crate::compiler::source::{HasSourceLocation, Location};
 use crate::vm::scheme::ffi::{explicit_rename_transformer, FunctionResult};
+use crate::vm::value::access::Access;
 use crate::vm::value::error;
 use crate::vm::value::procedure::{foreign, Arity, Procedure};
 use crate::vm::value::Value;
@@ -24,7 +25,7 @@ fn make_let_expander() -> Procedure {
     ))
 }
 
-fn expand_let(args: Vec<Value>) -> FunctionResult<Value> {
+fn expand_let(args: Vec<Value>) -> FunctionResult<Access<Value>> {
     explicit_rename_transformer(&args).and_then({
         |(datum, _rename, _compare)| match datum.list_slice() {
             // Named let
@@ -42,10 +43,7 @@ fn expand_let(args: Vec<Value>) -> FunctionResult<Value> {
                 let mut application = vec![lambda];
                 application.extend(values);
 
-                Ok(Value::syntax(Datum::list(
-                    application,
-                    datum.source_location().clone(),
-                )))
+                Ok(Value::syntax(Datum::list(application, datum.source_location().clone())).into())
             }
             _ => Err(error::argument_error(
                 Value::Syntax(datum.clone()),
@@ -113,4 +111,14 @@ mod tests {
         assert_expands_equal("(let () #t)", "((lambda () #t))", false)?;
         Ok(())
     }
+
+    #[test]
+    fn test_let_after_define() -> Result<()> {
+        assert_expands_equal(
+            "(define x (let ((f #t)) f))",
+            "(define x ((lambda (f) f) #t))",
+            false,
+        )?;
+        Ok(())
+    }
 }

src/compiler/frontend/parser/assignment.rs

@@ -4,41 +4,36 @@ use crate::compiler::frontend::reader::datum::Datum;
 use crate::compiler::source::{HasSourceLocation, Location};
 
 use super::frontend::error::Error;
-use super::identifier;
 use super::Expression;
 use super::ParseResult;
 use super::Result;
 
 #[derive(Clone, PartialEq, Debug)]
 pub struct SetExpression {
-    pub name: identifier::Identifier,
+    pub location: Box<Expression>,
     pub value: Box<Expression>,
-    location: Location,
+    source_location: Location,
 }
 
 impl SetExpression {
-    pub fn new(id: identifier::Identifier, expr: Expression, loc: Location) -> Self {
+    pub fn new(location: Expression, expr: Expression, loc: Location) -> Self {
         SetExpression {
-            name: id,
+            location: Box::new(location),
             value: Box::new(expr),
-            location: loc,
+            source_location: loc,
         }
     }
 }
 
 impl HasSourceLocation for SetExpression {
     fn source_location(&self) -> &Location {
-        &self.location
+        &self.source_location
     }
 }
 
 impl Expression {
-    pub fn assign<L: Into<Location>>(
-        id: identifier::Identifier,
-        expr: Expression,
-        loc: L,
-    ) -> Expression {
-        Expression::Assign(SetExpression::new(id, expr, loc.into()))
+    pub fn assign<L: Into<Location>>(location: Expression, expr: Expression, loc: L) -> Expression {
+        Expression::Assign(SetExpression::new(location, expr, loc.into()))
     }
 }
 
@@ -57,14 +52,14 @@ impl CoreParser {
 
     fn do_parse_set(&mut self, datum: &Datum) -> Result<SetExpression> {
         match self.parse_list(datum)? {
-            [_, identifier, expr] => Ok(SetExpression::new(
-                self.do_parse_identifier(identifier).res()?,
+            [_, location, expr] => Ok(SetExpression::new(
+                self.parse(location)?,
                 self.parse(expr)?,
                 datum.source_location().clone(),
             )),
 
             _other => Err(Error::parse_error(
-                "Expected (set! <identifier> <expression>)",
+                "Expected (set! <expression> <expression>)",
                 Detail::new("", datum.source_location().clone()),
                 vec![],
             )),
@@ -82,7 +77,7 @@ mod tests {
         assert_parse_as(
             "(set! foo #t)",
             Expression::assign(
-                identifier::Identifier::synthetic("foo"),
+                Expression::identifier("foo".to_string(), 6..9),
                 Expression::literal(Datum::boolean(true, 10..12)),
                 0..13,
             ),

src/compiler/frontend/reader/datum.rs

@@ -129,7 +129,7 @@ impl Datum {
             Value::ProperList(ls) => {
                 let elts: Option<Vec<_>> = ls
                     .iter()
-                    .map(|e| Self::from_value(e, location.clone()))
+                    .map(|e| Self::from_value(&e.get_inner_ref(), location.clone()))
                     .collect();
                 elts.map(|e| Self::list(e, location))
             }

src/repl/command.rs

@@ -82,13 +82,16 @@ impl Commands {
 
     fn handle_disass(&self, ident: &str, vm: &mut VM) -> anyhow::Result<()> {
         match vm.top_level.get(&vm.values.sym(ident)) {
-            Some(Value::Closure(closure)) => match vm.disassemble(closure.procedure()) {
-                Err(e) => Err(anyhow!("{}", e)),
-                _ => Ok(()),
-            },
-            Some(Value::Procedure(Procedure::Native(proc))) => match vm.disassemble(&(*proc)) {
-                Err(e) => Err(anyhow!("{}", e)),
-                _ => Ok(()),
+            Some(v) => match &*v.get_inner_ref() {
+                Value::Closure(closure) => match vm.disassemble(closure.procedure()) {
+                    Err(e) => Err(anyhow!("{}", e)),
+                    _ => Ok(()),
+                },
+                Value::Procedure(Procedure::Native(proc)) => match vm.disassemble(&(*proc)) {
+                    Err(e) => Err(anyhow!("{}", e)),
+                    _ => Ok(()),
+                },
+                _ => Err(anyhow!("Can't disassemble non-procedure")),
             },
             _ => Err(anyhow!("Can't disassemble non-procedure")),
         }