Skip to content

Commit

Permalink
fix: evaluator if order bug
Browse files Browse the repository at this point in the history
Signed-off-by: peefy <[email protected]>
  • Loading branch information
Peefy committed Aug 14, 2024
1 parent 848b26b commit a072718
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 11 deletions.
8 changes: 7 additions & 1 deletion kclvm/evaluator/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,20 @@ 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<T>(&self, node: &'ctx ast::Node<T>) {
let mut ctx = self.runtime_ctx.borrow_mut();
ctx.panic_info.kcl_file = node.filename.clone();
ctx.panic_info.kcl_line = node.line as i32;
}

/// Update current AST index.
#[inline]
pub(crate) fn update_ast_id<T>(&self, node: &'ctx ast::Node<T>) {
*self.ast_id.borrow_mut() = node.id.clone();
}

/// Push a lambda definition scope into the lambda stack
#[inline]
pub fn push_lambda(
Expand Down
56 changes: 49 additions & 7 deletions kclvm/evaluator/src/lazy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -97,6 +119,8 @@ pub struct Setter {
pub index: Option<Index>,
/// 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<AstIndex>,
/// Setter kind.
Expand Down Expand Up @@ -183,6 +207,7 @@ impl<'ctx> Evaluator<'ctx> {
) {
let add_stmt = |name: &str,
i: usize,
stmt_id: &AstIndex,
stopped: Option<AstIndex>,
body_map: &mut IndexMap<String, Vec<Setter>>,
kind: SetterKind| {
Expand All @@ -193,6 +218,7 @@ impl<'ctx> Evaluator<'ctx> {
body_vec.push(Setter {
index,
stmt: i,
stmt_id: stmt_id.clone(),
stopped,
kind,
});
Expand All @@ -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) => {
Expand All @@ -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);
}
}
}
Expand All @@ -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) => {
Expand All @@ -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![];
Expand All @@ -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,
);
}
}
}
Expand All @@ -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);
}
}
_ => {}
Expand Down Expand Up @@ -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());
}
}
Expand Down
7 changes: 5 additions & 2 deletions kclvm/evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -84,8 +84,10 @@ pub struct Evaluator<'ctx> {
pub scope_covers: RefCell<Vec<(usize, usize)>>,
/// Local variables in the loop.
pub local_vars: RefCell<HashSet<String>>,
/// Schema attr backtrack meta
/// Schema attr backtrack meta.
pub backtrack_meta: RefCell<Vec<BacktrackMeta>>,
/// Current AST id for the evaluator walker.
pub ast_id: RefCell<AstIndex>,
}

#[derive(Clone)]
Expand Down Expand Up @@ -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()),
}
}

Expand Down
1 change: 1 addition & 0 deletions kclvm/evaluator/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ impl<'ctx> TypedResultWalker<'ctx> for Evaluator<'ctx> {
fn walk_stmt(&self, stmt: &'ctx ast::Node<ast::Stmt>) -> 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),
Expand Down
4 changes: 3 additions & 1 deletion kclvm/evaluator/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
source: evaluator/src/tests.rs
expression: "format!(\"{}\", evaluator.run().unwrap().1)"
---
a: 3
c:
a: 3
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
source: evaluator/src/tests.rs
expression: "format!(\"{}\", evaluator.run().unwrap().1)"
---
items:
- key2: value2
c:
items:
- key2: value2
34 changes: 34 additions & 0 deletions kclvm/evaluator/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a072718

Please sign in to comment.