Update GetAttr: use Arc<String> instead of String

This lets us share strings more easily. It needs to be Arc (as opposed
to Rc) because these are stored inside of UserFun objects, which need to
be Send (for some reason).

Signed-off-by: Alek Ratzloff <alekratz@gmail.com>

src/compile/compiler.rs

@@ -1,4 +1,5 @@
-use std::collections::VecDeque;
+use std::collections::{HashMap, VecDeque};
+use std::sync::Arc;
 
 use crate::compile::{scope::GlobalScope, thunk::Thunk};
 use crate::obj::prelude::*;
@@ -8,12 +9,24 @@ use crate::vm::inst::*;
 
 pub struct Compiler {
     scope: GlobalScope,
+    attr_names: HashMap<String, Arc<String>>,
 }
 
 impl Compiler {
     pub fn new() -> Self {
         Self {
             scope: Default::default(),
+            attr_names: Default::default(),
+        }
+    }
+
+    fn intern_attr(&mut self, name: &str) -> Arc<String> {
+        if let Some(name) = self.attr_names.get(name) {
+            Arc::clone(name)
+        } else {
+            let name = Arc::new(name.to_string());
+            self.attr_names.insert(name.to_string(), Arc::clone(&name));
+            name
         }
     }
 
@@ -151,7 +164,7 @@ impl Compiler {
             }
             Expr::Get(expr, name) => {
                 let mut thunk = self.emit_expr(expr);
-                thunk.push_back(Inst::GetAttr(name.clone()));
+                thunk.push_back(Inst::GetAttr(self.intern_attr(name)));
                 thunk
             }
             Expr::Atom(atom) => self.emit_atom(atom),

src/vm/inst.rs

@@ -1,6 +1,8 @@
 use crate::obj::ObjPtr;
 use crate::vm::name::Name;
 
+use std::sync::Arc;
+
 #[derive(Debug, Clone)]
 pub enum JumpCondition {
     False,
@@ -20,7 +22,7 @@ pub enum Inst {
     Store(Name),
 
     /// Get an attribute from the top item of the stack.
-    GetAttr(String),
+    GetAttr(Arc<String>),
 
     /// Pops the top item off of the stack, discarding it.
     Pop,
@@ -58,7 +60,7 @@ impl Inst {
             Inst::Push(ptr) => format!("push <{:#x}>", (ptr as *const _ as usize)),
             Inst::Load(name) => format!("load {name:?}"),
             Inst::Store(name) => format!("store {name:?}"),
-            Inst::GetAttr(attr) => format!("getattr {attr}"),
+            Inst::GetAttr(name) => format!("getattr {name}"),
             Inst::Pop => format!("pop"),
             Inst::Call(argc) => format!("call {argc}"),
             Inst::Return => format!("return"),

src/vm/mod.rs

@@ -163,29 +163,20 @@ impl Vm {
                 let value = self.pop_stack().expect("Stack underflow");
                 self.store(name, Some(value));
             }
-            Inst::GetAttr(attr) => {
+            Inst::GetAttr(name) => {
-                // XXX
+                let name = std::sync::Arc::clone(name);
-                // there's an annoying thing with this. We can't use
-                // self.pop_stack() and then use "attr" afterwards because they
-                // belong to the same owner (self).
-                // ALSO:
-                // We can't just do peek_stack() either, because since `attr`
-                // above was borrowed while `self` is mutable. If you want to
-                // borrow `self` immutably, you have to drop the reference above
-                // as well.
-                // So either we have to do some unsafe stuff (not a bad option
-                // here, since nothing is going to move) or clone(). Maybe a
-                // `get_attr()` function is in order, I don't know.
-                // For now we're cloning.
-                let attr = attr.clone();
                 let obj = self
                     .pop_stack()
-                    .expect("Inst::GetAttr executed, but there was no value on top of the stack.");
+                    .expect("Inst::GetAttr, but there was no value on top of the stack.");
-                let value = obj.get(&attr);
+                let value = obj.get(&name);
                 if let Some(value) = value {
                     self.push_stack(value);
                 } else {
-                    todo!("Throw exception: Could not get attr {attr:?} on object {obj:?}");
+                    todo!(
+                        "Throw exception: GetAttr object {:?} did not have attribute {}",
+                        obj,
+                        name,
+                    );
                 }
             }
             Inst::Pop => {