Skip to content

Commit bcb6ed2

Browse files
jtranjtran
authored
Fix unlabeled arg execution order (#8184)
* Fix unlabeled arg execution order * Fix callee execution order * Update output
1 parent c13deac commit bcb6ed2

File tree

5 files changed

+92
-85
lines changed

5 files changed

+92
-85
lines changed

‎rust/kcl-lib/src/execution/fn_call.rs

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,32 @@ impl Node<CallExpressionKw> {
197197
let fn_name = &self.callee;
198198
let callsite: SourceRange = self.into();
199199

200+
// Clone the function so that we can use a mutable reference to
201+
// exec_state.
202+
let func = fn_name.get_result(exec_state, ctx).await?.clone();
203+
204+
let Some(fn_src) = func.as_function() else {
205+
return Err(KclError::new_semantic(KclErrorDetails::new(
206+
"cannot call this because it isn't a function".to_string(),
207+
vec![callsite],
208+
)));
209+
};
210+
211+
// Evaluate the unlabeled first param, if any exists.
212+
let unlabeled = if let Some(ref arg_expr) = self.unlabeled {
213+
let source_range = SourceRange::from(arg_expr.clone());
214+
let metadata = Metadata { source_range };
215+
let value = ctx
216+
.execute_expr(arg_expr, exec_state, &metadata, &[], StatementKind::Expression)
217+
.await?;
218+
219+
let label = arg_expr.ident_name().map(str::to_owned);
220+
221+
Some((label, Arg::new(value, source_range)))
222+
} else {
223+
None
224+
};
225+
200226
// Build a hashmap from argument labels to the final evaluated values.
201227
let mut fn_args = IndexMap::with_capacity(self.arguments.len());
202228
let mut errors = Vec::new();
@@ -221,21 +247,6 @@ impl Node<CallExpressionKw> {
221247
}
222248
}
223249

224-
// Evaluate the unlabeled first param, if any exists.
225-
let unlabeled = if let Some(ref arg_expr) = self.unlabeled {
226-
let source_range = SourceRange::from(arg_expr.clone());
227-
let metadata = Metadata { source_range };
228-
let value = ctx
229-
.execute_expr(arg_expr, exec_state, &metadata, &[], StatementKind::Expression)
230-
.await?;
231-
232-
let label = arg_expr.ident_name().map(str::to_owned);
233-
234-
Some((label, Arg::new(value, source_range)))
235-
} else {
236-
None
237-
};
238-
239250
let args = Args::new_kw(
240251
KwArgs {
241252
unlabeled,
@@ -247,17 +258,6 @@ impl Node<CallExpressionKw> {
247258
exec_state.pipe_value().map(|v| Arg::new(v.clone(), callsite)),
248259
);
249260

250-
// Clone the function so that we can use a mutable reference to
251-
// exec_state.
252-
let func = fn_name.get_result(exec_state, ctx).await?.clone();
253-
254-
let Some(fn_src) = func.as_function() else {
255-
return Err(KclError::new_semantic(KclErrorDetails::new(
256-
"cannot call this because it isn't a function".to_string(),
257-
vec![callsite],
258-
)));
259-
};
260-
261261
let return_value = fn_src
262262
.call_kw(Some(fn_name.to_string()), exec_state, ctx, args, callsite)
263263
.await

‎rust/kcl-lib/tests/var_ref_in_own_def/ast.snap

Lines changed: 60 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -68,24 +68,58 @@ description: Result of parsing var_ref_in_own_def.kcl
6868
"arguments": [
6969
{
7070
"type": "LabeledArg",
71-
"label": null,
71+
"label": {
72+
"commentStart": 0,
73+
"end": 0,
74+
"moduleId": 0,
75+
"name": "at",
76+
"start": 0,
77+
"type": "Identifier"
78+
},
7279
"arg": {
73-
"abs_path": false,
7480
"commentStart": 0,
81+
"elements": [
82+
{
83+
"commentStart": 0,
84+
"end": 0,
85+
"moduleId": 0,
86+
"raw": "20",
87+
"start": 0,
88+
"type": "Literal",
89+
"type": "Literal",
90+
"value": {
91+
"value": 20.0,
92+
"suffix": "None"
93+
}
94+
},
95+
{
96+
"argument": {
97+
"commentStart": 0,
98+
"end": 0,
99+
"moduleId": 0,
100+
"raw": "20",
101+
"start": 0,
102+
"type": "Literal",
103+
"type": "Literal",
104+
"value": {
105+
"value": 20.0,
106+
"suffix": "None"
107+
}
108+
},
109+
"commentStart": 0,
110+
"end": 0,
111+
"moduleId": 0,
112+
"operator": "-",
113+
"start": 0,
114+
"type": "UnaryExpression",
115+
"type": "UnaryExpression"
116+
}
117+
],
75118
"end": 0,
76119
"moduleId": 0,
77-
"name": {
78-
"commentStart": 0,
79-
"end": 0,
80-
"moduleId": 0,
81-
"name": "sketch001",
82-
"start": 0,
83-
"type": "Identifier"
84-
},
85-
"path": [],
86120
"start": 0,
87-
"type": "Name",
88-
"type": "Name"
121+
"type": "ArrayExpression",
122+
"type": "ArrayExpression"
89123
}
90124
}
91125
],
@@ -98,7 +132,7 @@ description: Result of parsing var_ref_in_own_def.kcl
98132
"commentStart": 0,
99133
"end": 0,
100134
"moduleId": 0,
101-
"name": "startProfileAt",
135+
"name": "startProfile",
102136
"start": 0,
103137
"type": "Identifier"
104138
},
@@ -113,49 +147,22 @@ description: Result of parsing var_ref_in_own_def.kcl
113147
"type": "CallExpressionKw",
114148
"type": "CallExpressionKw",
115149
"unlabeled": {
150+
"abs_path": false,
116151
"commentStart": 0,
117-
"elements": [
118-
{
119-
"commentStart": 0,
120-
"end": 0,
121-
"moduleId": 0,
122-
"raw": "20",
123-
"start": 0,
124-
"type": "Literal",
125-
"type": "Literal",
126-
"value": {
127-
"value": 20.0,
128-
"suffix": "None"
129-
}
130-
},
131-
{
132-
"argument": {
133-
"commentStart": 0,
134-
"end": 0,
135-
"moduleId": 0,
136-
"raw": "20",
137-
"start": 0,
138-
"type": "Literal",
139-
"type": "Literal",
140-
"value": {
141-
"value": 20.0,
142-
"suffix": "None"
143-
}
144-
},
145-
"commentStart": 0,
146-
"end": 0,
147-
"moduleId": 0,
148-
"operator": "-",
149-
"start": 0,
150-
"type": "UnaryExpression",
151-
"type": "UnaryExpression"
152-
}
153-
],
154152
"end": 0,
155153
"moduleId": 0,
154+
"name": {
155+
"commentStart": 0,
156+
"end": 0,
157+
"moduleId": 0,
158+
"name": "sketch001",
159+
"start": 0,
160+
"type": "Identifier"
161+
},
162+
"path": [],
156163
"start": 0,
157-
"type": "ArrayExpression",
158-
"type": "ArrayExpression"
164+
"type": "Name",
165+
"type": "Name"
159166
}
160167
}
161168
],

