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 <anakrish@microsoft.com>
This commit is contained in:
Anand Krishnamoorthi
2024-04-07 18:21:32 +05:30
committed by GitHub
parent e326f3c629
commit d2049d07f3
6 changed files with 125 additions and 101 deletions

View File

@@ -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<T> = NodeRef<T>;
#[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<Expr>,
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, .. }

View File

@@ -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<SourceStr, Value>;
@@ -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, &params[..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::<Value>(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!(
"{}",

View File

@@ -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<Expr> {
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<Expr> {
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::<Value>(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<Expr> {
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,
})

View File

@@ -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<Expr>, 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<Ref<Expr>>,
) -> 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

View File

@@ -14,17 +14,17 @@ pub fn get_path_string(refr: &Expr, document: Option<&str>) -> Result<String> {
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<SourceStr> {
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),
}

View File

@@ -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"])?;