Skip to content

Commit a31af5c

Browse files
committed
fix: schema lambda calling
Signed-off-by: Peefy <xpf6677@163.com>
1 parent 7ec2561 commit a31af5c

File tree

12 files changed

+274
-22
lines changed

12 files changed

+274
-22
lines changed

crates/evaluator/src/context.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,68 @@ impl<'ctx> Evaluator<'ctx> {
7474
.push(LambdaOrSchemaEvalContext::Lambda(lambda_ctx));
7575
}
7676

77+
/// Push a lambda definition scope into the lambda stack with an optional receiver
78+
pub fn push_lambda_with_receiver(
79+
&self,
80+
lambda_ctx: FunctionEvalContextRef,
81+
current_pkgpath: &str,
82+
frame_pkgpath: &str,
83+
level: usize,
84+
receiver: Option<&kcl_runtime::ValueRef>,
85+
) {
86+
// Capture function schema this reference.
87+
// Use the provided receiver if available, otherwise use captured this
88+
if let Some(recv) = receiver {
89+
if recv.is_schema() || recv.is_dict() {
90+
// For schema/dict method calls, use the captured this as a template
91+
// but update its value with the receiver
92+
if let Some(this_ctx) = &lambda_ctx.this {
93+
// Create a new schema context using the captured context as template,
94+
// but with the receiver's value and config
95+
let updated_ctx = match &this_ctx.ctx {
96+
EvalContext::Schema(schema_ctx) => {
97+
// For schemas, config is the same as value
98+
// For dicts, use the dict itself as config
99+
let config = recv.clone();
100+
let new_ctx = schema_ctx.borrow().new_with_value(recv, &config);
101+
EvalContext::Schema(new_ctx)
102+
}
103+
EvalContext::Rule(_) => {
104+
// Should not happen for schema methods
105+
this_ctx.eval_ctx()
106+
}
107+
};
108+
self.push_schema(updated_ctx);
109+
} else {
110+
// No captured this, but receiver is schema/dict
111+
// Try to find a schema context from the stack to use as template
112+
if let Some(ctx) = self.schema_stack.borrow().last()
113+
&& let EvalContext::Schema(schema_ctx) = ctx
114+
{
115+
let config = recv.clone();
116+
let new_ctx = schema_ctx.borrow().new_with_value(recv, &config);
117+
self.push_schema(EvalContext::Schema(new_ctx));
118+
}
119+
}
120+
} else if let Some(this) = &lambda_ctx.this {
121+
self.push_schema(this.eval_ctx());
122+
}
123+
} else if let Some(this) = &lambda_ctx.this {
124+
self.push_schema(this.eval_ctx());
125+
}
126+
127+
// Inner scope function calling.
128+
// Note the minimum lambda.ctx.level is 2 for the top level lambda definitions.
129+
if frame_pkgpath == current_pkgpath && level >= lambda_ctx.level {
130+
// The scope cover is [lambda.ctx.level, self.scope_level()]
131+
self.push_scope_cover(lambda_ctx.level, level);
132+
}
133+
self.lambda_stack.borrow_mut().push(lambda_ctx.clone());
134+
self.ctx_stack
135+
.borrow_mut()
136+
.push(LambdaOrSchemaEvalContext::Lambda(lambda_ctx));
137+
}
138+
77139
/// Pop a lambda definition scope.
78140
#[inline]
79141
pub fn pop_lambda(

crates/evaluator/src/func.rs

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,25 @@ impl FunctionCaller {
8383
}
8484

8585
impl<'ctx> Evaluator<'ctx> {
86-
#[inline]
8786
pub(crate) fn invoke_proxy_function(
8887
&'ctx self,
8988
proxy_index: Index,
9089
args: &ValueRef,
9190
kwargs: &ValueRef,
9291
) -> ValueRef {
92+
self.invoke_proxy_function_with_receiver(proxy_index, args, kwargs, None)
93+
}
94+
95+
pub(crate) fn invoke_proxy_function_with_receiver(
96+
&'ctx self,
97+
proxy_index: Index,
98+
args: &ValueRef,
99+
kwargs: &ValueRef,
100+
receiver: Option<&ValueRef>,
101+
) -> ValueRef {
102+
// Capture the current_method_receiver before it might be overwritten by arg evaluation
103+
let stored_receiver = self.current_method_receiver.borrow().clone();
104+
93105
let frame = {
94106
let frames = self.frames.borrow();
95107
frames
@@ -114,9 +126,23 @@ impl<'ctx> Evaluator<'ctx> {
114126
// Push the current lambda scope level in the lambda stack.
115127
let pkgpath = self.current_pkgpath();
116128
let level = self.scope_level();
117-
self.push_lambda(lambda.ctx.clone(), &pkgpath, &frame.pkgpath, level);
129+
130+
// Use the stored receiver we captured earlier
131+
let effective_receiver = receiver.or(stored_receiver.as_ref());
132+
133+
self.push_lambda_with_receiver(
134+
lambda.ctx.clone(),
135+
&pkgpath,
136+
&frame.pkgpath,
137+
level,
138+
effective_receiver,
139+
);
118140
let value = (lambda.body)(self, &lambda.ctx, args, kwargs);
119141
self.pop_lambda(lambda.ctx.clone(), &pkgpath, &frame.pkgpath, level);
142+
143+
// Clear the stored receiver after lambda execution completes
144+
*self.current_method_receiver.borrow_mut() = None;
145+
120146
value
121147
}
122148
// Call a schema and return the schema value.

crates/evaluator/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ pub struct Evaluator<'ctx> {
9191
pub backtrack_meta: RefCell<Vec<BacktrackMeta>>,
9292
/// Current AST id for the evaluator walker.
9393
pub ast_id: RefCell<AstIndex>,
94+
/// Current method receiver for schema method calls (e.g., b2 in b2.add())
95+
pub current_method_receiver: RefCell<Option<kcl_runtime::ValueRef>>,
9496
}
9597

9698
#[derive(Clone)]
@@ -156,6 +158,7 @@ impl<'ctx> Evaluator<'ctx> {
156158
backtrack_meta: RefCell::new(Default::default()),
157159
ast_id: RefCell::new(AstIndex::default()),
158160
ctx_stack: RefCell::new(Default::default()),
161+
current_method_receiver: RefCell::new(None),
159162
}
160163
}
161164

crates/evaluator/src/node.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -954,6 +954,8 @@ impl<'ctx> TypedResultWalker<'ctx> for Evaluator<'ctx> {
954954
fn walk_lambda_expr(&self, lambda_expr: &'ctx ast::LambdaExpr) -> Self::Result {
955955
let func = Arc::new(func_body);
956956
// Capture schema self
957+
let closure_map = self.get_current_closure_map();
958+
957959
let proxy = FunctionCaller::new(
958960
FunctionEvalContext {
959961
node: lambda_expr.clone(),
@@ -966,7 +968,7 @@ impl<'ctx> TypedResultWalker<'ctx> for Evaluator<'ctx> {
966968
value: ctx.value(),
967969
config: ctx.config(),
968970
}),
969-
closure: self.get_current_closure_map(),
971+
closure: closure_map,
970972
level: self.scope_level() + 1,
971973
},
972974
func,
@@ -1478,7 +1480,7 @@ impl<'ctx> Evaluator<'ctx> {
14781480
// Positional arguments
14791481
let is_in_range = i < argument_len;
14801482
if is_in_range {
1481-
let mut arg_value = match args.list_get_option(i as isize) {
1483+
let mut arg_value = match args.list_get(i as isize) {
14821484
Some(v) => v,
14831485
None => self.undefined_value(),
14841486
};
@@ -1489,7 +1491,10 @@ impl<'ctx> Evaluator<'ctx> {
14891491
// Mark positional argument as a local variable
14901492
let name = arg_name.names[0].node.as_str();
14911493
self.add_local_var(name);
1492-
self.store_variable(&arg_name.names[0].node, arg_value);
1494+
// FIX: Use add_variable instead of store_variable for positional arguments
1495+
// store_variable only updates existing variables, but positional parameters
1496+
// don't exist yet, so we need to use add_variable to create them
1497+
self.add_variable(name, arg_value);
14931498
} else {
14941499
break;
14951500
}

crates/evaluator/src/scope.rs

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,7 @@ impl<'ctx> Evaluator<'ctx> {
214214
let msg = format!("pkgpath {} is not found", current_pkgpath);
215215
let scopes = pkg_scopes.get_mut(&current_pkgpath).expect(&msg);
216216
if let Some(last) = scopes.last_mut() {
217-
let variables = &mut last.variables;
218-
variables.insert(name.to_string(), pointer);
217+
last.variables.insert(name.to_string(), pointer);
219218
}
220219
}
221220

@@ -349,6 +348,7 @@ impl<'ctx> Evaluator<'ctx> {
349348
/// Get the variable value named `name` from the scope, return Err when not found
350349
pub fn get_variable(&self, name: &str) -> ValueRef {
351350
let current_pkgpath = self.current_pkgpath();
351+
// Use a more obvious debug marker
352352
self.get_variable_in_pkgpath(name, &current_pkgpath)
353353
}
354354

@@ -519,10 +519,28 @@ impl<'ctx> Evaluator<'ctx> {
519519
} else {
520520
false
521521
};
522-
if index >= last_lambda_scope && !ignore {
523-
var.clone()
522+
// Determine if we should use the current scope value or closure value
523+
// The logic differs for schema methods vs regular lambdas:
524+
// - Schema methods: lambda level may be > execution level, need special handling
525+
// - Regular lambdas: use original logic
526+
let is_schema_method = self.current_method_receiver.borrow().is_some();
527+
if is_schema_method {
528+
// Schema method: prefer current value if variable is in current scope
529+
// (even if it's also in the closure from previous execution)
530+
if index >= INNER_LEVEL && !ignore {
531+
// Variable is in current execution scope, use current value
532+
var.clone()
533+
} else {
534+
// Variable from outer scope or builtin/global, check closure
535+
lambda_ctx.closure.get(name).unwrap_or(var).clone()
536+
}
524537
} else {
525-
lambda_ctx.closure.get(name).unwrap_or(var).clone()
538+
// Regular lambda: use original logic
539+
if index >= last_lambda_scope && !ignore {
540+
var.clone()
541+
} else {
542+
lambda_ctx.closure.get(name).unwrap_or(var).clone()
543+
}
526544
}
527545
} else {
528546
// Not in the lambda, maybe a local variable.
@@ -547,7 +565,9 @@ impl<'ctx> Evaluator<'ctx> {
547565
.as_ref()
548566
.map(|ctx| ctx.closure.clone())
549567
.unwrap_or_default();
550-
// Get variable map including schema in the current scope.
568+
569+
// Capture all variables from scopes >= INNER_LEVEL
570+
// This is the original simple logic that works correctly for nested lambdas
551571
for i in INNER_LEVEL..scopes.len() {
552572
let variables = &scopes
553573
.get(i)
@@ -574,12 +594,25 @@ impl<'ctx> Evaluator<'ctx> {
574594
} else {
575595
self.undefined_value()
576596
};
597+
// Track the receiver for schema method calls
598+
let mut receiver = value.clone();
577599
for i in 0..names.len() - 1 {
578600
let attr = names[i + 1];
579601
if i == 0 && !pkgpath.is_empty() {
580602
value = self.get_variable_in_pkgpath(attr, pkgpath);
603+
receiver = value.clone();
581604
} else {
582-
value = value.load_attr(attr)
605+
value = value.load_attr(attr);
606+
// Fix: If we're loading a lambda method from a schema instance,
607+
// store the receiver so it can be used when the lambda is called
608+
// Check if this is a schema/dict method call
609+
if (receiver.is_schema() || receiver.is_dict())
610+
&& value.is_func()
611+
&& let Some(_proxy) = value.try_get_proxy()
612+
{
613+
// Store the receiver for use when the lambda is called
614+
*self.current_method_receiver.borrow_mut() = Some(receiver.clone());
615+
}
583616
}
584617
}
585618
value
@@ -614,11 +647,9 @@ impl<'ctx> Evaluator<'ctx> {
614647

615648
/// Load global or local value from name.
616649
pub fn load_name(&self, name: &str) -> ValueRef {
617-
match (
618-
self.is_in_schema(),
619-
self.is_in_lambda(),
620-
self.is_local_var(name),
621-
) {
650+
let is_local = self.is_local_var(name);
651+
652+
match (self.is_in_schema(), self.is_in_lambda(), is_local) {
622653
// Get variable from the global lazy scope.
623654
(false, _, false) => {
624655
let variable = self.get_variable(name);

crates/runtime/src/value/api.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2298,11 +2298,11 @@ pub unsafe extern "C-unwind" fn kcl_schema_get_value(
22982298
// This can happen when accessing attributes on a schema instance returned by
22992299
// instances(). In that case, we should return the existing value rather than
23002300
// re-computing it, which would fail because schema parameters are out of scope.
2301-
if let Some(v) = schema.dict_get_value(key) {
2302-
if cal_map.dict_get_value(key).is_some() {
2303-
// This is a computed attribute that already has a value
2304-
return v.into_raw(unsafe { mut_ptr_as_ref(ctx) });
2305-
}
2301+
if let Some(v) = schema.dict_get_value(key)
2302+
&& cal_map.dict_get_value(key).is_some()
2303+
{
2304+
// This is a computed attribute that already has a value
2305+
return v.into_raw(unsafe { mut_ptr_as_ref(ctx) });
23062306
}
23072307
if let Some(attr_code) = cal_map.dict_get_value(key) {
23082308
let now_level = level + 1;
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# Test for lambda functions in schema methods to ensure correct parameter passing
2+
# when called multiple times on different instances
3+
4+
schema Builder:
5+
"""A builder schema with a lambda method that adds items"""
6+
items: [str] = []
7+
8+
add: (str) -> Builder = lambda item: str {
9+
"""Add an item to the builder and return a new builder"""
10+
new_items = items + [item]
11+
Builder{items = new_items}
12+
}
13+
14+
# Test: Chain multiple calls
15+
b1 = Builder{}
16+
b2 = b1.add("first")
17+
b3 = b2.add("second")
18+
b4 = b3.add("third")
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
b1:
2+
items: []
3+
b2:
4+
items:
5+
- first
6+
b3:
7+
items:
8+
- first
9+
b4:
10+
items:
11+
- first
12+
- second
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Test lambda accessing schema attributes
2+
# This tests the fix for the bug where lambda parameters were incorrectly
3+
# captured from closure instead of using the current value
4+
5+
schema Processor:
6+
name: str = "default"
7+
data: [str] = []
8+
9+
process: (str) -> Processor = lambda item: str {
10+
# Lambda should access the current schema's name and data
11+
new_data = data + [item]
12+
Processor{name: name, data: new_data}
13+
}
14+
15+
processWithPrefix: (str) -> Processor = lambda item: str {
16+
# Lambda should access the current schema's name
17+
prefixed = "{name}:{item}"
18+
new_data = data + [prefixed]
19+
Processor{name: name, data: new_data}
20+
}
21+
22+
# Test 1: Default processor
23+
p1 = Processor{}
24+
p2 = p1.process("item1")
25+
p3 = p2.process("item2")
26+
27+
# Test 2: Named processor
28+
n1 = Processor{name: "my_processor"}
29+
n2 = n1.process("first")
30+
n3 = n2.process("second")
31+
32+
# Test 3: Process with prefix
33+
x1 = Processor{name: "test", data: []}
34+
x2 = x1.processWithPrefix("a")
35+
x3 = x2.processWithPrefix("b")
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
p1:
2+
name: default
3+
data: []
4+
p2:
5+
name: default
6+
data:
7+
- item1
8+
p3:
9+
name: default
10+
data:
11+
- item1
12+
n1:
13+
name: my_processor
14+
data: []
15+
n2:
16+
name: my_processor
17+
data:
18+
- first
19+
n3:
20+
name: my_processor
21+
data:
22+
- first
23+
x1:
24+
name: test
25+
data: []
26+
x2:
27+
name: test
28+
data:
29+
- '{name}:{item}'
30+
x3:
31+
name: test
32+
data:
33+
- '{name}:{item}'

0 commit comments

Comments
 (0)