Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix switch default execution #2907

Merged
merged 3 commits into from
May 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 56 additions & 34 deletions boa_ast/src/statement/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}
}
Expand All @@ -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.
Expand All @@ -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
}
}

Expand All @@ -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
Expand All @@ -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(())
}

Expand All @@ -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(())
}
}
3 changes: 2 additions & 1 deletion boa_engine/src/bytecompiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ enum Literal {
}

#[must_use]
#[derive(Debug, Clone, Copy)]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) struct Label {
index: u32,
}
Expand Down Expand Up @@ -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]
Expand Down
27 changes: 21 additions & 6 deletions boa_engine/src/bytecompiler/statement/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/vm/opcode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
19 changes: 10 additions & 9 deletions boa_parser/src/parser/statement/switch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()?;
Expand Down Expand Up @@ -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),
Expand All @@ -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,
_ => {
Expand All @@ -216,6 +217,6 @@ where
}
}

Ok((cases.into_boxed_slice(), default))
Ok(cases.into_boxed_slice())
}
}
29 changes: 15 additions & 14 deletions boa_parser/src/parser/statement/switch/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
],
Expand Down