feat(tazjin/rlox): Implement global variable access r/2274

This also includes a fix for an issue where the identifiers of
variables were pushed onto the stack, which is incorrect.

Change-Id: Id89b388268efad295f29978d767aa4b33c4ded14
Reviewed-on: https://cl.tvl.fyi/c/depot/+/2594
Reviewed-by: tazjin <mail@tazj.in>
Tested-by: BuildkiteCI

4 files changed, 48 insertions, 6 deletions

diff --git a/users/tazjin/rlox/src/bytecode/compiler.rs b/users/tazjin/rlox/src/bytecode/compiler.rs
index f993e35557ad..b8b91667d3fa 100644
--- a/users/tazjin/rlox/src/bytecode/compiler.rs
+++ b/users/tazjin/rlox/src/bytecode/compiler.rs
@@ -145,6 +145,10 @@ fn rule_for<T: Iterator<Item = Token>>(token: &TokenKind) -> ParseRule<T> {
             ParseRule::new(None, Some(Compiler::binary), Precedence::Comparison)
         }
 
+        TokenKind::Identifier(_) => {
+            ParseRule::new(Some(Compiler::variable), None, Precedence::None)
+        }
+
         TokenKind::String(_) => {
             ParseRule::new(Some(Compiler::string), None, Precedence::None)
         }
@@ -238,7 +242,7 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
 
     fn number(&mut self) -> LoxResult<()> {
         if let TokenKind::Number(num) = self.previous().kind {
-            self.emit_constant(Value::Number(num));
+            self.emit_constant(Value::Number(num), true);
             return Ok(());
         }
 
@@ -330,11 +334,23 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
         };
 
         let id = self.strings.intern(val);
-        self.emit_constant(Value::String(id.into()));
+        self.emit_constant(Value::String(id.into()), true);
+
+        Ok(())
+    }
 
+    fn named_variable(&mut self) -> LoxResult<()> {
+        let ident = self.identifier_str(Self::previous)?;
+        let constant_id =
+            self.emit_constant(Value::String(ident.into()), false);
+        self.emit_op(OpCode::OpGetGlobal(constant_id));
         Ok(())
     }
 
+    fn variable(&mut self) -> LoxResult<()> {
+        self.named_variable()
+    }
+
     fn parse_precedence(&mut self, precedence: Precedence) -> LoxResult<()> {
         self.advance();
         let rule: ParseRule<T> = rule_for(&self.previous().kind);
@@ -385,7 +401,7 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
         );
 
         let id = self.identifier_str(Self::previous)?;
-        Ok(self.emit_constant(Value::String(id.into())))
+        Ok(self.emit_constant(Value::String(id.into()), false))
     }
 
     fn current_chunk(&mut self) -> &mut Chunk {
@@ -409,9 +425,13 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
         self.current_chunk().add_op(op, line);
     }
 
-    fn emit_constant(&mut self, val: Value) -> usize {
+    fn emit_constant(&mut self, val: Value, with_op: bool) -> usize {
         let idx = self.chunk.add_constant(val);
-        self.emit_op(OpCode::OpConstant(idx));
+
+        if with_op {
+            self.emit_op(OpCode::OpConstant(idx));
+        }
+
         idx
     }
 
diff --git a/users/tazjin/rlox/src/bytecode/opcode.rs b/users/tazjin/rlox/src/bytecode/opcode.rs
index 25ce03c3c182..1c23449e76b3 100644
--- a/users/tazjin/rlox/src/bytecode/opcode.rs
+++ b/users/tazjin/rlox/src/bytecode/opcode.rs
@@ -30,6 +30,7 @@ pub enum OpCode {
     OpPrint,
     OpPop,
 
-    // Variable definitions
+    // Variable management
     OpDefineGlobal(usize),
+    OpGetGlobal(usize),
 }
diff --git a/users/tazjin/rlox/src/bytecode/tests.rs b/users/tazjin/rlox/src/bytecode/tests.rs
index b346da19810d..d5b6ab020389 100644
--- a/users/tazjin/rlox/src/bytecode/tests.rs
+++ b/users/tazjin/rlox/src/bytecode/tests.rs
@@ -108,3 +108,13 @@ fn strings() {
     expect_str("\"hello\";", "hello");
     expect_str("\"hello\" + \" world\";", "hello world");
 }
+
+#[test]
+fn variables() {
+    expect_num("var a = 5; a;", 5.0);
+    expect_num("var a = 5; var b = 2; a * b;", 10.0);
+    expect_str(
+        "var greeting = \"hello\"; var name = \"Zubnog\"; greeting + \" \" + name;",
+        "hello Zubnog",
+    );
+}
diff --git a/users/tazjin/rlox/src/bytecode/vm.rs b/users/tazjin/rlox/src/bytecode/vm.rs
index 53431a083785..0cd0853764e0 100644
--- a/users/tazjin/rlox/src/bytecode/vm.rs
+++ b/users/tazjin/rlox/src/bytecode/vm.rs
@@ -170,6 +170,17 @@ impl VM {
                         self.globals.insert(name, val);
                     });
                 }
+
+                OpCode::OpGetGlobal(name_idx) => {
+                    let name = self.chunk.constant(*name_idx);
+                    with_type!(self, name, Value::String(name), {
+                        let val = match self.globals.get(name) {
+                            None => unimplemented!("variable not found error"),
+                            Some(val) => val.clone(),
+                        };
+                        self.push(val)
+                    });
+                }
             }
 
             #[cfg(feature = "disassemble")]