Fix switch default execution (#2907)

* Fix switch `default` execution

* Fix typo

* Update switch.rs

boa_ast/src/statement/switch.rs

@@ -22,23 +22,39 @@ use core::ops::ControlFlow;
 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 #[derive(Clone, Debug, PartialEq)]
 pub struct Case {
-    condition: Expression,
+    condition: Option<Expression>,
     body: StatementList,
 }
 
 impl Case {
-    /// Creates a `Case` AST node.
+    /// Creates a regular `Case` AST node.
     #[inline]
     #[must_use]
     pub const fn new(condition: Expression, body: StatementList) -> Self {
-        Self { condition, body }
+        Self {
+            condition: Some(condition),
+            body,
+        }
+    }
+
+    /// Creates a default `Case` AST node.
+    #[inline]
+    #[must_use]
+    pub const fn default(body: StatementList) -> Self {
+        Self {
+            condition: None,
+            body,
+        }
     }
 
     /// Gets the condition of the case.
+    ///
+    /// If it's a regular case returns [`Some`] with the [`Expression`],
+    /// otherwise [`None`] is returned if it's the default case.
     #[inline]
     #[must_use]
-    pub const fn condition(&self) -> &Expression {
-        &self.condition
+    pub const fn condition(&self) -> Option<&Expression> {
+        self.condition.as_ref()
     }
 
     /// Gets the statement listin the body of the case.
@@ -47,22 +63,34 @@ impl Case {
     pub const fn body(&self) -> &StatementList {
         &self.body
     }
+
+    /// Check if the case is the `default` case.
+    #[inline]
+    #[must_use]
+    pub const fn is_default(&self) -> bool {
+        self.condition.is_none()
+    }
 }
 
 impl VisitWith for Case {
     fn visit_with<'a, V>(&'a self, visitor: &mut V) -> ControlFlow<V::BreakTy>
     where
         V: Visitor<'a>,
     {
-        try_break!(visitor.visit_expression(&self.condition));
+        if let Some(condition) = &self.condition {
+            try_break!(visitor.visit_expression(condition));
+        }
+
         visitor.visit_statement_list(&self.body)
     }
 
     fn visit_with_mut<'a, V>(&'a mut self, visitor: &mut V) -> ControlFlow<V::BreakTy>
     where
         V: VisitorMut<'a>,
     {
-        try_break!(visitor.visit_expression_mut(&mut self.condition));
+        if let Some(condition) = &mut self.condition {
+            try_break!(visitor.visit_expression_mut(condition));
+        }
         visitor.visit_statement_list_mut(&mut self.body)
     }
 }
@@ -89,19 +117,14 @@ impl VisitWith for Case {
 pub struct Switch {
     val: Expression,
     cases: Box<[Case]>,
-    default: Option<StatementList>,
 }
 
 impl Switch {
     /// Creates a `Switch` AST node.
     #[inline]
     #[must_use]
-    pub fn new(val: Expression, cases: Box<[Case]>, default: Option<StatementList>) -> Self {
-        Self {
-            val,
-            cases,
-            default,
-        }
+    pub fn new(val: Expression, cases: Box<[Case]>) -> Self {
+        Self { val, cases }
     }
 
     /// Gets the value to switch.
@@ -121,8 +144,13 @@ impl Switch {
     /// Gets the default statement list, if any.
     #[inline]
     #[must_use]
-    pub const fn default(&self) -> Option<&StatementList> {
-        self.default.as_ref()
+    pub fn default(&self) -> Option<&StatementList> {
+        for case in self.cases.as_ref() {
+            if case.is_default() {
+                return Some(case.body());
+            }
+        }
+        None
     }
 }
 
@@ -131,20 +159,20 @@ impl ToIndentedString for Switch {
         let indent = "    ".repeat(indentation);
         let mut buf = format!("switch ({}) {{\n", self.val().to_interned_string(interner));
         for e in self.cases().iter() {
-            buf.push_str(&format!(
-                "{}    case {}:\n{}",
-                indent,
-                e.condition().to_interned_string(interner),
-                e.body().to_indented_string(interner, indentation + 2)
-            ));
+            if let Some(condition) = e.condition() {
+                buf.push_str(&format!(
+                    "{indent}    case {}:\n{}",
+                    condition.to_interned_string(interner),
+                    e.body().to_indented_string(interner, indentation + 2)
+                ));
+            } else {
+                buf.push_str(&format!(
+                    "{indent}    default:\n{}",
+                    e.body().to_indented_string(interner, indentation + 2)
+                ));
+            }
         }
 
-        if let Some(ref default) = self.default {
-            buf.push_str(&format!(
-                "{indent}    default:\n{}",
-                default.to_indented_string(interner, indentation + 2)
-            ));
-        }
         buf.push_str(&format!("{indent}}}"));
 
         buf
@@ -167,9 +195,6 @@ impl VisitWith for Switch {
         for case in self.cases.iter() {
             try_break!(visitor.visit_case(case));
         }
-        if let Some(sl) = &self.default {
-            try_break!(visitor.visit_statement_list(sl));
-        }
         ControlFlow::Continue(())
     }
 
@@ -181,9 +206,6 @@ impl VisitWith for Switch {
         for case in self.cases.iter_mut() {
             try_break!(visitor.visit_case_mut(case));
         }
-        if let Some(sl) = &mut self.default {
-            try_break!(visitor.visit_statement_list_mut(sl));
-        }
         ControlFlow::Continue(())
     }
 }

boa_engine/src/bytecompiler/mod.rs

@@ -174,7 +174,7 @@ enum Literal {
 }
 
 #[must_use]
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) struct Label {
     index: u32,
 }
@@ -295,6 +295,7 @@ pub struct ByteCompiler<'ctx, 'host> {
 impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
     /// Represents a placeholder address that will be patched later.
     const DUMMY_ADDRESS: u32 = u32::MAX;
+    const DUMMY_LABEL: Label = Label { index: u32::MAX };
 
     /// Creates a new [`ByteCompiler`].
     #[inline]

boa_engine/src/bytecompiler/statement/switch.rs

@@ -19,20 +19,35 @@ impl ByteCompiler<'_, '_> {
 
         let mut labels = Vec::with_capacity(switch.cases().len());
         for case in switch.cases() {
-            self.compile_expr(case.condition(), true);
-            labels.push(self.emit_opcode_with_operand(Opcode::Case));
+            // If it does not have a condition it is the default case.
+            let label = if let Some(condition) = case.condition() {
+                self.compile_expr(condition, true);
+
+                self.emit_opcode_with_operand(Opcode::Case)
+            } else {
+                Self::DUMMY_LABEL
+            };
+
+            labels.push(label);
         }
 
-        let exit = self.emit_opcode_with_operand(Opcode::Default);
+        let default_label = self.emit_opcode_with_operand(Opcode::Default);
+        let mut default_label_set = false;
 
         for (label, case) in labels.into_iter().zip(switch.cases()) {
+            // Check if it's the default case.
+            let label = if label == Self::DUMMY_LABEL {
+                default_label_set = true;
+                default_label
+            } else {
+                label
+            };
             self.patch_jump(label);
             self.compile_statement_list(case.body(), false);
         }
 
-        self.patch_jump(exit);
-        if let Some(body) = switch.default() {
-            self.compile_statement_list(body, false);
+        if !default_label_set {
+            self.patch_jump(default_label);
         }
 
         self.pop_switch_control_info();

boa_engine/src/vm/opcode/mod.rs

@@ -164,7 +164,7 @@ pub(crate) trait Operation {
 }
 
 generate_impl! {
-    #[derive(Debug, Clone, Copy, TryFromPrimitive)]
+    #[derive(Debug, Clone, Copy, PartialEq, Eq, TryFromPrimitive)]
     #[repr(u8)]
     pub enum Opcode {
         /// Pop the top value from the stack.

boa_parser/src/parser/statement/switch/mod.rs

@@ -72,11 +72,10 @@ where
 
         let position = cursor.peek(0, interner).or_abrupt()?.span().start();
 
-        let (cases, default) =
-            CaseBlock::new(self.allow_yield, self.allow_await, self.allow_return)
-                .parse(cursor, interner)?;
+        let cases = CaseBlock::new(self.allow_yield, self.allow_await, self.allow_return)
+            .parse(cursor, interner)?;
 
-        let switch = Switch::new(condition, cases, default);
+        let switch = Switch::new(condition, cases);
 
         // It is a Syntax Error if the LexicallyDeclaredNames of CaseBlock contains any duplicate
         // entries, unless the source text matched by this production is not strict mode code and the
@@ -144,13 +143,13 @@ impl<R> TokenParser<R> for CaseBlock
 where
     R: Read,
 {
-    type Output = (Box<[statement::Case]>, Option<ast::StatementList>);
+    type Output = Box<[statement::Case]>;
 
     fn parse(self, cursor: &mut Cursor<R>, interner: &mut Interner) -> ParseResult<Self::Output> {
         cursor.expect(Punctuator::OpenBlock, "switch case block", interner)?;
 
         let mut cases = Vec::new();
-        let mut default = None;
+        let mut has_default_case = false;
 
         loop {
             let token = cursor.next(interner).or_abrupt()?;
@@ -181,7 +180,7 @@ where
                     cases.push(statement::Case::new(cond, statement_list));
                 }
                 TokenKind::Keyword((Keyword::Default, false)) => {
-                    if default.is_some() {
+                    if has_default_case {
                         // If default has already been defined then it cannot be defined again and to do so is an error.
                         return Err(Error::unexpected(
                             token.to_string(interner),
@@ -202,7 +201,9 @@ where
                     )
                     .parse(cursor, interner)?;
 
-                    default = Some(statement_list);
+                    cases.push(statement::Case::default(statement_list));
+
+                    has_default_case = true;
                 }
                 TokenKind::Punctuator(Punctuator::CloseBlock) => break,
                 _ => {
@@ -216,6 +217,6 @@ where
             }
         }
 
-        Ok((cases.into_boxed_slice(), default))
+        Ok(cases.into_boxed_slice())
     }
 }

boa_parser/src/parser/statement/switch/tests.rs

@@ -197,22 +197,23 @@ fn check_separated_switch() {
                         ]
                         .into(),
                     ),
-                ]
-                .into(),
-                Some(
-                    vec![Statement::Expression(Expression::from(Call::new(
-                        Expression::PropertyAccess(
-                            SimplePropertyAccess::new(Identifier::new(console).into(), log).into(),
-                        ),
-                        vec![Literal::from(
-                            interner.get_or_intern_static("Default", utf16!("Default")),
-                        )
+                    Case::default(
+                        vec![Statement::Expression(Expression::from(Call::new(
+                            Expression::PropertyAccess(
+                                SimplePropertyAccess::new(Identifier::new(console).into(), log)
+                                    .into(),
+                            ),
+                            vec![Literal::from(
+                                interner.get_or_intern_static("Default", utf16!("Default")),
+                            )
+                            .into()]
+                            .into(),
+                        )))
                         .into()]
                         .into(),
-                    )))
-                    .into()]
-                    .into(),
-                ),
+                    ),
+                ]
+                .into(),
             ))
             .into(),
         ],