Allow duplicated switch cases.

This commit is contained in:
Stephen Chung 2022-07-18 13:40:41 +08:00
parent 4b760d1d0f
commit 7dca916c45
9 changed files with 235 additions and 145 deletions

View File

@ -14,10 +14,17 @@ New features
Enhancements
------------
### `switch` statement
* `switch` cases can now include multiple values separated by `|`.
* Duplicated `switch` cases are now allowed.
* The error `ParseErrorType::DuplicatedSwitchCase` is deprecated.
* Ranges in `switch` statements that are small (currently no more than 16 items) are unrolled if possible.
### Others
* `EvalContext::eval_expression_tree_raw` and `Expression::eval_with_context_raw` are added to allow for not rewinding the `Scope` at the end of a statements block.
* A new `range` function variant that takes an exclusive range with a step.
* Ranges in `switch` statements that are small (currently no more than 16 items) are unrolled if possible.
Version 1.8.0

View File

@ -22,8 +22,8 @@ pub use script_fn::EncapsulatedEnviron;
#[cfg(not(feature = "no_function"))]
pub use script_fn::{ScriptFnDef, ScriptFnMetadata};
pub use stmt::{
ConditionalStmtBlock, OpAssignment, RangeCase, Stmt, StmtBlock, StmtBlockContainer,
SwitchCases, TryCatchBlock,
CaseBlocksList, ConditionalStmtBlock, OpAssignment, RangeCase, Stmt, StmtBlock,
StmtBlockContainer, SwitchCasesCollection, TryCatchBlock,
};
#[cfg(not(feature = "no_float"))]

View File

