From d2049d07f35b589b6e0944324618198f300dd333 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi <35780660+anakrish@users.noreply.github.com> Date: Sun, 7 Apr 2024 18:21:32 +0530 Subject: [PATCH] Store Value instances in AST for strings, numbers and idents (#197) This avoids having to create value instances during evaluation Signed-off-by: Anand Krishnamoorthi --- src/ast.rs | 14 ++++---- src/interpreter.rs | 86 ++++++++++++++++++--------------------------- src/parser.rs | 68 ++++++++++++++++++++++++++--------- src/scheduler.rs | 40 +++++++++++---------- src/utils.rs | 8 ++--- tests/parser/mod.rs | 10 +++--- 6 files changed, 125 insertions(+), 101 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 4b425ba..a2ae6c1 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -2,6 +2,7 @@ // Licensed under the MIT License. use crate::lexer::*; +use crate::value::Value; use crate::Rc; use std::ops::Deref; @@ -98,13 +99,13 @@ pub type Ref = NodeRef; #[derive(Debug)] pub enum Expr { // Simple items that only have a span as content. - String(Span), - RawString(Span), - Number(Span), + String((Span, Value)), + RawString((Span, Value)), + Number((Span, Value)), True(Span), False(Span), Null(Span), - Var(Span), + Var((Span, Value)), // array Array { @@ -158,7 +159,7 @@ pub enum Expr { RefDot { span: Span, refr: Ref, - field: Span, + field: (Span, Value), }, RefBrack { @@ -207,7 +208,8 @@ impl Expr { pub fn span(&self) -> &Span { use Expr::*; match self { - String(s) | RawString(s) | Number(s) | True(s) | False(s) | Null(s) | Var(s) => s, + String(s) | RawString(s) | Number(s) | Var(s) => &s.0, + True(s) | False(s) | Null(s) => s, Array { span, .. } | Set { span, .. } | Object { span, .. } diff --git a/src/interpreter.rs b/src/interpreter.rs index a662778..252946d 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -4,7 +4,6 @@ use crate::ast::*; use crate::builtins::{self, BuiltinFcn}; use crate::lexer::*; -use crate::number::*; use crate::parser::Parser; use crate::scheduler::*; use crate::utils::*; @@ -16,7 +15,6 @@ use anyhow::{anyhow, bail, Result}; use std::collections::btree_map::Entry as BTreeMapEntry; use std::collections::{hash_map::Entry, BTreeMap, BTreeSet, HashMap, HashSet}; use std::ops::Bound::*; -use std::str::FromStr; type Scope = BTreeMap; @@ -323,18 +321,18 @@ impl Interpreter { // Stop path collection upon encountering the leading variable. Expr::Var(v) => { path.reverse(); - return self.lookup_var(v, &path[..], false); + return self.lookup_var(&v.0, &path[..], false); } // Accumulate chained . field accesses. Expr::RefDot { refr, field, .. } => { expr = refr; - path.push(field.text()); + path.push(field.0.text()); } Expr::RefBrack { refr, index, .. } => match index.as_ref() { // refr["field"] is the same as refr.field Expr::String(s) => { expr = refr; - path.push(s.text()); + path.push(s.0.text()); } // Handle other forms of refr. // Note, we have the choice to evaluate a non-string index @@ -416,8 +414,8 @@ impl Interpreter { // Then hoist the current bracket operation. let mut indices = Vec::with_capacity(1); let _ = traverse(index, &mut |e| match e.as_ref() { - Var(ident) if self.is_loop_index_var(&ident.source_str()) => { - indices.push(ident.source_str()); + Var(ident) if self.is_loop_index_var(&ident.0.source_str()) => { + indices.push(ident.0.source_str()); Ok(false) } Array { .. } | Object { .. } => Ok(true), @@ -586,16 +584,16 @@ impl Interpreter { AssignOp::Eq => { match (lhs.as_ref(), rhs.as_ref()) { (_, Expr::Var(var)) - if var.source_str().text() != "input" - && self.lookup_var(var, &[], true)? == Value::Undefined => + if var.0.source_str().text() != "input" + && self.lookup_var(&var.0, &[], true)? == Value::Undefined => { - (var.source_str(), self.eval_expr(lhs)?) + (var.0.source_str(), self.eval_expr(lhs)?) } (Expr::Var(var), _) - if var.source_str().text() != "input" - && self.lookup_var(var, &[], true)? == Value::Undefined => + if var.0.source_str().text() != "input" + && self.lookup_var(&var.0, &[], true)? == Value::Undefined => { - (var.source_str(), self.eval_expr(rhs)?) + (var.0.source_str(), self.eval_expr(rhs)?) } ( Expr::Array { @@ -696,8 +694,8 @@ impl Interpreter { return Ok(rhs_value); } - let name = if let Expr::Var(span) = lhs.as_ref() { - span.source_str() + let name = if let Expr::Var(s) = lhs.as_ref() { + s.0.source_str() } else { let mut cache = BTreeMap::new(); let mut type_match = BTreeSet::new(); @@ -829,16 +827,16 @@ impl Interpreter { let raise_error = is_last && type_match.get(expr).is_none(); match (expr.as_ref(), value) { - (Expr::Var(ident), _) if ident.text() == "_" => Ok(true), + (Expr::Var(ident), _) if ident.0.text() == "_" => Ok(true), (Expr::Var(ident), _) if check_existing_value - && self.lookup_local_var(&ident.source_str()) == Some(value.clone()) => + && self.lookup_local_var(&ident.0.source_str()) == Some(value.clone()) => { Ok(false) } (Expr::Var(ident), _) => { - self.add_variable(&ident.source_str(), value.clone())?; + self.add_variable(&ident.0.source_str(), value.clone())?; Ok(true) } @@ -1389,7 +1387,7 @@ impl Interpreter { // then evaluate statements only if the index applies to this collection. let loop_expr_index = loop_expr.index(); if let Some(Expr::Var(index_var)) = loop_expr_index.as_ref().map(|r| r.as_ref()) { - if let Some(idx) = self.lookup_local_var(&index_var.source_str()) { + if let Some(idx) = self.lookup_local_var(&index_var.0.source_str()) { if loop_expr_value[&idx] != Value::Undefined { result = self.eval_stmts_in_loop(stmts, &loops[1..])? || result; return Ok(result); @@ -1502,7 +1500,7 @@ impl Interpreter { loop { match expr.as_ref() { Expr::Var(v) => { - comps.push(Value::String(v.text().into())); + comps.push(Value::String(v.0.text().into())); break; } Expr::RefBrack { refr, index, .. } => { @@ -1510,7 +1508,7 @@ impl Interpreter { expr = refr; } Expr::RefDot { refr, field, .. } => { - comps.push(Value::String(field.text().into())); + comps.push(Value::String(field.0.text().into())); expr = refr; } _ => { @@ -2392,12 +2390,12 @@ impl Interpreter { if let Some(ea) = extra_arg { match ea.as_ref() { Expr::Var(var) - if allow_return_arg && self.lookup_local_var(&var.source_str()).is_none() => + if allow_return_arg && self.lookup_local_var(&var.0.source_str()).is_none() => { let value = self.eval_call_impl(span, expr, fcn, ¶ms[..params.len() - 1])?; - if var.text() != "_" { - self.add_variable(&var.source_str(), value)?; + if var.0.text() != "_" { + self.add_variable(&var.0.source_str(), value)?; } Ok(Value::Bool(true)) } @@ -2664,24 +2662,10 @@ impl Interpreter { Expr::Null(_) => Ok(Value::Null), Expr::True(_) => Ok(Value::Bool(true)), Expr::False(_) => Ok(Value::Bool(false)), - Expr::Number(span) => { - let v = match Number::from_str(span.text()) { - Ok(v) => Ok(Value::Number(v)), - Err(_) => Err(span - .source - .error(span.line, span.col, "could not parse number")), - }; - v - } + Expr::Number((_, v)) => Ok(v.clone()), // TODO: Handle string vs rawstring - Expr::String(span) => { - match serde_json::from_str::(format!("\"{}\"", span.text()).as_str()) { - Ok(s) => Ok(s), - Err(e) => bail!(span.error(format!("invalid string literal. {e}").as_str())), - } - } - Expr::RawString(span) => Ok(Value::String(span.text().to_string().into())), - + Expr::String((_, v)) => Ok(v.clone()), + Expr::RawString((_, v)) => Ok(v.clone()), // TODO: Handle undefined variables Expr::Var(_) => self.eval_chained_ref_dot_or_brack(expr), Expr::RefDot { .. } => self.eval_chained_ref_dot_or_brack(expr), @@ -2909,17 +2893,17 @@ impl Interpreter { while let Some(e) = expr { match e { Expr::RefDot { refr, field, .. } => { - comps.push(field.text()); + comps.push(field.0.text()); expr = Some(refr); } Expr::RefBrack { refr, index, .. } if matches!(index.as_ref(), Expr::String(_)) => { if let Expr::String(s) = index.as_ref() { - comps.push(s.text()); + comps.push(s.0.text()); expr = Some(refr); } } Expr::Var(v) => { - comps.push(v.text()); + comps.push(v.0.text()); expr = None; } _ => bail!(e.span().error("invalid ref expression")), @@ -2985,7 +2969,7 @@ impl Interpreter { } // The following may evaluate to undefined. - Var(span) => ("var", span), + Var((span, _)) => ("var", span), Call { span, .. } => ("call", span), UnaryExpr { span, .. } => ("unaryexpr", span), RefDot { span, .. } => ("ref", span), @@ -3359,19 +3343,19 @@ impl Interpreter { loop { refr = match refr.as_ref() { Expr::Var(v) => { - components.push(v.text().into()); + components.push(v.0.text().into()); break; } Expr::RefBrack { refr, index, .. } => { if let Expr::String(s) = index.as_ref() { - components.push(s.text().into()); + components.push(s.0.text().into()); } else { components.clear(); } refr } Expr::RefDot { refr, field, .. } => { - components.push(field.text().into()); + components.push(field.0.text().into()); refr } _ => break, @@ -3492,12 +3476,12 @@ impl Interpreter { let target = match &import.r#as { Some(s) => s.text(), _ => match import.refr.as_ref() { - Expr::RefDot { field, .. } => field.text(), + Expr::RefDot { field, .. } => field.0.text(), Expr::RefBrack { index, .. } => match index.as_ref() { - Expr::String(s) => s.text(), + Expr::String(s) => s.0.text(), _ => "", }, - Expr::Var(v) if v.text() == "input" => { + Expr::Var(v) if v.0.text() == "input" => { // Warn redundant import of input. Ignore it. eprintln!( "{}", diff --git a/src/parser.rs b/src/parser.rs index 6a1c42f..58f60af 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3,7 +3,11 @@ use crate::ast::*; use crate::lexer::*; +use crate::number::*; +use crate::value::*; + use std::collections::BTreeMap; +use std::str::FromStr; use anyhow::{anyhow, bail, Result}; @@ -102,14 +106,14 @@ impl<'source> Parser<'source> { match refr.as_ref() { Expr::RefDot { refr, field, .. } => { Self::get_path_ref_components_into(refr, comps)?; - comps.push(field.clone()); + comps.push(field.0.clone()); } Expr::RefBrack { refr, index, .. } => { Self::get_path_ref_components_into(refr, comps)?; Self::get_path_ref_components_into(index, comps)?; } - Expr::Var(v) => comps.push(v.clone()), - Expr::String(s) => comps.push(s.clone()), + Expr::Var(v) => comps.push(v.0.clone()), + Expr::String(s) => comps.push(s.0.clone()), _ => bail!("internal error: not a simple ref"), } Ok(()) @@ -231,17 +235,38 @@ impl<'source> Parser<'source> { } } + fn read_number(span: Span) -> Result { + match Number::from_str(span.text()) { + Ok(v) => Ok(Expr::Number((span, Value::Number(v)))), + Err(_) => bail!(span.error("could not parse number")), + } + } + fn parse_scalar_or_var(&mut self) -> Result { let span = self.tok.1.clone(); let node = match &self.tok.0 { - TokenKind::Number => Expr::Number(span), - TokenKind::String => Expr::String(span), - TokenKind::RawString => Expr::RawString(span), + TokenKind::Number => Self::read_number(span)?, + TokenKind::String => { + let v = match serde_json::from_str::(format!("\"{}\"", span.text()).as_str()) + { + Ok(v) => v, + Err(e) => bail!(span.error(format!("invalid string literal. {e}").as_str())), + }; + Expr::String((span, v)) + } + TokenKind::RawString => { + let v = Value::from(span.text().to_string()); + Expr::RawString((span, v)) + } TokenKind::Ident => match self.token_text() { "null" => Expr::Null(span), "true" => Expr::True(span), "false" => Expr::False(span), - _ => return Ok(Expr::Var(self.parse_var()?)), + _ => { + let ident = self.parse_var()?; + let v = Value::from(ident.text()); + return Ok(Expr::Var((ident, v))); + } }, _ => { return Err(self.source.error( @@ -529,10 +554,11 @@ impl<'source> Parser<'source> { ) ); } + let fieldv = Value::from(field.text()); term = Expr::RefDot { span, refr: Ref::new(term), - field, + field: (field, fieldv), }; } "[" => { @@ -632,7 +658,7 @@ impl<'source> Parser<'source> { rhs_span.col += 1; self.next_token()?; - Expr::Number(rhs_span) + Self::read_number(rhs_span)? } else { self.next_token()?; self.parse_mul_div_mod_expr()? @@ -786,10 +812,10 @@ impl<'source> Parser<'source> { "=" => AssignOp::Eq, ":=" if self.rego_v1 => { if let Expr::Var(v) = &expr { - if v.text() == "input" { + if v.0.text() == "input" { bail!(span.error("input cannot be shadowed")); } - if v.text() == "data" { + if v.0.text() == "data" { bail!(span.error("data cannot be shadowed")); } } @@ -1055,11 +1081,16 @@ impl<'source> Parser<'source> { })) } + fn span_and_value(s: Span) -> (Span, Value) { + let v = Value::from(s.text()); + (s, v) + } + fn parse_path_ref(&mut self) -> Result { let start = self.tok.1.start; let var = self.parse_var()?; - let mut refr = Expr::Var(var); + let mut refr = Expr::Var(Self::span_and_value(var)); loop { let mut span = self.tok.1.clone(); let sep_pos = span.start; @@ -1095,13 +1126,13 @@ impl<'source> Parser<'source> { refr = Expr::RefDot { span, refr: Ref::new(refr), - field, + field: Self::span_and_value(field), }; } "[" => { self.next_token()?; let index = match &self.tok.0 { - TokenKind::String => Expr::String(self.tok.1.clone()), + TokenKind::String => Expr::String(Self::span_and_value(self.tok.1.clone())), _ => { return Err(self.source.error( self.tok.1.line, @@ -1140,7 +1171,7 @@ impl<'source> Parser<'source> { bail!(span.error("data cannot be shadowed")); } } - Expr::Var(v) + Expr::Var(Self::span_and_value(v)) } else { return Err(self.source.error( span.line, @@ -1184,7 +1215,7 @@ impl<'source> Parser<'source> { term = Expr::RefDot { span, refr: Ref::new(term), - field, + field: Self::span_and_value(field), }; } "[" => { @@ -1488,7 +1519,10 @@ impl<'source> Parser<'source> { Ok(Rule::Default { span, refr: rule_ref, - args: args.into_iter().map(|a| Ref::new(Expr::Var(a))).collect(), + args: args + .into_iter() + .map(|a| Ref::new(Expr::Var(Self::span_and_value(a)))) + .collect(), op, value, }) diff --git a/src/scheduler.rs b/src/scheduler.rs index 0eb7969..83f974b 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -299,23 +299,23 @@ fn gather_assigned_vars( ) -> Result<()> { traverse(expr, &mut |e| match e.as_ref() { // Ignore _, input, data. - Var(v) if matches!(v.text(), "_" | "input" | "data") => Ok(false), + Var(v) if matches!(v.0.text(), "_" | "input" | "data") => Ok(false), // Record local var that can shadow input var. Var(v) if can_shadow => { - scope.locals.insert(v.source_str(), v.clone()); + scope.locals.insert(v.0.source_str(), v.0.clone()); Ok(false) } // Record input vars. - Var(v) if var_exists(v, parent_scopes) => { - scope.inputs.insert(v.source_str()); + Var(v) if var_exists(&v.0, parent_scopes) => { + scope.inputs.insert(v.0.source_str()); Ok(false) } // Record local var. Var(v) => { - scope.unscoped.insert(v.source_str()); + scope.unscoped.insert(v.0.source_str()); Ok(false) } @@ -327,8 +327,10 @@ fn gather_assigned_vars( fn gather_input_vars(expr: &Ref, parent_scopes: &[Scope], scope: &mut Scope) -> Result<()> { traverse(expr, &mut |e| match e.as_ref() { - Var(v) if !scope.unscoped.contains(&v.source_str()) && var_exists(v, parent_scopes) => { - scope.inputs.insert(v.source_str()); + Var(v) + if !scope.unscoped.contains(&v.0.source_str()) && var_exists(&v.0, parent_scopes) => + { + scope.inputs.insert(v.0.source_str()); Ok(false) } _ => Ok(true), @@ -537,7 +539,7 @@ impl Analyzer { for a in args.iter() { traverse(a, &mut |e| { if let Var(v) = e.as_ref() { - scope.unscoped.insert(v.source_str()); + scope.unscoped.insert(v.0.source_str()); } Ok(true) })?; @@ -630,10 +632,10 @@ impl Analyzer { let full_expr = expr; std::convert::identity(&full_expr); traverse(expr, &mut |e| match e.as_ref() { - Var(v) if !matches!(v.text(), "_" | "input" | "data") => { - let name = v.source_str(); + Var(v) if !matches!(v.0.text(), "_" | "input" | "data") => { + let name = v.0.source_str(); let is_extra_arg = match assigned_vars { - Some(vars) => vars.contains(&v.source_str()), + Some(vars) => vars.contains(&v.0.source_str()), _ => false, }; @@ -642,7 +644,7 @@ impl Analyzer { { if !is_extra_arg { used_vars.push(name.clone()); - first_use.entry(name).or_insert(v.clone()); + first_use.entry(name).or_insert(v.0.clone()); } } else if !scope.inputs.contains(&name) { #[cfg(feature = "deprecated")] @@ -656,7 +658,9 @@ impl Analyzer { } } } - bail!(v.error(format!("use of undefined variable `{name}` is unsafe").as_str())); + bail!(v + .0 + .error(format!("use of undefined variable `{name}` is unsafe").as_str())); } Ok(false) } @@ -664,7 +668,7 @@ impl Analyzer { RefBrack { refr, index, .. } => { traverse(index, &mut |e| match e.as_ref() { Var(v) => { - let var = v.source_str(); + let var = v.0.source_str(); if scope.locals.contains_key(&var) || scope.unscoped.contains(&var) { let (rb_used_vars, rb_comprs) = Self::gather_used_vars_comprs_index_vars( @@ -758,10 +762,10 @@ impl Analyzer { let mut vars = vec![]; traverse(expr, &mut |e| match e.as_ref() { Var(v) => { - let var = v.source_str(); + let var = v.0.source_str(); if scope.locals.contains_key(&var) { if check_first_use { - Self::check_first_use(v, first_use)?; + Self::check_first_use(&v.0, first_use)?; } vars.push(var); } else if scope.unscoped.contains(&var) { @@ -947,8 +951,8 @@ impl Analyzer { non_vars: &mut Vec>, ) -> Result<()> { traverse(expr, &mut |e| match e.as_ref() { - Var(v) if scope.locals.contains_key(&v.source_str()) => { - vars.push(v.source_str()); + Var(v) if scope.locals.contains_key(&v.0.source_str()) => { + vars.push(v.0.source_str()); Ok(false) } // TODO: Object key/value diff --git a/src/utils.rs b/src/utils.rs index ef12eca..a03ffad 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -14,17 +14,17 @@ pub fn get_path_string(refr: &Expr, document: Option<&str>) -> Result { while expr.is_some() { match expr { Some(Expr::RefDot { refr, field, .. }) => { - comps.push(field.text()); + comps.push(field.0.text()); expr = Some(refr); } Some(Expr::RefBrack { refr, index, .. }) => { if let Expr::String(s) = index.as_ref() { - comps.push(s.text()); + comps.push(s.0.text()); } expr = Some(refr); } Some(Expr::Var(v)) => { - comps.push(v.text()); + comps.push(v.0.text()); expr = None; } _ => bail!("internal error: not a simple ref {expr:?}"), @@ -121,7 +121,7 @@ pub fn get_root_var(mut expr: &Expr) -> Result { let empty = expr.span().source_str().clone_empty(); loop { match expr { - Expr::Var(v) => return Ok(v.source_str()), + Expr::Var(v) => return Ok(v.0.source_str()), Expr::RefDot { refr, .. } | Expr::RefBrack { refr, .. } => expr = refr, _ => return Ok(empty), } diff --git a/tests/parser/mod.rs b/tests/parser/mod.rs index 65d721f..7f611fd 100644 --- a/tests/parser/mod.rs +++ b/tests/parser/mod.rs @@ -102,13 +102,13 @@ fn match_expr_impl(e: &Expr, v: &Value) -> Result<()> { return Ok(()); } match e { - Expr::String(s) => match_span(s, &v["string"]), - Expr::RawString(s) => match_span(s, &v["rawstring"]), - Expr::Number(s) => match_span(s, &v["number"]), + Expr::String(s) => match_span(&s.0, &v["string"]), + Expr::RawString(s) => match_span(&s.0, &v["rawstring"]), + Expr::Number(s) => match_span(&s.0, &v["number"]), Expr::True(s) => match_span(s, v), Expr::False(s) => match_span(s, v), Expr::Null(s) => match_span(s, v), - Expr::Var(s) => match_span(s, &v["var"]), + Expr::Var(s) => match_span(&s.0, &v["var"]), Expr::Array { span, items } => match_vec(span, items, &v["array"]), Expr::Set { span, items } => match_vec(span, items, &v["set"]), Expr::Object { span, fields } => match_object(span, fields, &v["object"]), @@ -141,7 +141,7 @@ fn match_expr_impl(e: &Expr, v: &Value) -> Result<()> { Expr::RefDot { span, refr, field } => { match_span_opt(span, &v["refdot"]["span"])?; match_expr(refr, &v["refdot"]["refr"])?; - match_span(field, &v["refdot"]["field"]) + match_span(&field.0, &v["refdot"]["field"]) } Expr::RefBrack { span, refr, index } => { match_span_opt(span, &v["refbrack"]["span"])?;