‎rust/kcl-lib/tests/var_ref_in_own_def/execution_error.snap

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ KCL UndefinedValue error
66

77
× undefined value: You can't use `sketch001` because you're currently trying
88
│ to define it. Use a different variable here instead.
9-
╭─[3:32]
9+
╭─[3:19]
1010
2 │ sketch001 = startSketchOn(XY)
11-
3 │ |> startProfileAt([20, -20], sketch001)
12-
· ────┬────
13-
· ╰── tests/var_ref_in_own_def/input.kcl
11+
3 │ |> startProfile(sketch001, at = [20, -20])
12+
· ────┬────
13+
· ╰── tests/var_ref_in_own_def/input.kcl
1414
╰────

‎rust/kcl-lib/tests/var_ref_in_own_def/input.kcl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
// This won't work, because `sketch001` is being referenced in its own definition.
22
sketch001 = startSketchOn(XY)
3-
|> startProfileAt([20, -20], sketch001)
3+
|> startProfile(sketch001, at = [20, -20])

‎rust/kcl-lib/tests/var_ref_in_own_def/unparsed.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@ description: Result of unparsing var_ref_in_own_def.kcl
44
---
55
// This won't work, because `sketch001` is being referenced in its own definition.
66
sketch001 = startSketchOn(XY)
7-
|> startProfileAt([20, -20], sketch001)
7+
|> startProfile(sketch001, at = [20, -20])

0 commit comments

Comments
 (0)