lower switch using if-else instead of its own opcode

the previous lowering was wrong, this fixes it

crates/dash_compiler/src/builder.rs

@@ -24,7 +24,10 @@ pub enum Label {
     LoopIncrement {
         loop_id: usize,
     },
-    SwitchCase {
+    SwitchCaseCondition {
+        case_id: u16,
+    },
+    SwitchCaseCode {
         case_id: u16,
     },
     SwitchEnd {

crates/dash_compiler/src/instruction.rs

@@ -293,12 +293,6 @@ impl<'cx, 'interner> InstructionBuilder<'cx, 'interner> {
         Ok(())
     }
 
-    pub fn build_switch(&mut self, case_count: u16, has_default: bool) {
-        self.write(Instruction::Switch as u8);
-        self.writew(case_count);
-        self.write(has_default.into());
-    }
-
     pub fn build_objdestruct(&mut self, count: u16) {
         self.write_instr(Instruction::ObjDestruct);
         self.writew(count);

crates/dash_compiler/src/lib.rs

@@ -21,8 +21,8 @@ use dash_middle::parser::expr::{
 use dash_middle::parser::statement::{
     BlockStatement, Class, ClassMemberKind, DoWhileLoop, ExportKind, ForInLoop, ForLoop, ForOfLoop, FuncId,
     FunctionDeclaration, FunctionKind, IfStatement, ImportKind, Loop, Parameter, ReturnStatement, SpecifierKind,
-    Statement, StatementKind, SwitchStatement, TryCatch, VariableBinding, VariableDeclaration, VariableDeclarationKind,
-    VariableDeclarationName, VariableDeclarations, WhileLoop,
+    Statement, StatementKind, SwitchCase, SwitchStatement, TryCatch, VariableBinding, VariableDeclaration,
+    VariableDeclarationKind, VariableDeclarationName, VariableDeclarations, WhileLoop,
 };
 use dash_middle::sourcemap::Span;
 use dash_middle::visitor::Visitor;
@@ -2046,59 +2046,113 @@ impl<'interner> Visitor<Result<(), Error>> for FunctionCompiler<'interner> {
         SwitchStatement { expr, cases, default }: SwitchStatement,
     ) -> Result<(), Error> {
         let mut ib = InstructionBuilder::new(self);
-
         let switch_id = ib.current_function_mut().prepare_switch();
-        let has_default = default.is_some();
-        let case_count = cases.len();
-
-        // First, compile all case expressions in reverse order, so that the first pop() in the vm
-        // will be the first case value
-        for case in cases.iter().rev() {
-            ib.accept_expr(case.value.clone())?;
-        }
-
-        // Then, compile switch expression
-        ib.accept_expr(expr)?;
-
-        // Write switch metadata (case count, has default case)
-        let case_count = case_count
-            .try_into()
-            .map_err(|_| Error::SwitchCaseLimitExceeded(span))?;
-
-        ib.build_switch(case_count, has_default);
-
-        // Then, build jump headers for every case
-        for (case_id, ..) in cases.iter().enumerate() {
-            ib.build_jmp_header(
-                Label::SwitchCase {
-                    case_id: case_id as u16,
-                },
-                true,
-            );
-        }
-        if has_default {
-            ib.build_jmp_header(Label::SwitchCase { case_id: case_count }, true);
-        }
-
-        // If no case matches, then jump to SwitchEnd
-        ib.build_jmp(Label::SwitchEnd { switch_id }, false);
 
-        // Finally, compile case bodies
-        // All of the case bodies must be adjacent because of fallthrough
-        for (case_id, case) in cases.into_iter().enumerate() {
-            ib.add_local_label(Label::SwitchCase {
-                case_id: case_id as u16,
-            });
-            ib.accept_multiple(case.body)?;
-        }
-        if let Some(default) = default {
-            ib.add_local_label(Label::SwitchCase { case_id: case_count });
-            ib.accept_multiple(default)?;
-        }
+        // TODO: this currently always calls the naive implementation.
+        // We can be smarter in certain cases, such as if all cases are literals,
+        // we can create some kind of table.
+        compile_switch_naive(&mut ib, span, expr, cases, default)?;
 
         ib.current_function_mut()
             .add_global_label(Label::SwitchEnd { switch_id });
         ib.current_function_mut().exit_switch();
+
         Ok(())
     }
 }
+
+/// "Naive" switch lowering:
+/// ```js
+/// switch(1) {
+///     case w: /* FALLTHROUGH */
+///     case x:
+///         return 'case 1';
+///         break;
+///     case y:
+///         return 'case 2';
+///         break;
+///     default:
+///         return 'default';
+/// }
+/// ```
+/// to
+/// ```
+/// _1 = condition
+///
+/// case_w_cond:
+///     eq (ld _1 ld w)
+///     jmpfalsep case_x_cond
+///     # code for case w
+///		# if there's a break anywhere in here, jump to end of switch
+///     jmp case_x
+///
+///	case_x_cond:
+///		eq (ld_1 ld x)
+///		jmpfalsep case_y_cond
+///  case_x:
+///     # code for case x
+///		constant 'case 1'
+///		ret
+///		jmp case_y
+///
+/// case_x_code:
+///     ...
+/// case_y_cond:
+///		eq (ld_1 ld y)
+///		jmpfalsep default
+///	case y:
+/// 	...
+///		jmp default
+///
+/// default:
+///     ...
+/// ```
+fn compile_switch_naive(
+    ib: &mut InstructionBuilder<'_, '_>,
+    span: Span,
+    condition: Expr,
+    cases: Vec<SwitchCase>,
+    default: Option<Vec<Statement>>,
+) -> Result<(), Error> {
+    let condition_id = ib
+        .current_scope_mut()
+        .add_local(sym::switch_cond_desugar, VariableDeclarationKind::Unnameable, None)
+        .map_err(|_| Error::LocalLimitExceeded(span))?;
+
+    // Store condition in temporary
+    ib.accept_expr(condition)?;
+    ib.build_local_store(AssignKind::Assignment, condition_id, false);
+    ib.build_pop();
+
+    let local_load = compile_local_load(condition_id, false);
+
+    let case_count = cases.len().try_into().unwrap();
+
+    for (case_id, case) in cases.into_iter().enumerate() {
+        let case_id = u16::try_from(case_id).unwrap();
+        ib.add_local_label(Label::SwitchCaseCondition { case_id });
+
+        let eq = Expr::binary(
+            Expr {
+                kind: ExprKind::compiled(local_load.clone()),
+                span,
+            },
+            case.value,
+            TokenType::Equality,
+        );
+        ib.accept_expr(eq)?;
+        ib.build_jmpfalsep(Label::SwitchCaseCondition { case_id: case_id + 1 }, true);
+
+        ib.add_local_label(Label::SwitchCaseCode { case_id });
+        ib.accept_multiple(case.body)?;
+        ib.build_jmp(Label::SwitchCaseCode { case_id: case_id + 1 }, true);
+    }
+
+    ib.add_local_label(Label::SwitchCaseCondition { case_id: case_count });
+    ib.add_local_label(Label::SwitchCaseCode { case_id: case_count });
+    if let Some(default) = default {
+        ib.accept_multiple(default)?;
+    }
+
+    Ok(())
+}

crates/dash_decompiler/src/decompiler.rs

@@ -392,20 +392,6 @@ impl<'interner, 'buf> FunctionDecompiler<'interner, 'buf> {
                 Instruction::CallForInIterator => self.handle_opless_instr("@@forInIterator"),
                 Instruction::DeletePropertyStatic => self.handle_incw_op_instr("deletepropertystatic")?,
                 Instruction::DeletePropertyDynamic => self.handle_opless_instr("deletepropertydynamic"),
-                Instruction::Switch => {
-                    let case_count = self.read_u16()?;
-                    let has_default = self.read()? == 1;
-
-                    for _ in 0..case_count {
-                        self.read_u16()?; // discard case offsets for now..
-                    }
-
-                    if has_default {
-                        self.read_u16()?;
-                    }
-
-                    self.handle_op_map_instr("switch", &[("case_count", &case_count), ("has_default", &has_default)])
-                }
                 Instruction::ObjDestruct => {
                     let count = self.read_u16()?;
                     for _ in 0..count {

crates/dash_middle/src/compiler/constant.rs

@@ -106,6 +106,8 @@ pub enum Constant {
     Function(Rc<Function>),
     // Boxed because this otherwise bloats the enum way too much.
     // This makes evaluating regex constants slower but they're *far* less common than e.g. number literals
+    // TODO: avoid cloning `Constant`s when turning them into Values,
+    // because there's no point in cloning this box
     Regex(Box<(dash_regex::ParsedRegex, dash_regex::Flags, Symbol)>),
     Null,
     Undefined,

crates/dash_middle/src/compiler/instruction.rs

@@ -91,7 +91,6 @@ pub enum Instruction {
     CallForInIterator,
     DeletePropertyStatic,
     DeletePropertyDynamic,
-    Switch,
     ObjDestruct,
     ArrayDestruct,
     // Nop exists solely for the sake of benchmarking the raw throughput of the VM dispatch loop

crates/dash_middle/src/interner.rs

@@ -69,6 +69,7 @@ pub mod sym {
                 this,
                 for_of_iter,
                 for_of_gen_step,
+                switch_cond_desugar,
                 value,
                 done,
                 next,

crates/dash_vm/src/dispatch.rs

@@ -1557,43 +1557,6 @@ mod handlers {
         Ok(None)
     }
 
-    pub fn switch<'sc, 'vm>(mut cx: DispatchContext<'sc, 'vm>) -> Result<Option<HandleResult>, Unrooted> {
-        let case_count = cx.fetchw_and_inc_ip();
-        let has_default = cx.fetch_and_inc_ip() == 1;
-
-        let switch_expr = cx.pop_stack_rooted();
-
-        let mut target_ip = None;
-
-        for _ in 0..case_count {
-            let case_value = cx.pop_stack_rooted();
-            let case_offset = cx.fetchw_and_inc_ip() as usize;
-            let ip = cx.active_frame().ip;
-
-            let is_eq = switch_expr.strict_eq(&case_value, cx.scope)?.to_boolean(cx.scope)?;
-            let has_matching_case = target_ip.is_some();
-
-            if is_eq && !has_matching_case {
-                target_ip = Some(ip + case_offset);
-            }
-        }
-
-        if has_default {
-            let default_offset = cx.fetchw_and_inc_ip() as usize;
-            let ip = cx.active_frame().ip;
-
-            if target_ip.is_none() {
-                target_ip = Some(ip + default_offset);
-            }
-        }
-
-        if let Some(target_ip) = target_ip {
-            cx.active_frame_mut().ip = target_ip;
-        }
-
-        Ok(None)
-    }
-
     pub fn objdestruct<'sc, 'vm>(mut cx: DispatchContext<'sc, 'vm>) -> Result<Option<HandleResult>, Unrooted> {
         let count = cx.fetchw_and_inc_ip();
         let obj = cx.pop_stack_rooted();
@@ -1897,7 +1860,6 @@ pub fn handle(vm: &mut Vm, instruction: Instruction) -> Result<Option<HandleResu
         Instruction::CallForInIterator => handlers::call_for_in_iterator(cx),
         Instruction::DeletePropertyDynamic => handlers::delete_property_dynamic(cx),
         Instruction::DeletePropertyStatic => handlers::delete_property_static(cx),
-        Instruction::Switch => handlers::switch(cx),
         Instruction::ObjDestruct => handlers::objdestruct(cx),
         Instruction::ArrayDestruct => handlers::arraydestruct(cx),
         Instruction::Nop => Ok(None),