Simplify instruction_needs_mutable_this

valuescript_compiler/src/function_compiler.rs

@@ -13,7 +13,6 @@ use crate::asm::{
 use crate::diagnostic::{Diagnostic, DiagnosticLevel};
 use crate::expression_compiler::CompiledExpression;
 use crate::expression_compiler::ExpressionCompiler;
-use crate::instruction_mutates_this::instruction_mutates_this;
 use crate::name_allocator::{NameAllocator, RegAllocator};
 use crate::scope::{NameId, OwnerId};
 use crate::scope_analysis::{fn_to_owner_id, Name, ScopeAnalysis};
@@ -108,8 +107,8 @@ impl FunctionCompiler {
     };
   }
 
-  pub fn push(&mut self, instruction: Instruction) {
-    if instruction_mutates_this(&instruction) {
+  pub fn push(&mut self, mut instruction: Instruction) {
+    if instruction_needs_mutable_this(&mut instruction) {
       self.push_raw(Instruction::RequireMutableThis);
     }
 
@@ -1215,3 +1214,27 @@ impl FunctionCompiler {
     mutated_registers
   }
 }
+
+fn instruction_needs_mutable_this(
+  // We don't really mutate `instruction`, but we're using visit_registers_mut_rev which doesn't
+  // have a non-mut equivalent. Writing it just for this seems unnecessary.
+  instruction: &mut Instruction,
+) -> bool {
+  if let Instruction::ThisSubCall(_this, _, _, dst) = instruction {
+    // visit_registers_mut_rev flags `this` as write:true since a write can occur, but the whole
+    // purpose of this instruction is to conditionally propagate constness into the next call to
+    // deal with this issue. Therefore, we only check `dst` here.
+
+    return dst.is_this();
+  }
+
+  let mut result = false;
+
+  instruction.visit_registers_mut_rev(&mut |rvm| {
+    if rvm.write && rvm.register.is_this() {
+      result = true;
+    }
+  });
+
+  result
+}

valuescript_compiler/src/instruction_mutates_this.rs

@@ -1,61 +0,0 @@
-use crate::asm::Instruction;
-
-pub fn instruction_mutates_this(instruction: &Instruction) -> bool {
-  use Instruction::*;
-
-  match instruction {
-    End | Jmp(..) | JmpIf(..) | Throw(..) | UnsetCatch | RequireMutableThis => false,
-    Mov(_, reg)
-    | OpInc(reg)
-    | OpDec(reg)
-    | OpPlus(_, _, reg)
-    | OpMinus(_, _, reg)
-    | OpMul(_, _, reg)
-    | OpDiv(_, _, reg)
-    | OpMod(_, _, reg)
-    | OpExp(_, _, reg)
-    | OpEq(_, _, reg)
-    | OpNe(_, _, reg)
-    | OpTripleEq(_, _, reg)
-    | OpTripleNe(_, _, reg)
-    | OpAnd(_, _, reg)
-    | OpOr(_, _, reg)
-    | OpNot(_, reg)
-    | OpLess(_, _, reg)
-    | OpLessEq(_, _, reg)
-    | OpGreater(_, _, reg)
-    | OpGreaterEq(_, _, reg)
-    | OpNullishCoalesce(_, _, reg)
-    | OpOptionalChain(_, _, reg)
-    | OpBitAnd(_, _, reg)
-    | OpBitOr(_, _, reg)
-    | OpBitNot(_, reg)
-    | OpBitXor(_, _, reg)
-    | OpLeftShift(_, _, reg)
-    | OpRightShift(_, _, reg)
-    | OpRightShiftUnsigned(_, _, reg)
-    | TypeOf(_, reg)
-    | InstanceOf(_, _, reg)
-    | In(_, _, reg)
-    | Call(_, _, reg)
-    | Bind(_, _, reg)
-    | Sub(_, _, reg)
-    | SubMov(_, _, reg)
-    | UnaryPlus(_, reg)
-    | UnaryMinus(_, reg)
-    | New(_, _, reg)
-    | Import(_, reg)
-    | ImportStar(_, reg)
-    | SetCatch(_, reg)
-    | ConstSubCall(_, _, _, reg)
-    | ThisSubCall(_, _, _, reg)
-    | Cat(_, reg)
-    | Yield(_, reg)
-    | YieldStar(_, reg) => reg.is_this(),
-
-    Next(iter, res) => iter.is_this() || res.is_this(),
-    UnpackIterRes(_, value_reg, done_reg) => value_reg.is_this() || done_reg.is_this(),
-
-    Apply(_, ctx, _, reg) | SubCall(ctx, _, _, reg) => ctx.is_this() || reg.is_this(),
-  }
-}

valuescript_compiler/src/lib.rs

@@ -9,7 +9,6 @@ mod function_compiler;
 mod gather_modules;
 mod import_pattern;
 mod instruction;
-mod instruction_mutates_this;
 mod link_module;
 mod module_compiler;
 mod name_allocator;