@ -176,14 +176,14 @@ impl fmt::Debug for RangeCase {
impl From<Range<INT>> for RangeCase {
#[inline(always)]
fn from(value: Range<INT>) -> Self {
Self::ExclusiveInt(value, 0)
Self::ExclusiveInt(value, usize::MAX)
}
}
impl From<RangeInclusive<INT>> for RangeCase {
#[inline(always)]
fn from(value: RangeInclusive<INT>) -> Self {
Self::InclusiveInt(value, 0)
Self::InclusiveInt(value, usize::MAX)
}
}
@ -256,14 +256,16 @@ impl RangeCase {
}
}
pub type CaseBlocksList = smallvec::SmallVec<[usize; 1]>;
/// _(internals)_ A type containing all cases for a `switch` statement.
/// Exported under the `internals` feature only.
#[derive(Debug, Clone, Hash)]
pub struct SwitchCases {
pub struct SwitchCasesCollection {
/// List of [`ConditionalStmtBlock`]'s.
pub blocks: StaticVec<ConditionalStmtBlock>,
pub case_blocks: StaticVec<ConditionalStmtBlock>,
/// Dictionary mapping value hashes to [`ConditionalStmtBlock`]'s.
pub cases: BTreeMap<u64, usize>,
pub cases: BTreeMap<u64, CaseBlocksList>,
/// Statements block for the default case (there can be no condition for the default case).
pub def_case: usize,
/// List of range cases.
@ -512,7 +514,7 @@ pub enum Stmt {
/// 0) Hash table for (condition, block)
/// 1) Default block
/// 2) List of ranges: (start, end, inclusive, condition, statement)
Switch(Box<(Expr, SwitchCases)>, Position),
Switch(Box<(Expr, SwitchCasesCollection)>, Position),
/// `while` expr `{` stmt `}` | `loop` `{` stmt `}`
///
/// If the guard expression is [`UNIT`][Expr::Unit], then it is a `loop` statement.
@ -755,15 +757,18 @@ impl Stmt {
Self::Switch(x, ..) => {
let (expr, sw) = &**x;
expr.is_pure()
&& sw.cases.values().all(|&c| {
let block = &sw.blocks[c];
&& sw.cases.values().flat_map(|cases| cases.iter()).all(|&c| {
let block = &sw.case_blocks[c];
block.condition.is_pure() && block.statements.iter().all(Stmt::is_pure)
})
&& sw.ranges.iter().all(|r| {
let block = &sw.blocks[r.index()];
let block = &sw.case_blocks[r.index()];
block.condition.is_pure() && block.statements.iter().all(Stmt::is_pure)
})
&& sw.blocks[sw.def_case].statements.iter().all(Stmt::is_pure)
&& sw.case_blocks[sw.def_case]
.statements
.iter()
.all(Stmt::is_pure)
}
// Loops that exit can be pure because it can never be infinite.
@ -904,20 +909,22 @@ impl Stmt {
if !expr.walk(path, on_node) {
return false;
}
for (.., &b) in &sw.cases {
let block = &sw.blocks[b];
for (.., blocks) in &sw.cases {
for &b in blocks {
let block = &sw.case_blocks[b];
if !block.condition.walk(path, on_node) {
return false;
}
for s in &block.statements {
if !s.walk(path, on_node) {
if !block.condition.walk(path, on_node) {
return false;
}
for s in &block.statements {
if !s.walk(path, on_node) {
return false;
}
}
}
}
for r in &sw.ranges {
let block = &sw.blocks[r.index()];
let block = &sw.case_blocks[r.index()];
if !block.condition.walk(path, on_node) {
return false;
@ -928,7 +935,7 @@ impl Stmt {
}
}
}
for s in &sw.blocks[sw.def_case].statements {
for s in &sw.case_blocks[sw.def_case].statements {
if !s.walk(path, on_node) {
return false;
}

View File

@ -3,7 +3,7 @@
use super::{Caches, EvalContext, GlobalRuntimeState, Target};
use crate::api::events::VarDefInfo;
use crate::ast::{
ASTFlags, BinaryExpr, Expr, Ident, OpAssignment, Stmt, SwitchCases, TryCatchBlock,
ASTFlags, BinaryExpr, Expr, Ident, OpAssignment, Stmt, SwitchCasesCollection, TryCatchBlock,
};
use crate::func::get_hasher;
use crate::types::dynamic::{AccessMode, Union};
@ -393,8 +393,8 @@ impl Engine {
Stmt::Switch(x, ..) => {
let (
expr,
SwitchCases {
blocks,
SwitchCasesCollection {
case_blocks,
cases,
def_case,
ranges,
@ -411,32 +411,49 @@ impl Engine {
let hash = hasher.finish();
// First check hashes
if let Some(&case_block) = cases.get(&hash) {
let case_block = &blocks[case_block];
if let Some(case_blocks_list) = cases.get(&hash) {
assert!(!case_blocks_list.is_empty());
let cond_result = match case_block.condition {
Expr::BoolConstant(b, ..) => Ok(b),
ref c => self
.eval_expr(scope, global, caches, lib, this_ptr, c, level)
.and_then(|v| {
v.as_bool().map_err(|typ| {
self.make_type_mismatch_err::<bool>(typ, c.position())
})
}),
};
let mut result = Ok(None);
match cond_result {
Ok(true) => Ok(Some(&case_block.statements)),
Ok(false) => Ok(None),
_ => cond_result.map(|_| None),
for &index in case_blocks_list {
let block = &case_blocks[index];
let cond_result = match block.condition {
Expr::BoolConstant(b, ..) => Ok(b),
ref c => self
.eval_expr(scope, global, caches, lib, this_ptr, c, level)
.and_then(|v| {
v.as_bool().map_err(|typ| {
self.make_type_mismatch_err::<bool>(
typ,
c.position(),
)
})
}),
};
match cond_result {
Ok(true) => {
result = Ok(Some(&block.statements));
break;
}
Ok(false) => (),
_ => {
result = cond_result.map(|_| None);
break;
}
}
}
result
} else if value.is::<INT>() && !ranges.is_empty() {
// Then check integer ranges
let value = value.as_int().expect("`INT`");
let mut result = Ok(None);
for r in ranges.iter().filter(|r| r.contains(value)) {
let block = &blocks[r.index()];
let block = &case_blocks[r.index()];
let cond_result = match block.condition {
Expr::BoolConstant(b, ..) => Ok(b),
@ -481,7 +498,7 @@ impl Engine {
}
} else if let Ok(None) = stmt_block_result {
// Default match clause
let def_case = &blocks[*def_case].statements;
let def_case = &case_blocks[*def_case].statements;
if !def_case.is_empty() {
self.eval_stmt_block(

View File

@ -285,7 +285,7 @@ pub use parser::ParseState;
#[cfg(feature = "internals")]
pub use ast::{
ASTFlags, ASTNode, BinaryExpr, ConditionalStmtBlock, Expr, FnCallExpr, FnCallHashes, Ident,
OpAssignment, RangeCase, ScriptFnDef, Stmt, StmtBlock, SwitchCases, TryCatchBlock,
OpAssignment, RangeCase, ScriptFnDef, Stmt, StmtBlock, SwitchCasesCollection, TryCatchBlock,
};
#[cfg(feature = "internals")]

View File

@ -1,7 +1,9 @@
//! Module implementing the [`AST`] optimizer.
#![cfg(not(feature = "no_optimize"))]
use crate::ast::{ASTFlags, Expr, OpAssignment, Stmt, StmtBlock, StmtBlockContainer, SwitchCases};
use crate::ast::{
ASTFlags, Expr, OpAssignment, Stmt, StmtBlock, StmtBlockContainer, SwitchCasesCollection,
};
use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF};
use crate::eval::{Caches, GlobalRuntimeState};
use crate::func::builtin::get_builtin_binary_op_fn;
@ -524,8 +526,8 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
Stmt::Switch(x, pos) if x.0.is_constant() => {
let (
match_expr,
SwitchCases {
blocks,
SwitchCasesCollection {
case_blocks,
cases,
ranges,
def_case,
@ -538,39 +540,65 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
let hash = hasher.finish();
// First check hashes
if let Some(b) = cases.remove(&hash) {
let mut b = mem::take(&mut blocks[b]);
cases.clear();
if let Some(case_blocks_list) = cases.get(&hash) {
match &case_blocks_list[..] {
[] => (),
[index] => {
let mut b = mem::take(&mut case_blocks[*index]);
cases.clear();
match b.condition {
Expr::BoolConstant(true, ..) => {
// Promote the matched case
let statements: StmtBlockContainer = mem::take(&mut b.statements);
let statements = optimize_stmt_block(statements, state, true, true, false);
*stmt = (statements, b.statements.span()).into();
match b.condition {
Expr::BoolConstant(true, ..) => {
// Promote the matched case
let statements: StmtBlockContainer = mem::take(&mut b.statements);
let statements =
optimize_stmt_block(statements, state, true, true, false);
*stmt = (statements, b.statements.span()).into();
}
ref mut condition => {
// switch const { case if condition => stmt, _ => def } => if condition { stmt } else { def }
optimize_expr(condition, state, false);
let def_case = &mut case_blocks[*def_case].statements;
let def_span = def_case.span_or_else(*pos, Position::NONE);
let def_case: StmtBlockContainer = mem::take(def_case);
let def_stmt =
optimize_stmt_block(def_case, state, true, true, false);
*stmt = Stmt::If(
(
mem::take(condition),
mem::take(&mut b.statements),
StmtBlock::new_with_span(def_stmt, def_span),
)
.into(),
match_expr.start_position(),
);
}
}
state.set_dirty();
return;
}
ref mut condition => {
// switch const { case if condition => stmt, _ => def } => if condition { stmt } else { def }
optimize_expr(condition, state, false);
_ => {
for &index in case_blocks_list {
let mut b = mem::take(&mut case_blocks[index]);
let def_case = &mut blocks[*def_case].statements;
let def_span = def_case.span_or_else(*pos, Position::NONE);
let def_case: StmtBlockContainer = mem::take(def_case);
let def_stmt = optimize_stmt_block(def_case, state, true, true, false);
*stmt = Stmt::If(
(
mem::take(condition),
mem::take(&mut b.statements),
StmtBlock::new_with_span(def_stmt, def_span),
)
.into(),
match_expr.start_position(),
);
match b.condition {
Expr::BoolConstant(true, ..) => {
// Promote the matched case
let statements: StmtBlockContainer =
mem::take(&mut b.statements);
let statements =
optimize_stmt_block(statements, state, true, true, false);
*stmt = (statements, b.statements.span()).into();
state.set_dirty();
return;
}
_ => (),
}
}
}
}
state.set_dirty();
return;
}
// Then check ranges
@ -580,16 +608,19 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
// Only one range or all ranges without conditions
if ranges.len() == 1
|| ranges.iter().all(|r| {
matches!(blocks[r.index()].condition, Expr::BoolConstant(true, ..))
matches!(
case_blocks[r.index()].condition,
Expr::BoolConstant(true, ..)
)
})
{
for r in ranges.iter().filter(|r| r.contains(value)) {
let condition = mem::take(&mut blocks[r.index()].condition);
let condition = mem::take(&mut case_blocks[r.index()].condition);
match condition {
Expr::BoolConstant(true, ..) => {
// Promote the matched case
let block = &mut blocks[r.index()];
let block = &mut case_blocks[r.index()];
let statements = mem::take(&mut *block.statements);
let statements =
optimize_stmt_block(statements, state, true, true, false);
@ -599,13 +630,13 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
// switch const { range if condition => stmt, _ => def } => if condition { stmt } else { def }
optimize_expr(&mut condition, state, false);
let def_case = &mut blocks[*def_case].statements;
let def_case = &mut case_blocks[*def_case].statements;
let def_span = def_case.span_or_else(*pos, Position::NONE);
let def_case: StmtBlockContainer = mem::take(def_case);
let def_stmt =
optimize_stmt_block(def_case, state, true, true, false);
let statements = mem::take(&mut blocks[r.index()].statements);
let statements = mem::take(&mut case_blocks[r.index()].statements);
*stmt = Stmt::If(
(
@ -638,7 +669,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
}
for r in &*ranges {
let b = &mut blocks[r.index()];
let b = &mut case_blocks[r.index()];
let statements = mem::take(&mut *b.statements);
*b.statements =
optimize_stmt_block(statements, state, preserve_result, true, false);
@ -659,7 +690,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
// Promote the default case
state.set_dirty();
let def_case = &mut blocks[*def_case].statements;
let def_case = &mut case_blocks[*def_case].statements;
let def_span = def_case.span_or_else(*pos, Position::NONE);
let def_case: StmtBlockContainer = mem::take(def_case);
let def_stmt = optimize_stmt_block(def_case, state, true, true, false);
@ -669,8 +700,8 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
Stmt::Switch(x, ..) => {
let (
match_expr,
SwitchCases {
blocks,
SwitchCasesCollection {
case_blocks,
cases,
ranges,
def_case,
@ -681,7 +712,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
optimize_expr(match_expr, state, false);
// Optimize blocks
for b in blocks.iter_mut() {
for b in case_blocks.iter_mut() {
let statements = mem::take(&mut *b.statements);
*b.statements =
optimize_stmt_block(statements, state, preserve_result, true, false);
@ -704,15 +735,31 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
}
// Remove false cases
cases.retain(|_, &mut block| match blocks[block].condition {
Expr::BoolConstant(false, ..) => {
state.set_dirty();
false
let cases_len = cases.len();
cases.retain(|_, list| {
// Remove all entries that have false conditions
list.retain(|index| match case_blocks[*index].condition {
Expr::BoolConstant(false, ..) => false,
_ => true,
});
// Remove all entries after a `true` condition
if let Some(n) = list
.iter()
.find(|&&index| match case_blocks[index].condition {
Expr::BoolConstant(true, ..) => true,
_ => false,
})
{
list.truncate(n + 1);
}
_ => true,
// Remove if no entry left
!list.is_empty()
});
if cases.len() != cases_len {
state.set_dirty();
}
// Remove false ranges
ranges.retain(|r| match blocks[r.index()].condition {
ranges.retain(|r| match case_blocks[r.index()].condition {
Expr::BoolConstant(false, ..) => {
state.set_dirty();
false
@ -720,20 +767,20 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
_ => true,
});
let def_stmt_block = &mut blocks[*def_case].statements;
let def_stmt_block = &mut case_blocks[*def_case].statements;
let def_block = mem::take(&mut **def_stmt_block);
**def_stmt_block = optimize_stmt_block(def_block, state, preserve_result, true, false);
// Remove unused block statements
for index in 0..blocks.len() {
for index in 0..case_blocks.len() {
if *def_case == index
|| cases.values().any(|&n| n == index)
|| cases.values().flat_map(|c| c.iter()).any(|&n| n == index)
|| ranges.iter().any(|r| r.index() == index)
{
continue;
}
let b = &mut blocks[index];
let b = &mut case_blocks[index];
if !b.statements.is_empty() {
b.statements = StmtBlock::NONE;

View File

@ -3,8 +3,9 @@
use crate::api::events::VarDefInfo;
use crate::api::options::LangOptions;
use crate::ast::{
ASTFlags, BinaryExpr, ConditionalStmtBlock, Expr, FnCallExpr, FnCallHashes, Ident,
OpAssignment, RangeCase, ScriptFnDef, Stmt, StmtBlockContainer, SwitchCases, TryCatchBlock,
ASTFlags, BinaryExpr, CaseBlocksList, ConditionalStmtBlock, Expr, FnCallExpr, FnCallHashes,
Ident, OpAssignment, RangeCase, ScriptFnDef, Stmt, StmtBlockContainer, SwitchCasesCollection,
TryCatchBlock,
};
use crate::engine::{Precedence, KEYWORD_THIS, OP_CONTAINS};
use crate::eval::GlobalRuntimeState;
@ -1054,11 +1055,11 @@ impl Engine {
}
}
let mut blocks = StaticVec::<ConditionalStmtBlock>::new();
let mut cases = BTreeMap::<u64, usize>::new();
let mut case_blocks = StaticVec::<ConditionalStmtBlock>::new();
let mut cases = BTreeMap::<u64, CaseBlocksList>::new();
let mut ranges = StaticVec::<RangeCase>::new();
let mut def_pos = Position::NONE;
let mut def_stmt = None;
let mut def_stmt_pos = Position::NONE;
let mut def_stmt_index = None;
loop {
const MISSING_RBRACE: &str = "to end this switch block";
@ -1074,8 +1075,8 @@ impl Engine {
.into_err(*pos),
)
}
(Token::Underscore, pos) if def_stmt.is_none() => {
def_pos = *pos;
(Token::Underscore, pos) if def_stmt_index.is_none() => {
def_stmt_pos = *pos;
eat_token(input, Token::Underscore);
let (if_clause, if_pos) = match_token(input, Token::If);
@ -1086,10 +1087,8 @@ impl Engine {
(Default::default(), Expr::BoolConstant(true, Position::NONE))
}
(Token::Underscore, pos) => return Err(PERR::DuplicatedSwitchCase.into_err(*pos)),
_ if def_stmt.is_some() => {
return Err(PERR::WrongSwitchDefaultCase.into_err(def_pos))
_ if def_stmt_index.is_some() => {
return Err(PERR::WrongSwitchDefaultCase.into_err(def_stmt_pos))
}
_ => {
@ -1143,8 +1142,8 @@ impl Engine {
let need_comma = !stmt.is_self_terminated();
let has_condition = !matches!(condition, Expr::BoolConstant(true, ..));
blocks.push((condition, stmt).into());
let index = blocks.len() - 1;
case_blocks.push((condition, stmt).into());
let index = case_blocks.len() - 1;
if !case_expr_list.is_empty() {
for expr in case_expr_list {
@ -1170,7 +1169,10 @@ impl Engine {
for n in r {
let hasher = &mut get_hasher();
Dynamic::from_int(n).hash(hasher);
cases.entry(hasher.finish()).or_insert(index);
cases
.entry(hasher.finish())
.and_modify(|cases| cases.push(index))
.or_insert_with(|| [index].into());
}
} else {
// Other range
@ -1189,13 +1191,13 @@ impl Engine {
value.hash(hasher);
let hash = hasher.finish();
if cases.contains_key(&hash) {
return Err(PERR::DuplicatedSwitchCase.into_err(expr.start_position()));
}
cases.insert(hash, index);
cases
.entry(hash)
.and_modify(|cases| cases.push(index))
.or_insert_with(|| [index].into());
}
} else {
def_stmt = Some(index);
def_stmt_index = Some(index);
}
match input.peek().expect(NEVER_ENDS) {
@ -1221,13 +1223,13 @@ impl Engine {
}
}
let def_case = def_stmt.unwrap_or_else(|| {
blocks.push(Default::default());
blocks.len() - 1
let def_case = def_stmt_index.unwrap_or_else(|| {
case_blocks.push(Default::default());
case_blocks.len() - 1
});
let cases = SwitchCases {
blocks,
let cases = SwitchCasesCollection {
case_blocks,
cases,
def_case,
ranges,

View File

@ -97,6 +97,14 @@ pub enum ParseErrorType {
/// A map definition has duplicated property names. Wrapped value is the property name.
DuplicatedProperty(String),
/// A `switch` case is duplicated.
///
/// # Deprecated
///
/// This error variant is deprecated. It never occurs and will be removed in the next major version.
#[deprecated(
since = "1.9.0",
note = "This error variant is deprecated. It never occurs and will be removed in the next major version."
)]
DuplicatedSwitchCase,
/// A variable name is duplicated. Wrapped value is the variable name.
DuplicatedVariable(String),
@ -211,6 +219,7 @@ impl fmt::Display for ParseErrorType {
Self::FnDuplicatedParam(s, arg) => write!(f, "Duplicated parameter {} for function {}", arg, s),
Self::DuplicatedProperty(s) => write!(f, "Duplicated property for object map literal: {}", s),
#[allow(deprecated)]
Self::DuplicatedSwitchCase => f.write_str("Duplicated switch case"),
Self::DuplicatedVariable(s) => write!(f, "Duplicated variable name: {}", s),

View File

@ -32,7 +32,6 @@ fn test_switch() -> Result<(), Box<EvalAltResult>> {
)?,
'a'
);
assert_eq!(
engine.eval_with_scope::<bool>(&mut scope, "switch x { 1 => (), 2 => 'a', 42 => true }")?,
true
@ -98,6 +97,16 @@ fn test_switch() -> Result<(), Box<EvalAltResult>> {
3
);
assert_eq!(
engine.eval_with_scope::<INT>(&mut scope, "switch 42 { 42 => 123, 42 => 999 }")?,
123
);
assert_eq!(
engine.eval_with_scope::<INT>(&mut scope, "switch x { 42 => 123, 42 => 999 }")?,
123
);
Ok(())
}
@ -105,13 +114,6 @@ fn test_switch() -> Result<(), Box<EvalAltResult>> {
fn test_switch_errors() -> Result<(), Box<EvalAltResult>> {
let engine = Engine::new();
assert!(matches!(
*engine
.compile("switch x { 1 => 123, 1 => 42 }")
.expect_err("should error")
.0,
ParseErrorType::DuplicatedSwitchCase
));
assert!(matches!(
*engine
.compile("switch x { _ => 123, 1 => 42 }")
@ -159,23 +161,22 @@ fn test_switch_condition() -> Result<(), Box<EvalAltResult>> {
9
);
assert!(matches!(
*engine
.compile(
"
switch x {
21 if x < 40 => 1,
21 if x == 10 => 10,
0 if x < 100 => 2,
1 => 3,
_ => 9
}
"
)
.expect_err("should error")
.0,
ParseErrorType::DuplicatedSwitchCase
));
assert_eq!(
engine.eval_with_scope::<INT>(
&mut scope,
"
switch x {
42 if x < 40 => 1,
42 if x > 40 => 7,
0 if x < 100 => 2,
1 => 3,
42 if x == 10 => 10,
_ => 9
}
"
)?,
7
);
assert!(matches!(
*engine