From aa80c8a24ded0aa266e35a40bcaa8d4f3bd7ddfd Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Tue, 30 Jan 2024 22:52:33 +0100 Subject: [PATCH] Use the shared Env type for values This is a backport of a larger refactor that I'm making in the typechecker. This change turned out to be huge, so I'm breaking it down into pieces. One of those pieces is having an Env type that can be used both for types and for values. The change that I'm backporting this from, first started using the new Env type for types and then later also for values. But in this case, we apply it to values first, and I'll rebase the typechecker, so it will start to use this type later. But it will be compatible, and unified. --- fuzz/fuzz_targets/main.rs | 4 +- fuzz/fuzz_targets/string_len.rs | 2 +- pyrcl/src/lib.rs | 4 +- src/eval.rs | 29 +++++--- src/main.rs | 13 ++-- src/runtime.rs | 116 ++------------------------------ src/typecheck.rs | 22 ++++++ 7 files changed, 58 insertions(+), 132 deletions(-) diff --git a/fuzz/fuzz_targets/main.rs b/fuzz/fuzz_targets/main.rs index 1eea12d..b5ae25d 100644 --- a/fuzz/fuzz_targets/main.rs +++ b/fuzz/fuzz_targets/main.rs @@ -87,7 +87,7 @@ fn fuzz_eval(loader: &mut Loader, input: &str) -> Result<()> { let id = loader.load_string(input.to_string()); let mut tracer = VoidTracer; let mut evaluator = Evaluator::new(loader, &mut tracer); - let mut env = rcl::runtime::Env::with_prelude(); + let mut env = rcl::runtime::prelude(); let _ = evaluator.eval_doc(&mut env, id)?; Ok(()) } @@ -119,7 +119,7 @@ fn fuzz_fmt(loader: &mut Loader, input: &str, cfg: pprint::Config) -> Result<()> /// expression, which should evaluate to `x`. fn fuzz_eval_json(loader: &mut Loader, input: &str, cfg: pprint::Config) -> Result<()> { let mut tracer = VoidTracer; - let mut env = rcl::runtime::Env::with_prelude(); + let mut env = rcl::runtime::prelude(); let doc_1 = loader.load_string(input.to_string()); let val_1 = loader.evaluate(doc_1, &mut env, &mut tracer)?; diff --git a/fuzz/fuzz_targets/string_len.rs b/fuzz/fuzz_targets/string_len.rs index 933afa5..ca5167b 100644 --- a/fuzz/fuzz_targets/string_len.rs +++ b/fuzz/fuzz_targets/string_len.rs @@ -32,7 +32,7 @@ fuzz_target!(|input: &str| { let mut loader = Loader::new(); let id = loader.load_string(expr_str); let mut evaluator = Evaluator::new(&mut loader, &mut tracer); - let mut env = rcl::runtime::Env::with_prelude(); + let mut env = rcl::runtime::prelude(); let v = evaluator.eval_doc(&mut env, id).expect("Expression is valid."); assert_eq!(v, Value::Bool(true)); diff --git a/pyrcl/src/lib.rs b/pyrcl/src/lib.rs index 64c89f5..38a14e9 100644 --- a/pyrcl/src/lib.rs +++ b/pyrcl/src/lib.rs @@ -9,7 +9,7 @@ use pyo3::prelude::*; use rcl::cli::Target; use rcl::error::Result; use rcl::loader::{Loader, SandboxMode}; -use rcl::runtime::{Env, Value}; +use rcl::runtime::{self, Value}; use rcl::source::DocId; use rcl::tracer::StderrTracer; @@ -18,7 +18,7 @@ fn evaluate Result>(load: F) -> Result { loader.initialize_filesystem(SandboxMode::Workdir, None)?; let doc = load(&mut loader)?; let mut tracer = StderrTracer::new(None); - let mut env = Env::with_prelude(); + let mut env = runtime::prelude(); loader.evaluate(doc, &mut env, &mut tracer) } diff --git a/src/eval.rs b/src/eval.rs index 1caad1b..abbbfd3 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -15,7 +15,9 @@ use crate::error::{Error, IntoError, Result}; use crate::fmt_rcl::{self, format_rcl}; use crate::loader::Loader; use crate::pprint::{concat, indent, Doc}; -use crate::runtime::{CallArg, Env, Function, FunctionCall, MethodCall, MethodInstance, Value}; +use crate::runtime::{ + self, CallArg, Env, Function, FunctionCall, MethodCall, MethodInstance, Value, +}; use crate::source::{DocId, Span}; use crate::stdlib; use crate::tracer::Tracer; @@ -124,6 +126,11 @@ pub struct Evaluator<'a> { /// /// This is used to break infinite loops. pub eval_count: EvalCount, + + /// The (static) environment for the types that are in scope. + /// + /// TODO: This will be removed with the static typechecker refactor. + pub type_env: crate::env::Env>, } impl<'a> Evaluator<'a> { @@ -135,6 +142,7 @@ impl<'a> Evaluator<'a> { stdlib: stdlib::initialize(), eval_depth: 0, eval_count: EvalCount::new(), + type_env: typecheck::prelude(), } } @@ -207,6 +215,7 @@ impl<'a> Evaluator<'a> { } let expr = self.loader.get_ast(doc)?; + let ctx = EvalContext { doc, imported_from: Some(imported_from), @@ -214,7 +223,7 @@ impl<'a> Evaluator<'a> { // Evaluate the import in its own clean environment, it should not be // affected by the surrounding environment of the import statement. - let mut env = Env::with_prelude(); + let mut env = runtime::prelude(); self.import_stack.push(ctx); let result = self.eval_expr(&mut env, &expr)?; @@ -333,7 +342,7 @@ impl<'a> Evaluator<'a> { result } - Expr::Var { span, ident } => match env.lookup_value(ident) { + Expr::Var { ident, span } => match env.lookup(ident) { Some(value) => Ok(value.clone()), None => Err(span.error("Unknown variable.").into()), }, @@ -592,7 +601,7 @@ impl<'a> Evaluator<'a> { // have to clone the full thing. let mut env = fun.env.clone(); for (arg_name, CallArg { value, .. }) in fun.args.iter().zip(call.args) { - env.push_value(arg_name.clone(), value.clone()); + env.push(arg_name.clone(), value.clone()); } self.eval_expr(&mut env, fun.body.as_ref()) @@ -857,7 +866,7 @@ impl<'a> Evaluator<'a> { typecheck::check_value(*ident_span, type_.as_ref(), &v)?; } - env.push_value(ident.clone(), v); + env.push(ident.clone(), v); } Stmt::Assert { condition_span, @@ -950,7 +959,7 @@ impl<'a> Evaluator<'a> { match (&idents[..], collection_value) { ([name], Value::List(xs)) => { for x in xs.iter() { - let ck = env.push_value(name.clone(), x.clone()); + let ck = env.push(name.clone(), x.clone()); self.eval_seq(env, body, out)?; env.pop(ck); } @@ -965,7 +974,7 @@ impl<'a> Evaluator<'a> { } ([name], Value::Set(xs)) => { for x in xs.iter() { - let ck = env.push_value(name.clone(), x.clone()); + let ck = env.push(name.clone(), x.clone()); self.eval_seq(env, body, out)?; env.pop(ck); } @@ -981,8 +990,8 @@ impl<'a> Evaluator<'a> { ([k_name, v_name], Value::Dict(xs)) => { for (k, v) in xs.iter() { let ck = env.checkpoint(); - env.push_value(k_name.clone(), k.clone()); - env.push_value(v_name.clone(), v.clone()); + env.push(k_name.clone(), k.clone()); + env.push(v_name.clone(), v.clone()); self.eval_seq(env, body, out)?; env.pop(ck); } @@ -1029,7 +1038,7 @@ impl<'a> Evaluator<'a> { fn eval_type_expr(&mut self, env: &mut Env, type_: &AType) -> Result> { match type_ { - AType::Term { span, name } => match env.lookup_type(name) { + AType::Term { span, name } => match self.type_env.lookup(name) { Some(t) => Ok(t.clone()), None => { let err = span.error("Unknown type."); diff --git a/src/main.rs b/src/main.rs index fb13a4c..dd6a134 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,8 +12,7 @@ use rcl::error::{Error, Result}; use rcl::loader::{Loader, SandboxMode}; use rcl::markup::MarkupMode; use rcl::pprint; -use rcl::runtime::Env; -use rcl::runtime::Value; +use rcl::runtime::{self, Value}; use rcl::source::{DocId, Span}; use rcl::tracer::StderrTracer; @@ -116,7 +115,7 @@ impl App { .initialize_filesystem(eval_opts.sandbox, self.opts.workdir.as_deref())?; let mut tracer = self.get_tracer(); - let mut env = Env::with_prelude(); + let mut env = runtime::prelude(); let doc = self.loader.load_cli_target(fname)?; let val = self.loader.evaluate(doc, &mut env, &mut tracer)?; // TODO: Need to get last inner span. @@ -138,13 +137,13 @@ impl App { // First we evaluate the input document. let mut tracer = self.get_tracer(); - let mut env = Env::with_prelude(); + let mut env = runtime::prelude(); let val_input = self.loader.evaluate(input, &mut env, &mut tracer)?; // Then we bind that to the variable `input`, and in that context, we - // evaluate the query expression. - let mut env = Env::with_prelude(); - env.push_value("input".into(), val_input); + // evaluate the query expression. The environment should be + // clean at this point, so we can reuse it. + env.push("input".into(), val_input); let val_result = self.loader.evaluate(query, &mut env, &mut tracer)?; let full_span = self.loader.get_span(query); diff --git a/src/runtime.rs b/src/runtime.rs index 6b926e9..f55addb 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -16,7 +16,6 @@ use crate::error::{IntoError, Result}; use crate::eval::Evaluator; use crate::pprint::{concat, Doc}; use crate::source::Span; -use crate::types::Type; /// A value provided as argument to a function call. pub struct CallArg { @@ -294,116 +293,13 @@ impl<'a> From<&'a str> for Value { } /// An environment binds names to values. -#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] -pub struct Env { - value_bindings: Vec<(Ident, Value)>, - type_bindings: Vec<(Ident, Rc)>, -} +pub type Env = crate::env::Env; -/// References a version of an environment that we can later restore to. -#[derive(Copy, Clone)] -pub struct EnvCheckpoint { - values_len: usize, - types_len: usize, -} - -impl Env { - /// Create a new empty environment. - pub fn new() -> Env { - Env { - value_bindings: Vec::new(), - type_bindings: Vec::new(), - } - } - - /// Create a new environment with an initialized standard library. - pub fn with_prelude() -> Env { - let mut env = Env::new(); - env.push_value("std".into(), crate::stdlib::initialize()); - - // The primitive types are in scope by default. - env.push_type("Bool".into(), Rc::new(Type::Bool)); - env.push_type("Int".into(), Rc::new(Type::Int)); - env.push_type("Null".into(), Rc::new(Type::Null)); - env.push_type("String".into(), Rc::new(Type::String)); - - // TODO: What to do about Dict, List, and Set? They are technically type - // constructors. Should those exist, at this level, if they can't be - // user-defined? It's easier to implement if we just hard-code those few, - // but then if you write `let xs: List = [1, 2, 3]`, it will lead to a - // confusing error. - - env - } - - pub fn lookup_value(&self, name: &Ident) -> Option<&Value> { - self.value_bindings - .iter() - .rev() - .find(|(k, _v)| k == name) - .map(|(_k, v)| v) - } - - pub fn lookup_type(&self, name: &Ident) -> Option<&Rc> { - self.type_bindings - .iter() - .rev() - .find(|(k, _v)| k == name) - .map(|(_k, v)| v) - } - - /// Return a checkpoint of the environment to later [`Env::pop`] to. - /// - /// Note, the environment is a stack and the pushes and pops have to be - /// balanced; popping can only remove bindings from the environment again, - /// it does not _restore_ the environment to that state, like something - /// transactional would, or a persistent data structure. - pub fn checkpoint(&self) -> EnvCheckpoint { - EnvCheckpoint { - values_len: self.value_bindings.len(), - types_len: self.type_bindings.len(), - } - } - - /// Push a value binding into the environment. - /// - /// If the name already existed, the new push will shadow the old one. - /// - /// Returns a checkpoint of the environment before the push. - pub fn push_value(&mut self, name: Ident, value: Value) -> EnvCheckpoint { - let checkpoint = self.checkpoint(); - self.value_bindings.push((name, value)); - checkpoint - } - - /// Push a type binding into the environment. - /// - /// If the name already existed, the new push will shadow the old one. - /// - /// Returns a checkpoint of the environment before the push. - pub fn push_type(&mut self, name: Ident, type_: Rc) -> EnvCheckpoint { - let checkpoint = self.checkpoint(); - self.type_bindings.push((name, type_)); - checkpoint - } - - /// Pop bindings to get back to a previous version of the environment. - pub fn pop(&mut self, to: EnvCheckpoint) { - let EnvCheckpoint { - values_len, - types_len, - } = to; - debug_assert!( - self.value_bindings.len() >= values_len, - "Cannot restore to checkpoint, more got popped already.", - ); - debug_assert!( - self.type_bindings.len() >= types_len, - "Cannot restore to checkpoint, more got popped already.", - ); - self.value_bindings.truncate(values_len); - self.type_bindings.truncate(types_len); - } +/// Create a new environment with an initialized standard library. +pub fn prelude() -> Env { + let mut env = Env::new(); + env.push("std".into(), crate::stdlib::initialize()); + env } macro_rules! builtin_function { diff --git a/src/typecheck.rs b/src/typecheck.rs index 899c6a7..9ae7005 100644 --- a/src/typecheck.rs +++ b/src/typecheck.rs @@ -12,6 +12,9 @@ //! but also for the type `List[String]`. Therefore we check whether a value //! _fits_ a particular type, and that same value may fit multiple types. +use std::rc::Rc; + +use crate::env::Env; use crate::error::{IntoError, Result}; use crate::fmt_rcl::format_rcl; use crate::fmt_type::format_type; @@ -20,6 +23,25 @@ use crate::runtime::{self, Value}; use crate::source::Span; use crate::types::{self, Type}; +/// Return the type prelude, all the types that are in scope by default. +pub fn prelude() -> Env> { + let mut env = Env::new(); + + // The primitive types are in scope by default. + env.push("Bool".into(), Rc::new(Type::Bool)); + env.push("Int".into(), Rc::new(Type::Int)); + env.push("Null".into(), Rc::new(Type::Null)); + env.push("String".into(), Rc::new(Type::String)); + + // TODO: What to do about Dict, List, and Set? They are technically type + // constructors. Should those exist, at this level, if they can't be + // user-defined? It's easier to implement if we just hard-code those few, + // but then if you write `let xs: List = [1, 2, 3]`, it will lead to a + // confusing error. + + env +} + /// Confirm that the value fits the given type. pub fn check_value(at: Span, type_: &Type, value: &Value) -> Result<()> { match (type_, value) {