[rust] Cleanup visitors

compiler/forget/crates/forget_hir/src/instruction.rs

@@ -14,7 +14,7 @@ pub struct Instruction {
 }
 
 impl Instruction {
-    pub fn each_identifier_store<F>(&mut self, mut f: F) -> ()
+    pub fn each_lvalue<F>(&mut self, mut f: F) -> ()
     where
         F: FnMut(&mut IdentifierOperand) -> (),
     {
@@ -29,7 +29,7 @@ impl Instruction {
                 f(&mut instr.lvalue.identifier);
             }
             InstructionValue::Destructure(instr) => {
-                instr.pattern.each_operand(f);
+                instr.pattern.each_operand(&mut f);
             }
             InstructionValue::Array(_)
             | InstructionValue::Binary(_)
@@ -42,9 +42,10 @@ impl Instruction {
             | InstructionValue::JSXElement(_)
             | InstructionValue::Tombstone => {}
         }
+        f(&mut self.lvalue);
     }
 
-    pub fn try_each_identifier_store<F, E>(&mut self, mut f: F) -> Result<(), E>
+    pub fn try_each_lvalue<F, E>(&mut self, mut f: F) -> Result<(), E>
     where
         F: FnMut(&mut IdentifierOperand) -> Result<(), E>,
     {
@@ -58,7 +59,7 @@ impl Instruction {
             InstructionValue::StoreLocal(instr) => {
                 f(&mut instr.lvalue.identifier)?;
             }
-            InstructionValue::Destructure(instr) => instr.pattern.try_each_operand(f)?,
+            InstructionValue::Destructure(instr) => instr.pattern.try_each_operand(&mut f)?,
             InstructionValue::Array(_)
             | InstructionValue::Binary(_)
             | InstructionValue::Call(_)
@@ -70,17 +71,11 @@ impl Instruction {
             | InstructionValue::JSXElement(_)
             | InstructionValue::Tombstone => {}
         }
+        f(&mut self.lvalue)?;
         Ok(())
     }
 
-    pub fn each_identifier_load<F>(&mut self, f: F) -> ()
-    where
-        F: FnMut(&mut IdentifierOperand) -> (),
-    {
-        self.each_operand(f);
-    }
-
-    pub fn each_operand<F>(&mut self, mut f: F) -> ()
+    pub fn each_rvalue<F>(&mut self, mut f: F) -> ()
     where
         F: FnMut(&mut IdentifierOperand) -> (),
     {
@@ -281,7 +276,7 @@ pub enum DestructurePattern {
 }
 
 impl DestructurePattern {
-    pub fn try_each_operand<E, F>(&mut self, mut f: F) -> Result<(), E>
+    pub fn try_each_operand<E, F>(&mut self, f: &mut F) -> Result<(), E>
     where
         F: FnMut(&mut IdentifierOperand) -> Result<(), E>,
     {
@@ -308,7 +303,7 @@ impl DestructurePattern {
         }
         Ok(())
     }
-    pub fn each_operand<F>(&mut self, mut f: F) -> ()
+    pub fn each_operand<F>(&mut self, f: &mut F) -> ()
     where
         F: FnMut(&mut IdentifierOperand) -> (),
     {

compiler/forget/crates/forget_ssa/src/eliminate_redundant_phis.rs

@@ -70,8 +70,8 @@ pub fn eliminate_redundant_phis(env: &Environment, fun: &mut Function) {
 
             for instr_ix in block.instructions.iter() {
                 let instr = &mut hir.instructions[usize::from(*instr_ix)];
-                instr.each_identifier_store(|store| rewrite(&rewrites, &mut store.identifier));
-                instr.each_identifier_load(|load| rewrite(&rewrites, &mut load.identifier));
+                instr.each_lvalue(|lvalue| rewrite(&rewrites, &mut lvalue.identifier));
+                instr.each_rvalue(|rvalue| rewrite(&rewrites, &mut rvalue.identifier));
 
                 if let InstructionValue::Function(fun) = &mut instr.value {
                     for context_identifier in &mut fun.lowered_function.context {

compiler/forget/crates/forget_ssa/src/enter.rs

@@ -1,8 +1,7 @@
 use std::cell::RefCell;
-use std::process::id;
 use std::rc::Rc;
 
-use forget_diagnostics::{invariant, Diagnostic};
+use forget_diagnostics::Diagnostic;
 use forget_hir::{
     BasicBlock, BlockId, BlockRewriter, BlockRewriterAction, Blocks, Environment, Function,
     Identifier, IdentifierData, IdentifierId, IdentifierOperand, InstructionValue, MutableRange,
@@ -52,9 +51,8 @@ fn visit_instructions<'e>(
         builder.start_block(&block);
         for instr_ix in &block.instructions {
             let instr = &mut instructions[usize::from(*instr_ix)];
-            instr.each_identifier_load(|load| builder.visit_load(load));
-            instr.try_each_identifier_store(|store| builder.visit_store(store))?;
-            builder.visit_store(&mut instr.lvalue)?;
+            instr.each_rvalue(|rvalue| builder.visit_load(rvalue));
+            instr.try_each_lvalue(|lvalue| builder.visit_store(lvalue))?;
 
             if let InstructionValue::Function(fun) = &mut instr.value {
                 // Lookup each of the context variables referenced in the function
@@ -158,14 +156,12 @@ impl<'e> Builder<'e> {
 
     fn visit_store(&mut self, lvalue: &mut IdentifierOperand) -> Result<(), Diagnostic> {
         let old_identifier = &lvalue.identifier;
-        // TODO: use Result (?)
-        invariant(!self.unknown.contains(&old_identifier.id), || {
-            Diagnostic::invariant(
+        if self.unknown.contains(&old_identifier.id) {
+            return Err(Diagnostic::invariant(
                 "EnterSSA: Expected identifier to be defined before being used",
                 None,
-            )
-        })?;
-
+            ));
+        }
         if self.context.contains(&old_identifier.id) {
             let new_identifier = self.get_id_at(self.current, old_identifier);
             lvalue.identifier = new_identifier;