From d59c4a22580d320ef9adcde08e9f2b17f16061c7 Mon Sep 17 00:00:00 2001 From: Adam Chalmers Date: Fri, 12 Jan 2024 14:42:42 -0600 Subject: [PATCH] Grackle: Compile member expressions (#1290) Member expressions like "obj.property" just look up "property" under the binding for "obj". --- src/wasm-lib/Cargo.lock | 8 ++-- src/wasm-lib/grackle/src/binding_scope.rs | 33 ++++++++++++++++ src/wasm-lib/grackle/src/kcl_value_group.rs | 5 +-- src/wasm-lib/grackle/src/lib.rs | 44 ++++++++++++++++++++- src/wasm-lib/grackle/src/tests.rs | 43 ++++++++++++++++++++ src/wasm-lib/kcl/src/parser/parser_impl.rs | 2 +- 6 files changed, 125 insertions(+), 10 deletions(-) diff --git a/src/wasm-lib/Cargo.lock b/src/wasm-lib/Cargo.lock index f42e6f24e..6ee3d410a 100644 --- a/src/wasm-lib/Cargo.lock +++ b/src/wasm-lib/Cargo.lock @@ -1943,7 +1943,7 @@ dependencies = [ [[package]] name = "kittycad-execution-plan" version = "0.1.0" -source = "git+https://github.com/KittyCAD/modeling-api?branch=main#627d6b1c861ac1d534b4fecc280aa8c9b7d5df03" +source = "git+https://github.com/KittyCAD/modeling-api?branch=main#9200b9540fa5ae99b692db276c625223116f467f" dependencies = [ "bytes", "insta", @@ -1972,7 +1972,7 @@ dependencies = [ [[package]] name = "kittycad-execution-plan-macros" version = "0.1.2" -source = "git+https://github.com/KittyCAD/modeling-api?branch=main#627d6b1c861ac1d534b4fecc280aa8c9b7d5df03" +source = "git+https://github.com/KittyCAD/modeling-api?branch=main#9200b9540fa5ae99b692db276c625223116f467f" dependencies = [ "proc-macro2", "quote", @@ -1993,7 +1993,7 @@ dependencies = [ [[package]] name = "kittycad-modeling-cmds" version = "0.1.11" -source = "git+https://github.com/KittyCAD/modeling-api?branch=main#627d6b1c861ac1d534b4fecc280aa8c9b7d5df03" +source = "git+https://github.com/KittyCAD/modeling-api?branch=main#9200b9540fa5ae99b692db276c625223116f467f" dependencies = [ "anyhow", "chrono", @@ -2020,7 +2020,7 @@ dependencies = [ [[package]] name = "kittycad-modeling-session" version = "0.1.0" -source = "git+https://github.com/KittyCAD/modeling-api?branch=main#627d6b1c861ac1d534b4fecc280aa8c9b7d5df03" +source = "git+https://github.com/KittyCAD/modeling-api?branch=main#9200b9540fa5ae99b692db276c625223116f467f" dependencies = [ "futures", "kittycad", diff --git a/src/wasm-lib/grackle/src/binding_scope.rs b/src/wasm-lib/grackle/src/binding_scope.rs index 1f51e6c95..03d883f6a 100644 --- a/src/wasm-lib/grackle/src/binding_scope.rs +++ b/src/wasm-lib/grackle/src/binding_scope.rs @@ -1,3 +1,8 @@ +use kcl_lib::ast::types::LiteralIdentifier; +use kcl_lib::ast::types::LiteralValue; + +use crate::CompileError; + use super::native_functions; use super::Address; use super::KclFunction; @@ -18,6 +23,34 @@ pub enum EpBinding { Map(HashMap), } +impl EpBinding { + /// Look up the given property of this binding. + pub fn property_of(&self, property: LiteralIdentifier) -> Result<&Self, CompileError> { + match property { + LiteralIdentifier::Identifier(_) => todo!("Support identifier properties"), + LiteralIdentifier::Literal(litval) => match litval.value { + // Arrays can be indexed by integers. + LiteralValue::IInteger(i) => match self { + EpBinding::Single(_) => Err(CompileError::CannotIndex), + EpBinding::Sequence(seq) => { + let i = usize::try_from(i).map_err(|_| CompileError::InvalidIndex(i.to_string()))?; + seq.get(i).ok_or(CompileError::IndexOutOfBounds { i, len: seq.len() }) + } + EpBinding::Map(_) => Err(CompileError::CannotIndex), + }, + // Objects can be indexed by string properties. + LiteralValue::String(property) => match self { + EpBinding::Single(_) => Err(CompileError::NoProperties), + EpBinding::Sequence(_) => Err(CompileError::ArrayDoesNotHaveProperties), + EpBinding::Map(map) => map.get(&property).ok_or(CompileError::UndefinedProperty { property }), + }, + // It's never valid to index by a fractional number. + LiteralValue::Fractional(num) => Err(CompileError::InvalidIndex(num.to_string())), + }, + } + } +} + /// A set of bindings in a particular scope. /// Bindings are KCL values that get "compiled" into KCEP values, which are stored in KCEP memory /// at a particular KCEP address. diff --git a/src/wasm-lib/grackle/src/kcl_value_group.rs b/src/wasm-lib/grackle/src/kcl_value_group.rs index b2111d86a..480a4e867 100644 --- a/src/wasm-lib/grackle/src/kcl_value_group.rs +++ b/src/wasm-lib/grackle/src/kcl_value_group.rs @@ -58,9 +58,8 @@ impl From for KclValueGroup { ast::types::Value::UnaryExpression(e) => Self::Single(SingleValue::UnaryExpression(e)), ast::types::Value::ArrayExpression(e) => Self::ArrayExpression(e), ast::types::Value::ObjectExpression(e) => Self::ObjectExpression(e), - ast::types::Value::PipeSubstitution(_) - | ast::types::Value::FunctionExpression(_) - | ast::types::Value::MemberExpression(_) => todo!(), + ast::types::Value::MemberExpression(e) => Self::Single(SingleValue::MemberExpression(e)), + ast::types::Value::PipeSubstitution(_) | ast::types::Value::FunctionExpression(_) => todo!(), } } } diff --git a/src/wasm-lib/grackle/src/lib.rs b/src/wasm-lib/grackle/src/lib.rs index a2eee1ef1..57d400d89 100644 --- a/src/wasm-lib/grackle/src/lib.rs +++ b/src/wasm-lib/grackle/src/lib.rs @@ -179,9 +179,35 @@ impl Planner { instructions.extend(eval_instrs); Ok(EvalPlan { instructions, binding }) } + SingleValue::MemberExpression(mut expr) => { + let parse = move || { + let mut stack = Vec::new(); + loop { + stack.push((expr.property, expr.computed)); + match expr.object { + ast::types::MemberObject::MemberExpression(subexpr) => { + expr = subexpr; + } + ast::types::MemberObject::Identifier(id) => return (stack, id), + } + } + }; + let (properties, id) = parse(); + let name = id.name; + let mut binding = self.binding_scope.get(&name).ok_or(CompileError::Undefined { name })?; + for (property, computed) in properties { + if computed { + todo!("Support computed properties"); + } + binding = binding.property_of(property)?; + } + Ok(EvalPlan { + instructions: Vec::new(), + binding: binding.clone(), + }) + } SingleValue::PipeExpression(_) => todo!(), SingleValue::UnaryExpression(_) => todo!(), - SingleValue::MemberExpression(_) => todo!(), } } @@ -326,7 +352,7 @@ impl Planner { } } -#[derive(Debug, thiserror::Error, Eq, PartialEq, Clone)] +#[derive(Debug, thiserror::Error, PartialEq, Clone)] pub enum CompileError { #[error("the name {name} was not defined")] Undefined { name: String }, @@ -346,6 +372,20 @@ pub enum CompileError { NotCallable { name: String }, #[error("you're trying to use an operand that isn't compatible with the given arithmetic operator: {0}")] InvalidOperand(&'static str), + #[error("you cannot use the value {0} as an index")] + InvalidIndex(String), + #[error("you tried to index into a value that isn't an array. Only arrays have numeric indices!")] + CannotIndex, + #[error("you tried to get the element {i} but that index is out of bounds. The array only has a length of {len}")] + IndexOutOfBounds { i: usize, len: usize }, + #[error("you tried to access the property of a value that doesn't have any properties")] + NoProperties, + #[error("you tried to access a property of an array, but arrays don't have properties. They do have numeric indexes though, try using an index e.g. [0]")] + ArrayDoesNotHaveProperties, + #[error( + "you tried to read the '.{property}' of an object, but the object doesn't have any properties with that key" + )] + UndefinedProperty { property: String }, } #[derive(Debug, thiserror::Error)] diff --git a/src/wasm-lib/grackle/src/tests.rs b/src/wasm-lib/grackle/src/tests.rs index d43d4722e..444c4bcfe 100644 --- a/src/wasm-lib/grackle/src/tests.rs +++ b/src/wasm-lib/grackle/src/tests.rs @@ -171,6 +171,49 @@ fn use_native_function_id() { ); } +#[test] +fn member_expressions_object() { + let program = r#" + let obj = {x: 1, y: 2} + let prop = obj["y"] + "#; + let (_plan, scope) = must_plan(program); + match scope.get("prop").unwrap() { + EpBinding::Single(addr) => { + assert_eq!(*addr, Address::ZERO + 1); + } + other => { + panic!("expected 'number' bound to 0x0 but it was bound to {other:?}"); + } + } +} + +#[test] +fn member_expressions_array() { + let program = " + let array = [[1,2],[3,4]] + let first = array[0][0] + let last = array[1][1] + "; + let (_plan, scope) = must_plan(program); + match scope.get("first").unwrap() { + EpBinding::Single(addr) => { + assert_eq!(*addr, Address::ZERO); + } + other => { + panic!("expected 'number' bound to 0x0 but it was bound to {other:?}"); + } + } + match scope.get("last").unwrap() { + EpBinding::Single(addr) => { + assert_eq!(*addr, Address::ZERO + 3); + } + other => { + panic!("expected 'number' bound to 0x3 but it was bound to {other:?}"); + } + } +} + #[test] fn add_literals() { let program = "let x = 1 + 2"; diff --git a/src/wasm-lib/kcl/src/parser/parser_impl.rs b/src/wasm-lib/kcl/src/parser/parser_impl.rs index 8d19fd186..b7b561cea 100644 --- a/src/wasm-lib/kcl/src/parser/parser_impl.rs +++ b/src/wasm-lib/kcl/src/parser/parser_impl.rs @@ -2721,7 +2721,7 @@ show(b1) show(b2)"#; let tokens = crate::token::lexer(some_program_string); let parser = crate::parser::Parser::new(tokens); - parser.ast().unwrap(); + dbg!(parser.ast().unwrap()); } #[test]