From a0727189d639b26a4bf42d192a83bdbf1f925a1c Mon Sep 17 00:00:00 2001 From: peefy Date: Wed, 14 Aug 2024 14:42:24 +0800 Subject: [PATCH] fix: evaluator if order bug Signed-off-by: peefy --- kclvm/evaluator/src/context.rs | 8 ++- kclvm/evaluator/src/lazy.rs | 56 ++++++++++++++++--- kclvm/evaluator/src/lib.rs | 7 ++- kclvm/evaluator/src/node.rs | 1 + kclvm/evaluator/src/schema.rs | 4 +- .../kclvm_evaluator__tests__if_stmt_7.snap | 7 +++ .../kclvm_evaluator__tests__if_stmt_8.snap | 9 +++ kclvm/evaluator/src/tests.rs | 34 +++++++++++ 8 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 kclvm/evaluator/src/snapshots/kclvm_evaluator__tests__if_stmt_7.snap create mode 100644 kclvm/evaluator/src/snapshots/kclvm_evaluator__tests__if_stmt_8.snap diff --git a/kclvm/evaluator/src/context.rs b/kclvm/evaluator/src/context.rs index 10658c0c2..6aab5f6a8 100644 --- a/kclvm/evaluator/src/context.rs +++ b/kclvm/evaluator/src/context.rs @@ -36,7 +36,7 @@ impl<'ctx> Evaluator<'ctx> { .to_string() } - /// Current runtime context kcl line + /// Update current runtime context kcl filename and line #[inline] pub(crate) fn update_ctx_panic_info(&self, node: &'ctx ast::Node) { let mut ctx = self.runtime_ctx.borrow_mut(); @@ -44,6 +44,12 @@ impl<'ctx> Evaluator<'ctx> { ctx.panic_info.kcl_line = node.line as i32; } + /// Update current AST index. + #[inline] + pub(crate) fn update_ast_id(&self, node: &'ctx ast::Node) { + *self.ast_id.borrow_mut() = node.id.clone(); + } + /// Push a lambda definition scope into the lambda stack #[inline] pub fn push_lambda( diff --git a/kclvm/evaluator/src/lazy.rs b/kclvm/evaluator/src/lazy.rs index 9fb24f230..54b427f9e 100644 --- a/kclvm/evaluator/src/lazy.rs +++ b/kclvm/evaluator/src/lazy.rs @@ -76,6 +76,28 @@ impl LazyEvalScope { next_cal_time >= self.setter_len(key) } } + + /// Whether the current target walker stmt index is the last setter stmt index. + #[inline] + pub fn is_last_setter_ast_index(&mut self, key: &str, id: &AstIndex) -> bool { + if self.is_backtracking(key) { + false + } else { + if let Some(setters) = self.setters.get(key) { + if let Some(s) = setters.last() { + if let Some(stopped) = &s.stopped { + stopped == id + } else { + &s.stmt_id == id + } + } else { + false + } + } else { + false + } + } + } } /// Setter kind. @@ -97,6 +119,8 @@ pub struct Setter { pub index: Option, /// Statement index in the schema or body in the body array. pub stmt: usize, + /// Statement AST index. + pub stmt_id: AstIndex, /// If the statement is a if statement, stop the backtrack process at the stopped statement index. pub stopped: Option, /// Setter kind. @@ -183,6 +207,7 @@ impl<'ctx> Evaluator<'ctx> { ) { let add_stmt = |name: &str, i: usize, + stmt_id: &AstIndex, stopped: Option, body_map: &mut IndexMap>, kind: SetterKind| { @@ -193,6 +218,7 @@ impl<'ctx> Evaluator<'ctx> { body_vec.push(Setter { index, stmt: i, + stmt_id: stmt_id.clone(), stopped, kind, }); @@ -204,7 +230,7 @@ impl<'ctx> Evaluator<'ctx> { if is_in_if { in_if_names.push((name.to_string(), stmt.id.clone())); } else { - add_stmt(name, i, None, body_map, SetterKind::Normal); + add_stmt(name, i, &stmt.id, None, body_map, SetterKind::Normal); } } ast::Stmt::Assign(assign_stmt) => { @@ -213,7 +239,7 @@ impl<'ctx> Evaluator<'ctx> { if is_in_if { in_if_names.push((name.to_string(), stmt.id.clone())); } else { - add_stmt(name, i, None, body_map, SetterKind::Normal); + add_stmt(name, i, &stmt.id, None, body_map, SetterKind::Normal); } } } @@ -223,7 +249,7 @@ impl<'ctx> Evaluator<'ctx> { if is_in_if { in_if_names.push((name.to_string(), stmt.id.clone())); } else { - add_stmt(name, i, None, body_map, SetterKind::Normal); + add_stmt(name, i, &stmt.id, None, body_map, SetterKind::Normal); } } ast::Stmt::If(if_stmt) => { @@ -235,7 +261,14 @@ impl<'ctx> Evaluator<'ctx> { } } else { for (name, id) in &if_names { - add_stmt(name, i, Some(id.clone()), body_map, SetterKind::If); + add_stmt( + name, + i, + &stmt.id, + Some(id.clone()), + body_map, + SetterKind::If, + ); } } let mut or_else_names: Vec<(String, AstIndex)> = vec![]; @@ -252,7 +285,14 @@ impl<'ctx> Evaluator<'ctx> { } } else { for (name, id) in &or_else_names { - add_stmt(name, i, Some(id.clone()), body_map, SetterKind::OrElse); + add_stmt( + name, + i, + &stmt.id, + Some(id.clone()), + body_map, + SetterKind::OrElse, + ); } } } @@ -261,7 +301,7 @@ impl<'ctx> Evaluator<'ctx> { if is_in_if { in_if_names.push((name.to_string(), stmt.id.clone())); } else { - add_stmt(name, i, None, body_map, SetterKind::Normal); + add_stmt(name, i, &stmt.id, None, body_map, SetterKind::Normal); } } _ => {} @@ -343,7 +383,9 @@ impl<'ctx> Evaluator<'ctx> { pub(crate) fn set_value_to_lazy_scope(&self, pkgpath: &str, key: &str, value: &ValueRef) { let mut lazy_scopes = self.lazy_scopes.borrow_mut(); let scope = lazy_scopes.get_mut(pkgpath).expect(INTERNAL_ERROR_MSG); - if scope.cal_increment(key) && scope.cache.get(key).is_none() { + if (scope.cal_increment(key) || scope.is_last_setter_ast_index(key, &self.ast_id.borrow())) + && scope.cache.get(key).is_none() + { scope.cache.insert(key.to_string(), value.clone()); } } diff --git a/kclvm/evaluator/src/lib.rs b/kclvm/evaluator/src/lib.rs index de5a1256b..5f5196fb1 100644 --- a/kclvm/evaluator/src/lib.rs +++ b/kclvm/evaluator/src/lib.rs @@ -38,7 +38,7 @@ use std::{cell::RefCell, panic::UnwindSafe}; use crate::error as kcl_error; use anyhow::Result; -use kclvm_ast::ast; +use kclvm_ast::ast::{self, AstIndex}; use kclvm_runtime::{Context, ValueRef}; /// SCALAR_KEY denotes the temp scalar key for the global variable json plan process. @@ -84,8 +84,10 @@ pub struct Evaluator<'ctx> { pub scope_covers: RefCell>, /// Local variables in the loop. pub local_vars: RefCell>, - /// Schema attr backtrack meta + /// Schema attr backtrack meta. pub backtrack_meta: RefCell>, + /// Current AST id for the evaluator walker. + pub ast_id: RefCell, } #[derive(Clone)] @@ -125,6 +127,7 @@ impl<'ctx> Evaluator<'ctx> { scope_covers: RefCell::new(Default::default()), local_vars: RefCell::new(Default::default()), backtrack_meta: RefCell::new(Default::default()), + ast_id: RefCell::new(AstIndex::default()), } } diff --git a/kclvm/evaluator/src/node.rs b/kclvm/evaluator/src/node.rs index 1e02094ec..a80f99504 100644 --- a/kclvm/evaluator/src/node.rs +++ b/kclvm/evaluator/src/node.rs @@ -39,6 +39,7 @@ impl<'ctx> TypedResultWalker<'ctx> for Evaluator<'ctx> { fn walk_stmt(&self, stmt: &'ctx ast::Node) -> Self::Result { backtrack_break_here!(self, stmt); self.update_ctx_panic_info(stmt); + self.update_ast_id(stmt); let value = match &stmt.node { ast::Stmt::TypeAlias(type_alias) => self.walk_type_alias_stmt(type_alias), ast::Stmt::Expr(expr_stmt) => self.walk_expr_stmt(expr_stmt), diff --git a/kclvm/evaluator/src/schema.rs b/kclvm/evaluator/src/schema.rs index c65f937c5..8168e8110 100644 --- a/kclvm/evaluator/src/schema.rs +++ b/kclvm/evaluator/src/schema.rs @@ -356,7 +356,9 @@ impl SchemaEvalContext { pub fn set_value(&self, s: &Evaluator, key: &str) { if let Some(scope) = &self.scope { let mut scope = scope.borrow_mut(); - if scope.cal_increment(key) && scope.cache.get(key).is_none() { + if (scope.cal_increment(key) || scope.is_last_setter_ast_index(key, &s.ast_id.borrow())) + && scope.cache.get(key).is_none() + { scope .cache .insert(key.to_string(), s.dict_get_value(&self.value, key)); diff --git a/kclvm/evaluator/src/snapshots/kclvm_evaluator__tests__if_stmt_7.snap b/kclvm/evaluator/src/snapshots/kclvm_evaluator__tests__if_stmt_7.snap new file mode 100644 index 000000000..afb873c8d --- /dev/null +++ b/kclvm/evaluator/src/snapshots/kclvm_evaluator__tests__if_stmt_7.snap @@ -0,0 +1,7 @@ +--- +source: evaluator/src/tests.rs +expression: "format!(\"{}\", evaluator.run().unwrap().1)" +--- +a: 3 +c: + a: 3 diff --git a/kclvm/evaluator/src/snapshots/kclvm_evaluator__tests__if_stmt_8.snap b/kclvm/evaluator/src/snapshots/kclvm_evaluator__tests__if_stmt_8.snap new file mode 100644 index 000000000..10f5ca505 --- /dev/null +++ b/kclvm/evaluator/src/snapshots/kclvm_evaluator__tests__if_stmt_8.snap @@ -0,0 +1,9 @@ +--- +source: evaluator/src/tests.rs +expression: "format!(\"{}\", evaluator.run().unwrap().1)" +--- +items: +- key2: value2 +c: + items: + - key2: value2 diff --git a/kclvm/evaluator/src/tests.rs b/kclvm/evaluator/src/tests.rs index 25c3c09a8..f05faf42a 100644 --- a/kclvm/evaluator/src/tests.rs +++ b/kclvm/evaluator/src/tests.rs @@ -194,6 +194,40 @@ else: if True: c = 1 "#} +evaluator_snapshot! {if_stmt_7, r#" +_a = 1 +if True: + _a = 2 + _a += 1 +a = _a + +schema Config: + _a = 1 + if True: + _a = 2 + _a += 1 + a = _a + +c = Config {} +"#} +evaluator_snapshot! {if_stmt_8, r#" +_items = [] +if False: + _items += [ {key1 = "value1"} ] +if True: + _items += [ {key2 = "value2"} ] +items = _items + +schema Config: + _items = [] + if False: + _items += [ {key1 = "value1"} ] + if True: + _items += [ {key2 = "value2"} ] + items = _items + +c = Config {} +"#} evaluator_snapshot! {import_stmt_0, r#"import math a = 1