Fix bug in index expressions.

This commit is contained in:
Stephen Chung 2020-05-30 10:27:48 +08:00
parent e1242df5c8
commit 2bcc51cc45
5 changed files with 98 additions and 107 deletions

View File

@ -1051,8 +1051,7 @@ impl Engine {
let mut state = State::new();
let args = args.as_mut();
let result =
self.call_script_fn(Some(scope), &mut state, &lib, name, fn_def, args, pos, 0)?;
let result = self.call_script_fn(scope, &mut state, &lib, name, fn_def, args, pos, 0)?;
let return_type = self.map_type_name(result.type_name());

View File

@ -432,11 +432,11 @@ fn default_print(s: &str) {
}
/// Search for a variable within the scope
fn search_scope<'a>(
scope: &'a mut Scope,
fn search_scope<'s, 'a>(
scope: &'s mut Scope,
state: &mut State,
expr: &'a Expr,
) -> Result<(&'a mut Dynamic, &'a str, ScopeEntryType, Position), Box<EvalAltResult>> {
) -> Result<(&'s mut Dynamic, &'a str, ScopeEntryType, Position), Box<EvalAltResult>> {
let ((name, pos), modules, hash_var, index) = match expr {
Expr::Variable(x) => x.as_ref(),
_ => unreachable!(),
@ -592,7 +592,7 @@ impl Engine {
/// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`!
pub(crate) fn call_fn_raw(
&self,
scope: Option<&mut Scope>,
scope: &mut Scope,
state: &mut State,
lib: &FunctionsLib,
fn_name: &str,
@ -774,7 +774,7 @@ impl Engine {
/// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`!
pub(crate) fn call_script_fn(
&self,
scope: Option<&mut Scope>,
scope: &mut Scope,
state: &mut State,
lib: &FunctionsLib,
fn_name: &str,
@ -786,88 +786,42 @@ impl Engine {
let orig_scope_level = state.scope_level;
state.scope_level += 1;
match scope {
// Extern scope passed in which is not empty
Some(scope) if !scope.is_empty() => {
let scope_len = scope.len();
let scope_len = scope.len();
// Put arguments into scope as variables
// Actually consume the arguments instead of cloning them
scope.extend(
fn_def
.params
.iter()
.zip(args.iter_mut().map(|v| mem::take(*v)))
.map(|(name, value)| {
let var_name = unsafe_cast_var_name_to_lifetime(name.as_str(), state);
(var_name, ScopeEntryType::Normal, value)
}),
);
// Put arguments into scope as variables
// Actually consume the arguments instead of cloning them
scope.extend(
fn_def
.params
.iter()
.zip(args.iter_mut().map(|v| mem::take(*v)))
.map(|(name, value)| {
let var_name = unsafe_cast_var_name_to_lifetime(name.as_str(), state);
(var_name, ScopeEntryType::Normal, value)
}),
);
// Evaluate the function at one higher level of call depth
let result = self
.eval_stmt(scope, state, lib, &fn_def.body, level + 1)
.or_else(|err| match *err {
// Convert return statement to return value
EvalAltResult::Return(x, _) => Ok(x),
EvalAltResult::ErrorInFunctionCall(name, err, _) => {
Err(Box::new(EvalAltResult::ErrorInFunctionCall(
format!("{} > {}", fn_name, name),
err,
pos,
)))
}
_ => Err(Box::new(EvalAltResult::ErrorInFunctionCall(
fn_name.to_string(),
err,
pos,
))),
});
// Evaluate the function at one higher level of call depth
let result = self
.eval_stmt(scope, state, lib, &fn_def.body, level + 1)
.or_else(|err| match *err {
// Convert return statement to return value
EvalAltResult::Return(x, _) => Ok(x),
EvalAltResult::ErrorInFunctionCall(name, err, _) => Err(Box::new(
EvalAltResult::ErrorInFunctionCall(format!("{} > {}", fn_name, name), err, pos),
)),
_ => Err(Box::new(EvalAltResult::ErrorInFunctionCall(
fn_name.to_string(),
err,
pos,
))),
});
// Remove all local variables
scope.rewind(scope_len);
state.scope_level = orig_scope_level;
// Remove all local variables
scope.rewind(scope_len);
state.scope_level = orig_scope_level;
return result;
}
// No new scope - create internal scope
_ => {
let mut scope = Scope::new();
// Put arguments into scope as variables
// Actually consume the arguments instead of cloning them
scope.extend(
fn_def
.params
.iter()
.zip(args.iter_mut().map(|v| mem::take(*v)))
.map(|(name, value)| (name, ScopeEntryType::Normal, value)),
);
// Evaluate the function at one higher level of call depth
let result = self
.eval_stmt(&mut scope, state, lib, &fn_def.body, level + 1)
.or_else(|err| match *err {
// Convert return statement to return value
EvalAltResult::Return(x, _) => Ok(x),
EvalAltResult::ErrorInFunctionCall(name, err, _) => {
Err(Box::new(EvalAltResult::ErrorInFunctionCall(
format!("{} > {}", fn_name, name),
err,
pos,
)))
}
_ => Err(Box::new(EvalAltResult::ErrorInFunctionCall(
fn_name.to_string(),
err,
pos,
))),
});
state.scope_level = orig_scope_level;
return result;
}
}
result
}
// Has a system function an override?
@ -925,9 +879,12 @@ impl Engine {
}
// Normal function call
_ => self.call_fn_raw(
None, state, lib, fn_name, hashes, args, is_ref, def_val, pos, level,
),
_ => {
let mut scope = Scope::new();
self.call_fn_raw(
&mut scope, state, lib, fn_name, hashes, args, is_ref, def_val, pos, level,
)
}
}
}
@ -1252,14 +1209,16 @@ impl Engine {
Expr::FnCall(_) => unreachable!(),
Expr::Property(_) => idx_values.push(()), // Store a placeholder - no need to copy the property name
Expr::Index(x) | Expr::Dot(x) => {
let (lhs, rhs, _) = x.as_ref();
// Evaluate in left-to-right order
let lhs_val = match x.0 {
let lhs_val = match lhs {
Expr::Property(_) => Default::default(), // Store a placeholder in case of a property
_ => self.eval_expr(scope, state, lib, &x.0, level)?,
_ => self.eval_expr(scope, state, lib, lhs, level)?,
};
// Push in reverse order
self.eval_indexed_chain(scope, state, lib, &x.1, idx_values, size, level)?;
self.eval_indexed_chain(scope, state, lib, rhs, idx_values, size, level)?;
idx_values.push(lhs_val);
}
@ -1380,6 +1339,7 @@ impl Engine {
Dynamic(Union::Array(mut rhs_value)) => {
let op = "==";
let def_value = false.into();
let mut scope = Scope::new();
// Call the `==` operator to compare each value
for value in rhs_value.iter_mut() {
@ -1394,7 +1354,7 @@ impl Engine {
);
let (r, _) = self.call_fn_raw(
None, state, lib, op, hashes, args, false, def_value, pos, level,
&mut scope, state, lib, op, hashes, args, false, def_value, pos, level,
)?;
if r.as_bool().unwrap_or(false) {
return Ok(true.into());
@ -1432,6 +1392,8 @@ impl Engine {
self.inc_operations(state, expr.position())?;
match expr {
Expr::Expr(x) => self.eval_expr(scope, state, lib, x.as_ref(), level),
Expr::IntegerConstant(x) => Ok(x.0.into()),
#[cfg(not(feature = "no_float"))]
Expr::FloatConstant(x) => Ok(x.0.into()),
@ -1710,9 +1672,8 @@ impl Engine {
Ok(x) if x.is_script() => {
let args = args.as_mut();
let fn_def = x.get_fn_def();
let result =
self.call_script_fn(None, state, lib, name, fn_def, args, *pos, level)?;
Ok(result)
let mut scope = Scope::new();
self.call_script_fn(&mut scope, state, lib, name, fn_def, args, *pos, level)
}
Ok(x) => x.get_native_fn()(args.as_mut()).map_err(|err| err.new_position(*pos)),
Err(err)
@ -1772,9 +1733,9 @@ impl Engine {
}
/// Evaluate a statement
pub(crate) fn eval_stmt<'s>(
pub(crate) fn eval_stmt(
&self,
scope: &mut Scope<'s>,
scope: &mut Scope,
state: &mut State,
lib: &FunctionsLib,
stmt: &Stmt,

View File

@ -122,7 +122,7 @@ fn call_fn_with_constant_arguments(
state
.engine
.call_fn_raw(
None,
&mut Scope::new(),
&mut Default::default(),
state.lib,
fn_name,
@ -138,7 +138,7 @@ fn call_fn_with_constant_arguments(
}
/// Optimize a statement.
fn optimize_stmt<'a>(stmt: Stmt, state: &mut State<'a>, preserve_result: bool) -> Stmt {
fn optimize_stmt(stmt: Stmt, state: &mut State, preserve_result: bool) -> Stmt {
match stmt {
// if expr { Noop }
Stmt::IfThenElse(x) if matches!(x.1, Stmt::Noop(_)) => {
@ -359,11 +359,13 @@ fn optimize_stmt<'a>(stmt: Stmt, state: &mut State<'a>, preserve_result: bool) -
}
/// Optimize an expression.
fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr {
fn optimize_expr(expr: Expr, state: &mut State) -> Expr {
// These keywords are handled specially
const DONT_EVAL_KEYWORDS: [&str; 3] = [KEYWORD_PRINT, KEYWORD_DEBUG, KEYWORD_EVAL];
match expr {
// expr - do not promote because there is a reason it is wrapped in an `Expr::Expr`
Expr::Expr(x) => Expr::Expr(Box::new(optimize_expr(*x, state))),
// ( stmt )
Expr::Stmt(x) => match optimize_stmt(x.0, state, true) {
// ( Noop ) -> ()

View File

@ -23,6 +23,7 @@ use crate::stdlib::{
collections::HashMap,
format,
iter::{empty, Peekable},
mem,
num::NonZeroUsize,
ops::{Add, Deref, DerefMut},
string::{String, ToString},
@ -391,6 +392,8 @@ pub enum Expr {
Property(Box<((String, String, String), Position)>),
/// { stmt }
Stmt(Box<(Stmt, Position)>),
/// Wrapped expression - should not be optimized away.
Expr(Box<Expr>),
/// func(expr, ... ) - ((function name, native_only, position), optional modules, hash, arguments, optional default value)
/// Use `Cow<'static, str>` because a lot of operators (e.g. `==`, `>=`) are implemented as function calls
/// and the function names are predictable, so no need to allocate a new `String`.
@ -441,6 +444,8 @@ impl Expr {
/// Panics when the expression is not constant.
pub fn get_constant_value(&self) -> Dynamic {
match self {
Self::Expr(x) => x.get_constant_value(),
Self::IntegerConstant(x) => x.0.into(),
#[cfg(not(feature = "no_float"))]
Self::FloatConstant(x) => x.0.into(),
@ -475,6 +480,8 @@ impl Expr {
/// Panics when the expression is not constant.
pub fn get_constant_str(&self) -> String {
match self {
Self::Expr(x) => x.get_constant_str(),
#[cfg(not(feature = "no_float"))]
Self::FloatConstant(x) => x.0.to_string(),
@ -494,6 +501,8 @@ impl Expr {
/// Get the `Position` of the expression.
pub fn position(&self) -> Position {
match self {
Self::Expr(x) => x.position(),
#[cfg(not(feature = "no_float"))]
Self::FloatConstant(x) => x.1,
@ -519,6 +528,11 @@ impl Expr {
/// Override the `Position` of the expression.
pub(crate) fn set_position(mut self, new_pos: Position) -> Self {
match &mut self {
Self::Expr(ref mut x) => {
let expr = mem::take(x);
*x = Box::new(expr.set_position(new_pos));
}
#[cfg(not(feature = "no_float"))]
Self::FloatConstant(x) => x.1 = new_pos,
@ -550,6 +564,8 @@ impl Expr {
/// A pure expression has no side effects.
pub fn is_pure(&self) -> bool {
match self {
Self::Expr(x) => x.is_pure(),
Self::Array(x) => x.0.iter().all(Self::is_pure),
Self::Index(x) | Self::And(x) | Self::Or(x) | Self::In(x) => {
@ -568,6 +584,8 @@ impl Expr {
/// Is the expression a constant?
pub fn is_constant(&self) -> bool {
match self {
Self::Expr(x) => x.is_constant(),
#[cfg(not(feature = "no_float"))]
Self::FloatConstant(_) => true,
@ -598,6 +616,8 @@ impl Expr {
/// Is a particular token allowed as a postfix operator to this expression?
pub fn is_valid_postfix(&self, token: &Token) -> bool {
match self {
Self::Expr(x) => x.is_valid_postfix(token),
#[cfg(not(feature = "no_float"))]
Self::FloatConstant(_) => false,
@ -996,7 +1016,7 @@ fn parse_index_chain<'a>(
(Token::LeftBracket, _) => {
let idx_pos = eat_token(input, Token::LeftBracket);
// Recursively parse the indexing chain, right-binding each
let idx = parse_index_chain(
let idx_expr = parse_index_chain(
input,
state,
idx_expr,
@ -1005,10 +1025,20 @@ fn parse_index_chain<'a>(
allow_stmt_expr,
)?;
// Indexing binds to right
Ok(Expr::Index(Box::new((lhs, idx, pos))))
Ok(Expr::Index(Box::new((lhs, idx_expr, pos))))
}
// Otherwise terminate the indexing chain
_ => Ok(Expr::Index(Box::new((lhs, idx_expr, pos)))),
_ => {
match idx_expr {
// Terminate with an `Expr::Expr` wrapper to prevent the last index expression
// inside brackets to be mis-parsed as another level of indexing, or a
// dot expression/function call to be mis-parsed as following the indexing chain.
Expr::Index(_) | Expr::Dot(_) | Expr::FnCall(_) => Ok(Expr::Index(
Box::new((lhs, Expr::Expr(Box::new(idx_expr)), pos)),
)),
_ => Ok(Expr::Index(Box::new((lhs, idx_expr, pos)))),
}
}
}
}
(Token::LexError(err), pos) => return Err(PERR::BadInput(err.to_string()).into_err(*pos)),

View File

@ -20,21 +20,20 @@ fn test_string() -> Result<(), Box<EvalAltResult>> {
assert!(engine.eval::<bool>(r#"let y = "hello, world!"; 'w' in y"#)?);
assert!(!engine.eval::<bool>(r#"let y = "hello, world!"; "hey" in y"#)?);
#[cfg(not(feature = "no_stdlib"))]
assert_eq!(engine.eval::<String>(r#""foo" + 123"#)?, "foo123");
#[cfg(not(feature = "no_stdlib"))]
#[cfg(not(feature = "no_object"))]
assert_eq!(engine.eval::<String>("(42).to_string()")?, "42");
#[cfg(not(feature = "no_object"))]
assert_eq!(engine.eval::<char>(r#"let y = "hello"; y[y.len-1]"#)?, 'o');
#[cfg(not(feature = "no_float"))]
#[cfg(not(feature = "no_stdlib"))]
assert_eq!(engine.eval::<String>(r#""foo" + 123.4556"#)?, "foo123.4556");
Ok(())
}
#[cfg(not(feature = "no_stdlib"))]
#[cfg(not(feature = "no_object"))]
#[test]
fn test_string_substring() -> Result<(), Box<EvalAltResult>> {