Skip to content

Commit d2634bf

Browse files
authored
Avoid backtracking when parsing function call expressions (#7944)
Fixes #7866 This PR fixes two instances of unnecessary backtracking - one in the single unlabelled argument case of parsing function calls and one in pipeline/non-pipeline expression distinction which combined led to exponential blowup when parsing nested function calls. For the particular example in #7866 this changes non-termination in a reasonable time into practically instant. Signed-off-by: Nick Cameron <[email protected]>
1 parent a58e27d commit d2634bf

File tree

4 files changed

+121
-110
lines changed

4 files changed

+121
-110
lines changed

rust/kcl-lib/benches/compiler_benchmark_criterion.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ pub fn bench_parse(c: &mut Criterion) {
1010
("math", MATH_PROGRAM),
1111
("mike_stress_test", MIKE_STRESS_TEST_PROGRAM),
1212
("koch snowflake", LSYSTEM_KOCH_SNOWFLAKE_PROGRAM),
13+
("nested function calls", NESTED_FN_CALLS),
1314
] {
1415
c.bench_function(&format!("parse_{name}"), move |b| {
1516
b.iter(move || {
@@ -74,3 +75,20 @@ const MATH_PROGRAM: &str = include_str!("../e2e/executor/inputs/math.kcl");
7475
const MEDIUM_SKETCH: &str = include_str!("../e2e/executor/inputs/medium_sketch.kcl");
7576
const MIKE_STRESS_TEST_PROGRAM: &str = include_str!("../tests/mike_stress_test/input.kcl");
7677
const LSYSTEM_KOCH_SNOWFLAKE_PROGRAM: &str = include_str!("../e2e/executor/inputs/lsystem.kcl");
78+
// Previously had O(c^n) behaviour due to excessive backtracking in the parser, https://github.com/KittyCAD/modeling-app/issues/7866
79+
const NESTED_FN_CALLS: &str = "extrude(
80+
close(
81+
xLine(
82+
yLine(
83+
xLine(
84+
yLine(
85+
startProfile(at)
86+
),
87+
length = 10
88+
),
89+
length = 10
90+
),
91+
length = 10
92+
)
93+
)
94+
)";

rust/kcl-lib/src/parsing/ast/types/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ impl<T> Node<T> {
125125
}
126126
}
127127

128-
pub fn boxed(inner: T, start: usize, end: usize, module_id: ModuleId) -> BoxNode<T> {
128+
pub fn boxed(start: usize, end: usize, module_id: ModuleId, inner: T) -> BoxNode<T> {
129129
Box::new(Node {
130130
inner,
131131
start,

rust/kcl-lib/src/parsing/math.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,17 @@ fn evaluate(rpn: Vec<BinaryExpressionToken>) -> Result<Node<BinaryExpression>, C
2828
let Some(left) = operand_stack.pop() else {
2929
return Err(e);
3030
};
31-
let start = left.start();
32-
let end = right.end();
33-
let module_id = left.module_id();
3431

3532
BinaryPart::BinaryExpression(Node::boxed(
33+
left.start(),
34+
right.end(),
35+
left.module_id(),
3636
BinaryExpression {
3737
operator,
3838
left,
3939
right,
4040
digest: None,
4141
},
42-
start,
43-
end,
44-
module_id,
4542
))
4643
}
4744
BinaryExpressionToken::Operand(o) => o,
@@ -162,15 +159,15 @@ mod tests {
162159
lit(2).into(),
163160
BinaryOperator::Div.into(),
164161
BinaryPart::BinaryExpression(Node::boxed(
162+
0,
163+
0,
164+
ModuleId::default(),
165165
BinaryExpression {
166166
operator: BinaryOperator::Sub,
167167
left: lit(1),
168168
right: lit(5),
169169
digest: None,
170170
},
171-
0,
172-
0,
173-
ModuleId::default(),
174171
))
175172
.into(),
176173
BinaryOperator::Pow.into(),

0 commit comments

Comments
 (0)