From f1458e79e0db3f588fa844dc7bca47a45e6c09f5 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 31 Jan 2022 13:38:27 +0800 Subject: [PATCH] Improve AST debug display. --- src/ast/ast.rs | 109 +++++++++++++++++++++++++++++---------------- src/ast/expr.rs | 38 +++++++++------- src/lib.rs | 2 +- src/parser.rs | 44 ++++++++++-------- src/types/error.rs | 4 +- tests/optimizer.rs | 17 ++----- 6 files changed, 124 insertions(+), 90 deletions(-) diff --git a/src/ast/ast.rs b/src/ast/ast.rs index 948df827..18b02cff 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -1,10 +1,11 @@ //! Module defining the AST (abstract syntax tree). -use super::{Expr, FnAccess, Stmt, StmtBlock, AST_OPTION_FLAGS}; +use super::{Expr, FnAccess, Stmt, StmtBlock, AST_OPTION_FLAGS::*}; use crate::{Dynamic, FnNamespace, Identifier, Position, StaticVec}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{ + fmt, hash::Hash, ops::{Add, AddAssign}, }; @@ -14,7 +15,7 @@ use std::{ /// # Thread Safety /// /// Currently, [`AST`] is neither `Send` nor `Sync`. Turn on the `sync` feature to make it `Send + Sync`. -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct AST { /// Source of the [`AST`]. /// No source if string is empty. @@ -23,7 +24,7 @@ pub struct AST { body: StmtBlock, /// Script-defined functions. #[cfg(not(feature = "no_function"))] - functions: crate::Shared, + lib: crate::Shared, /// Embedded module resolver, if any. #[cfg(not(feature = "no_module"))] resolver: Option>, @@ -36,6 +37,31 @@ impl Default for AST { } } +impl fmt::Debug for AST { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut fp = f.debug_struct("AST"); + + if !self.source.is_empty() { + fp.field("source: ", &self.source); + } + #[cfg(not(feature = "no_module"))] + if let Some(ref resolver) = self.resolver { + fp.field("resolver: ", resolver); + } + + fp.field("body", &self.body.as_slice()); + + #[cfg(not(feature = "no_function"))] + if !self.lib.is_empty() { + for (_, _, _, _, ref fn_def) in self.lib.iter_script_fn() { + let sig = fn_def.to_string(); + fp.field(&sig, &fn_def.body.as_slice()); + } + } + fp.finish() + } +} + impl AST { /// Create a new [`AST`]. #[cfg(not(feature = "internals"))] @@ -49,7 +75,7 @@ impl AST { source: Identifier::new_const(), body: StmtBlock::new(statements, Position::NONE), #[cfg(not(feature = "no_function"))] - functions: functions.into(), + lib: functions.into(), #[cfg(not(feature = "no_module"))] resolver: None, } @@ -67,7 +93,7 @@ impl AST { source: Identifier::new_const(), body: StmtBlock::new(statements, Position::NONE), #[cfg(not(feature = "no_function"))] - functions: functions.into(), + lib: functions.into(), #[cfg(not(feature = "no_module"))] resolver: None, } @@ -115,7 +141,7 @@ impl AST { source: Identifier::new_const(), body: StmtBlock::NONE, #[cfg(not(feature = "no_function"))] - functions: crate::Module::new().into(), + lib: crate::Module::new().into(), #[cfg(not(feature = "no_module"))] resolver: None, } @@ -140,7 +166,7 @@ impl AST { pub fn set_source(&mut self, source: impl Into) -> &mut Self { let source = source.into(); #[cfg(not(feature = "no_function"))] - crate::Shared::get_mut(&mut self.functions) + crate::Shared::get_mut(&mut self.lib) .as_mut() .map(|m| m.set_id(source.clone())); self.source = source; @@ -181,7 +207,7 @@ impl AST { #[inline(always)] #[must_use] pub fn has_functions(&self) -> bool { - !self.functions.is_empty() + !self.lib.is_empty() } /// Get the internal shared [`Module`][crate::Module] containing all script-defined functions. #[cfg(not(feature = "internals"))] @@ -189,7 +215,7 @@ impl AST { #[inline(always)] #[must_use] pub(crate) fn shared_lib(&self) -> &crate::Shared { - &self.functions + &self.lib } /// _(internals)_ Get the internal shared [`Module`][crate::Module] containing all script-defined functions. /// Exported under the `internals` feature only. @@ -200,7 +226,7 @@ impl AST { #[inline(always)] #[must_use] pub fn shared_lib(&self) -> &crate::Shared { - &self.functions + &self.lib } /// Get the embedded [module resolver][`ModuleResolver`]. #[cfg(not(feature = "internals"))] @@ -260,12 +286,12 @@ impl AST { &self, filter: impl Fn(FnNamespace, FnAccess, bool, &str, usize) -> bool, ) -> Self { - let mut functions = crate::Module::new(); - functions.merge_filtered(&self.functions, &filter); + let mut lib = crate::Module::new(); + lib.merge_filtered(&self.lib, &filter); Self { source: self.source.clone(), body: StmtBlock::NONE, - functions: functions.into(), + lib: lib.into(), #[cfg(not(feature = "no_module"))] resolver: self.resolver.clone(), } @@ -279,7 +305,7 @@ impl AST { source: self.source.clone(), body: self.body.clone(), #[cfg(not(feature = "no_function"))] - functions: crate::Module::new().into(), + lib: crate::Module::new().into(), #[cfg(not(feature = "no_module"))] resolver: self.resolver.clone(), } @@ -472,24 +498,24 @@ impl AST { }; #[cfg(not(feature = "no_function"))] - let functions = { - let mut functions = self.functions.as_ref().clone(); - functions.merge_filtered(&other.functions, &_filter); - functions + let lib = { + let mut lib = self.lib.as_ref().clone(); + lib.merge_filtered(&other.lib, &_filter); + lib }; if !other.source.is_empty() { Self::new_with_source( merged, #[cfg(not(feature = "no_function"))] - functions, + lib, other.source.clone(), ) } else { Self::new( merged, #[cfg(not(feature = "no_function"))] - functions, + lib, ) } } @@ -562,9 +588,9 @@ impl AST { self.body.extend(other.body.into_iter()); #[cfg(not(feature = "no_function"))] - if !other.functions.is_empty() { - crate::func::native::shared_make_mut(&mut self.functions) - .merge_filtered(&other.functions, &_filter); + if !other.lib.is_empty() { + crate::func::native::shared_make_mut(&mut self.lib) + .merge_filtered(&other.lib, &_filter); } self } @@ -599,20 +625,32 @@ impl AST { &mut self, filter: impl Fn(FnNamespace, FnAccess, &str, usize) -> bool, ) -> &mut Self { - if !self.functions.is_empty() { - crate::func::native::shared_make_mut(&mut self.functions) - .retain_script_functions(filter); + if !self.lib.is_empty() { + crate::func::native::shared_make_mut(&mut self.lib).retain_script_functions(filter); } self } + /// _(internals)_ Iterate through all function definitions. + /// Exported under the `internals` feature only. + /// + /// Not available under `no_function`. + #[cfg(feature = "internals")] + #[cfg(not(feature = "no_function"))] + #[inline] + pub fn iter_fn_def(&self) -> impl Iterator { + self.lib + .iter_script_fn() + .map(|(_, _, _, _, fn_def)| fn_def.as_ref()) + } /// Iterate through all function definitions. /// /// Not available under `no_function`. + #[cfg(not(feature = "internals"))] #[cfg(not(feature = "no_function"))] #[allow(dead_code)] #[inline] pub(crate) fn iter_fn_def(&self) -> impl Iterator { - self.functions + self.lib .iter_script_fn() .map(|(_, _, _, _, fn_def)| fn_def.as_ref()) } @@ -622,7 +660,7 @@ impl AST { #[cfg(not(feature = "no_function"))] #[inline] pub fn iter_functions<'a>(&'a self) -> impl Iterator + 'a { - self.functions + self.lib .iter_script_fn() .map(|(_, _, _, _, fn_def)| fn_def.as_ref().into()) } @@ -632,7 +670,7 @@ impl AST { #[cfg(not(feature = "no_function"))] #[inline(always)] pub fn clear_functions(&mut self) -> &mut Self { - self.functions = crate::Module::new().into(); + self.lib = crate::Module::new().into(); self } /// Clear all statements in the [`AST`], leaving only function definitions. @@ -707,16 +745,11 @@ impl AST { ) -> impl Iterator { self.statements().iter().filter_map(move |stmt| match stmt { Stmt::Var(expr, name, options, _) - if options.contains(AST_OPTION_FLAGS::AST_OPTION_CONSTANT) && include_constants - || !options.contains(AST_OPTION_FLAGS::AST_OPTION_CONSTANT) - && include_variables => + if options.contains(AST_OPTION_CONSTANT) && include_constants + || !options.contains(AST_OPTION_CONSTANT) && include_variables => { if let Some(value) = expr.get_literal_value() { - Some(( - name.as_str(), - options.contains(AST_OPTION_FLAGS::AST_OPTION_CONSTANT), - value, - )) + Some((name.as_str(), options.contains(AST_OPTION_CONSTANT), value)) } else { None } @@ -871,6 +904,6 @@ impl AST { #[inline(always)] #[must_use] pub fn lib(&self) -> &crate::Module { - &self.functions + &self.lib } } diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 72dea97c..a9881804 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -165,7 +165,7 @@ impl FnCallHashes { /// _(internals)_ A function call. /// Exported under the `internals` feature only. -#[derive(Debug, Clone, Default, Hash)] +#[derive(Clone, Default, Hash)] pub struct FnCallExpr { /// Namespace of the function, if any. /// @@ -193,6 +193,24 @@ pub struct FnCallExpr { pub capture_parent_scope: bool, } +impl fmt::Debug for FnCallExpr { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut ff = f.debug_struct("FnCallExpr"); + #[cfg(not(feature = "no_module"))] + self.namespace.as_ref().map(|ns| ff.field("namespace", ns)); + ff.field("name", &self.name) + .field("hash", &self.hashes) + .field("arg_exprs", &self.args); + if !self.constants.is_empty() { + ff.field("constant_args", &self.constants); + } + if self.capture_parent_scope { + ff.field("capture_parent_scope", &self.capture_parent_scope); + } + ff.finish() + } +} + impl FnCallExpr { /// Does this function call contain a qualified namespace? /// @@ -459,26 +477,12 @@ impl fmt::Debug for Expr { f.write_str(")") } Self::Property(x, _) => write!(f, "Property({})", x.2), - Self::Stack(x, _) => write!(f, "StackSlot({})", x), + Self::Stack(x, _) => write!(f, "ConstantArg#{}", x), Self::Stmt(x) => { f.write_str("ExprStmtBlock")?; f.debug_list().entries(x.iter()).finish() } - Self::FnCall(x, _) => { - let mut ff = f.debug_struct("FnCall"); - #[cfg(not(feature = "no_module"))] - x.namespace.as_ref().map(|ns| ff.field("namespace", ns)); - ff.field("name", &x.name) - .field("hash", &x.hashes) - .field("args", &x.args); - if !x.constants.is_empty() { - ff.field("constants", &x.constants); - } - if x.capture_parent_scope { - ff.field("capture_parent_scope", &x.capture_parent_scope); - } - ff.finish() - } + Self::FnCall(x, _) => fmt::Debug::fmt(x, f), Self::Index(x, term, pos) => { display_pos = *pos; diff --git a/src/lib.rs b/src/lib.rs index e7d347d0..6d19cde0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -256,7 +256,7 @@ pub use parser::ParseState; pub use ast::{ ASTNode, BinaryExpr, ConditionalStmtBlock, CustomExpr, Expr, FnCallExpr, FnCallHashes, Ident, OpAssignment, OptionFlags, ScriptFnDef, Stmt, StmtBlock, SwitchCases, TryCatchBlock, - AST_OPTION_FLAGS::*, + AST_OPTION_FLAGS, }; #[cfg(feature = "internals")] diff --git a/src/parser.rs b/src/parser.rs index a24af289..2efdccee 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -52,11 +52,11 @@ pub struct ParseState<'e> { pub entry_stack_len: usize, /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_closure"))] - pub external_vars: BTreeMap, + pub external_vars: Vec, /// An indicator that disables variable capturing into externals one single time /// up until the nearest consumed Identifier token. /// If set to false the next call to [`access_var`][ParseState::access_var] will not capture the variable. - /// All consequent calls to [`access_var`][ParseState::access_var] will not be affected + /// All consequent calls to [`access_var`][ParseState::access_var] will not be affected. #[cfg(not(feature = "no_closure"))] pub allow_capture: bool, /// Encapsulates a local stack with imported [module][crate::Module] names. @@ -85,7 +85,7 @@ impl<'e> ParseState<'e> { #[cfg(not(feature = "no_function"))] max_function_expr_depth: NonZeroUsize::new(engine.max_function_expr_depth()), #[cfg(not(feature = "no_closure"))] - external_vars: BTreeMap::new(), + external_vars: Vec::new(), #[cfg(not(feature = "no_closure"))] allow_capture: true, interned_strings: StringsInterner::new(), @@ -128,8 +128,11 @@ impl<'e> ParseState<'e> { #[cfg(not(feature = "no_closure"))] if self.allow_capture { - if index.is_none() && !self.external_vars.contains_key(name) { - self.external_vars.insert(name.into(), _pos); + if index.is_none() && !self.external_vars.iter().any(|v| v.name == name) { + self.external_vars.push(crate::ast::Ident { + name: name.into(), + pos: _pos, + }); } } else { self.allow_capture = true @@ -1258,12 +1261,14 @@ fn parse_primary( #[cfg(not(feature = "no_closure"))] new_state.external_vars.iter().try_for_each( - |(captured_var, &pos)| -> ParseResult<_> { - let index = state.access_var(captured_var, pos); + |crate::ast::Ident { name, pos }| -> ParseResult<_> { + let index = state.access_var(name, *pos); - if !settings.is_closure && settings.strict_var && index.is_none() { + if settings.strict_var && !settings.is_closure && index.is_none() { // If the parent scope is not inside another capturing closure - Err(PERR::VariableUndefined(captured_var.to_string()).into_err(pos)) + // then we can conclude that the captured variable doesn't exist. + // Under Strict Variables mode, this is not allowed. + Err(PERR::VariableUndefined(name.to_string()).into_err(*pos)) } else { Ok(()) } @@ -3311,20 +3316,21 @@ fn parse_anon_fn( // External variables may need to be processed in a consistent order, // so extract them into a list. #[cfg(not(feature = "no_closure"))] - let externals: StaticVec = state - .external_vars - .iter() - .map(|(name, _)| name.clone()) - .collect(); + let (mut params, externals) = { + let externals: StaticVec = state + .external_vars + .iter() + .map(|crate::ast::Ident { name, .. }| name.clone()) + .collect(); - #[cfg(not(feature = "no_closure"))] - let mut params = StaticVec::with_capacity(params_list.len() + externals.len()); + let mut params = StaticVec::with_capacity(params_list.len() + externals.len()); + params.extend(externals.iter().cloned()); + + (params, externals) + }; #[cfg(feature = "no_closure")] let mut params = StaticVec::with_capacity(params_list.len()); - #[cfg(not(feature = "no_closure"))] - params.extend(externals.iter().cloned()); - params.append(&mut params_list); // Create unique function name by hashing the script body plus the parameters. diff --git a/src/types/error.rs b/src/types/error.rs index da507a9b..8131b7b0 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -149,7 +149,7 @@ impl fmt::Display for EvalAltResult { "" => f.write_str("Malformed dot expression"), s => f.write_str(s), }?, - Self::ErrorIndexingType(s, _) => write!(f, "Indexer not registered for {}", s)?, + Self::ErrorIndexingType(s, _) => write!(f, "Indexer not registered: {}", s)?, Self::ErrorUnboundThis(_) => f.write_str("'this' is not bound")?, Self::ErrorFor(_) => f.write_str("For loop expects a type that is iterable")?, Self::ErrorTooManyOperations(_) => f.write_str("Too many operations")?, @@ -166,7 +166,7 @@ impl fmt::Display for EvalAltResult { } Self::ErrorRuntime(d, _) => write!(f, "Runtime error: {}", d)?, - Self::ErrorAssignmentToConstant(s, _) => write!(f, "Cannot modify constant {}", s)?, + Self::ErrorAssignmentToConstant(s, _) => write!(f, "Cannot modify constant: {}", s)?, Self::ErrorMismatchOutputType(s, r, _) => match (r.as_str(), s.as_str()) { ("", s) => write!(f, "Output type is incorrect, expecting {}", s), (r, "") => write!(f, "Output type is incorrect: {}", r), diff --git a/tests/optimizer.rs b/tests/optimizer.rs index b6d6a289..af9754f4 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -78,33 +78,24 @@ fn test_optimizer_parse() -> Result<(), Box> { let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } else { 123 } }")?; - assert_eq!( - format!("{:?}", ast), - "AST { source: \"\", body: Block[Expr(123 @ 1:53)], functions: Module, resolver: None }" - ); + assert_eq!(format!("{:?}", ast), "AST { body: [Expr(123 @ 1:53)] }"); let ast = engine.compile("const DECISION = false; if DECISION { 42 } else { 123 }")?; assert_eq!( format!("{:?}", ast), - r#"AST { source: "", body: Block[Var(false @ 1:18, "DECISION" @ 1:7, (Constant), 1:1), Expr(123 @ 1:51)], functions: Module, resolver: None }"# + r#"AST { body: [Var(false @ 1:18, "DECISION" @ 1:7, (Constant), 1:1), Expr(123 @ 1:51)] }"# ); let ast = engine.compile("if 1 == 2 { 42 }")?; - assert_eq!( - format!("{:?}", ast), - "AST { source: \"\", body: Block[], functions: Module, resolver: None }" - ); + assert_eq!(format!("{:?}", ast), "AST { body: [] }"); engine.set_optimization_level(OptimizationLevel::Full); let ast = engine.compile("abs(-42)")?; - assert_eq!( - format!("{:?}", ast), - "AST { source: \"\", body: Block[Expr(42 @ 1:1)], functions: Module, resolver: None }" - ); + assert_eq!(format!("{:?}", ast), "AST { body: [Expr(42 @ 1:1)] }"); Ok(()) }