Move the math expression parser from Pest to Chumsky and add more features by urisinger · Pull Request #2685 · GraphiteEditor/Graphite

urisinger · 2025-06-03T15:00:29Z

Part of #2026

Switches to the chumsky parser for performance, remove Unit support until #586 is resolved.

Improves parser performance by about 40%, while also reducing compile times and improving readability.

Keavon · 2025-06-20T08:02:14Z

I have rebased this branch to remove merge commits, so you'll need to git reset --hard to its new ref here on origin.

RegenJacob · 2025-10-05T21:31:03Z

How far is this?

I might be interested in implementing some math expressions from #2026
I figured this PR would be a blocking factor before I could do that.

urisinger · 2025-10-06T09:26:52Z

How far is this?

I might be interested in implementing some math expressions from #2026
I figured this PR would be a blocking factor before I could do that.

Its ready for review, its just not being merged becuase without variadics of some form this node is pretty useless, creating the custom error message for the node is also a difficult task.

cubic-dev-ai

8 issues found across 14 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="libraries/math-parser/src/executer.rs">

<violation number="1" location="libraries/math-parser/src/executer.rs:28">
P2: Binary-operator failures are misreported as function-call `TypeError`, conflating distinct error kinds and producing misleading diagnostics.</violation>

<violation number="2" location="libraries/math-parser/src/executer.rs:47">
P2: Conditional truthiness treats `NaN` numeric results as true, allowing invalid conditions to execute the `if` branch.</violation>
</file>

<file name="frontend/wasm/src/editor_api.rs">

<violation number="1" location="frontend/wasm/src/editor_api.rs:931">
P2: Parser error diagnostics are dropped from logs before the API collapses failures to `None`, reducing debuggability of parse regressions.</violation>
</file>

<file name="libraries/math-parser/src/lexer.rs">

<violation number="1" location="libraries/math-parser/src/lexer.rs:173">
P2: Unchecked numeric accumulation and exponent casting can overflow for long literals/exponents, causing panics in debug builds or incorrect numeric values in release builds.</violation>
</file>

<file name="libraries/math-parser/src/value.rs">

<violation number="1" location="libraries/math-parser/src/value.rs:170">
P1: Factorial uses an unbounded linear loop over `n`, enabling very large inputs to cause extreme CPU consumption even after `f64` result has already overflowed.</violation>
</file>

<file name="node-graph/gmath-nodes/src/lib.rs">

<violation number="1" location="node-graph/gmath-nodes/src/lib.rs:474">
P1: GCD/LCM nodes accept `i32` but `binary_gcd` assumes non‑negative values. For negative inputs, the shift/subtract loop can cycle forever (e.g., -2 and 4), hanging evaluation or producing wrong results. Normalize to absolute values or remove `i32` support before calling `binary_gcd`.</violation>
</file>

<file name="libraries/math-parser/src/constants.rs">

<violation number="1" location="libraries/math-parser/src/constants.rs:336">
P1: `root` uses `powf(1/n)` for real inputs, causing odd roots of negative reals (e.g. `root(-8, 3)`) to return `NaN` instead of the expected real value.</violation>

<violation number="2" location="libraries/math-parser/src/constants.rs:524">
P1: `gcd`/`lcm` use unchecked float→`i64` conversion and integer arithmetic, allowing non-finite/huge inputs to produce invalid results or overflow paths.</violation>
</file>

Since this is your first cubic review, here's how it works:

cubic automatically reviews your code and comments on bugs and improvements
Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
Ask questions if you need clarification on any suggestion

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-03-16T21:41:23Z

+					for k in 1..=n {
+						acc *= k as f64;
+					}


P1: Factorial uses an unbounded linear loop over n, enabling very large inputs to cause extreme CPU consumption even after f64 result has already overflowed.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At libraries/math-parser/src/value.rs, line 170: <comment>Factorial uses an unbounded linear loop over `n`, enabling very large inputs to cause extreme CPU consumption even after `f64` result has already overflowed.</comment> <file context> @@ -107,15 +155,37 @@ impl Number { + } + let n = truncated as u64; + let mut acc = 1.0_f64; + for k in 1..=n { + acc *= k as f64; + } </file context>

Suggested change

for k in 1..=n {

acc *= k as f64;

}

if n > 170 {

return Number::Real(f64::INFINITY);

}

for k in 1..=n {

acc *= k as f64;

}

This is valid.

cubic-dev-ai · 2026-03-16T21:41:23Z

+fn greatest_common_divisor<T: num_traits::int::PrimInt + std::ops::ShrAssign<i32> + std::ops::SubAssign>(
+	_: impl Ctx,
+	/// One of the two numbers for which the GCD will be calculated.
+	#[implementations(u32, u64, i32)]


P1: GCD/LCM nodes accept i32 but binary_gcd assumes non‑negative values. For negative inputs, the shift/subtract loop can cycle forever (e.g., -2 and 4), hanging evaluation or producing wrong results. Normalize to absolute values or remove i32 support before calling binary_gcd.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/gmath-nodes/src/lib.rs, line 474: <comment>GCD/LCM nodes accept `i32` but `binary_gcd` assumes non‑negative values. For negative inputs, the shift/subtract loop can cycle forever (e.g., -2 and 4), hanging evaluation or producing wrong results. Normalize to absolute values or remove `i32` support before calling `binary_gcd`.</comment> <file context> @@ -0,0 +1,780 @@ +fn greatest_common_divisor<T: num_traits::int::PrimInt + std::ops::ShrAssign<i32> + std::ops::SubAssign>( + _: impl Ctx, + /// One of the two numbers for which the GCD will be calculated. + #[implementations(u32, u64, i32)] + value: T, + /// The other of the two numbers for which the GCD will be calculated. </file context>

cubic-dev-ai · 2026-03-16T21:41:23Z

@@ -2,13 +2,21 @@ use crate::value::{Number, Value};
 use lazy_static::lazy_static;


P1: gcd/lcm use unchecked float→i64 conversion and integer arithmetic, allowing non-finite/huge inputs to produce invalid results or overflow paths.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At libraries/math-parser/src/constants.rs, line 524: <comment>`gcd`/`lcm` use unchecked float→`i64` conversion and integer arithmetic, allowing non-finite/huge inputs to produce invalid results or overflow paths.</comment> <file context> @@ -107,12 +148,486 @@ lazy_static! { + if x == 0 && y == 0 { + return Some(Value::Number(Number::Real(0.0))); + } + x = x.abs(); + y = y.abs(); + while y != 0 { </file context>

cubic-dev-ai · 2026-03-16T21:41:23Z

+			"root",
+			Box::new(|values| match values {
+				[Value::Number(Number::Real(x)), Value::Number(Number::Real(n))] => {
+					Some(Value::Number(Number::Real(x.powf(1.0 / *n))))


P1: root uses powf(1/n) for real inputs, causing odd roots of negative reals (e.g. root(-8, 3)) to return NaN instead of the expected real value.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At libraries/math-parser/src/constants.rs, line 336: <comment>`root` uses `powf(1/n)` for real inputs, causing odd roots of negative reals (e.g. `root(-8, 3)`) to return `NaN` instead of the expected real value.</comment> <file context> @@ -107,12 +148,486 @@ lazy_static! { + "root", + Box::new(|values| match values { + [Value::Number(Number::Real(x)), Value::Number(Number::Real(n))] => { + Some(Value::Number(Number::Real(x.powf(1.0 / *n)))) + } + [Value::Number(Number::Complex(x)), Value::Number(Number::Real(n))] => { </file context>

Yes this should handle with cbrt() and for odd numbers separately(sign-flip).

cubic-dev-ai · 2026-03-16T21:41:23Z

+		let mut v = 0u64;
+		let mut digits = 0;
+		while let Some(d) = self.peek().and_then(|c| c.to_digit(10)) {
+			v = v * 10 + d as u64;


P2: Unchecked numeric accumulation and exponent casting can overflow for long literals/exponents, causing panics in debug builds or incorrect numeric values in release builds.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At libraries/math-parser/src/lexer.rs, line 173: <comment>Unchecked numeric accumulation and exponent casting can overflow for long literals/exponents, causing panics in debug builds or incorrect numeric values in release builds.</comment> <file context> @@ -0,0 +1,372 @@ + let mut v = 0u64; + let mut digits = 0; + while let Some(d) = self.peek().and_then(|c| c.to_digit(10)) { + v = v * 10 + d as u64; + digits += 1; + self.bump(); </file context>

Annonnymmousss · 2026-04-17T23:05:51Z

All the AI reviews are mostly valid.

Annonnymmousss · 2026-04-17T23:22:06Z

 impl Node {
-	pub fn try_parse_from_str(s: &str) -> Result<(Node, Unit), ParseError> {
-		let pairs = ExprParser::parse(Rule::program, s).map_err(Box::new)?;
-		let (node, metadata) = parse_expr(pairs)?;


Since, this migration remove the unit return value, We have to change it in the graph evaluation logic else it will break compilation.

Annonnymmousss · 2026-04-18T00:25:49Z

+		map.insert(
+			"sqrt",
+			Box::new(|values| match values {
+				[Value::Number(Number::Real(real))] => Some(Value::Number(Number::Real(real.sqrt()))),
+				[Value::Number(Number::Complex(complex))] => Some(Value::Number(Number::Complex(complex.sqrt()))),
+				_ => None,
+			}),
+		);


Annonnymmousss · 2026-04-18T00:34:37Z

+					BinaryOp::And => {
+						let l = lhs != 0.0;
+						let r = rhs != 0.0;
+						if l && r { 1.0 } else { 0.0 }
+					}
+					BinaryOp::Or => {
+						let l = lhs != 0.0;
+						let r = rhs != 0.0;
+						if l || r { 1.0 } else { 0.0 }
+					}


The NaN issue can occur here too.

Annonnymmousss · 2026-04-18T00:42:12Z

Not a good practice. It will suppress warnings.

Annonnymmousss · 2026-04-18T00:48:34Z

+		for diag in &self.diagnostics {
+			term::emit(&mut writer.lock(), &config, &self.file, diag).unwrap();
+		}
+		writer.flush();


This will cause warning.

Annonnymmousss · 2026-04-18T00:51:43Z

There are some unused imports too.

…r, adding functions, comparisons, logic, and conditionals

…arser, and clean up the math-parser rewrite

Keavon changed the title ~~Add if statments to math parser and switch to chumsky parser~~ Add if statements to math parser and switch to Chumsky parser Jun 5, 2025

Keavon force-pushed the master branch from 9fe2410 to 3c1cdb2 Compare June 7, 2025 20:46

Keavon changed the title ~~Add if statements to math parser and switch to Chumsky parser~~ Move the math expression parser from Pest to Chumsky and add more features Jun 9, 2025

Keavon marked this pull request as draft June 14, 2025 22:14

Keavon force-pushed the master branch 2 times, most recently from 34dced3 to 5cacab2 Compare June 20, 2025 06:47

Keavon force-pushed the Math branch from 50be352 to 71b96e8 Compare June 20, 2025 08:00

Keavon force-pushed the master branch 2 times, most recently from 316a737 to 532e913 Compare June 28, 2025 13:25

Keavon force-pushed the master branch 4 times, most recently from ec51271 to e025103 Compare July 10, 2025 05:48

urisinger force-pushed the Math branch from 55ce9c9 to 6cc0745 Compare July 15, 2025 12:20

Keavon force-pushed the master branch from 3abc227 to 97978c2 Compare August 21, 2025 17:09

TrueDoctor force-pushed the master branch from 4351bac to ffc7427 Compare September 26, 2025 12:30

Keavon force-pushed the master branch from f00fe63 to 123108c Compare December 22, 2025 03:33

timon-schelling force-pushed the master branch from 124235a to a42cad8 Compare January 6, 2026 03:38

Keavon force-pushed the master branch 4 times, most recently from 3697b7d to 90533e6 Compare March 11, 2026 12:15

Keavon marked this pull request as ready for review March 16, 2026 21:32

cubic-dev-ai Bot reviewed Mar 16, 2026

View reviewed changes

Keavon force-pushed the master branch 3 times, most recently from e58c1de to df8001f Compare March 17, 2026 00:10

Keavon force-pushed the master branch 4 times, most recently from 9b97ab7 to 2e842cb Compare March 19, 2026 11:00

Annonnymmousss reviewed Apr 17, 2026

View reviewed changes

Annonnymmousss reviewed Apr 18, 2026

View reviewed changes

Comment thread libraries/math-parser/src/lib.rs Outdated

Annonnymmousss Apr 18, 2026

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a good practice. It will suppress warnings.

Annonnymmousss reviewed Apr 18, 2026

View reviewed changes

Keavon force-pushed the master branch 2 times, most recently from f07c79b to 76938eb Compare April 29, 2026 12:16

Keavon force-pushed the master branch 2 times, most recently from 4b7a823 to 847b8e9 Compare May 17, 2026 14:37

timon-schelling force-pushed the master branch 2 times, most recently from 15fcaac to d5f0140 Compare May 17, 2026 15:37

timon-schelling force-pushed the master branch from ac6c8a5 to 51427be Compare May 31, 2026 20:32

urisinger and others added 2 commits July 15, 2026 19:22

Rewrite the math-parser library using a chumsky-based lexer and parse…

5ccb4f6

…r, adding functions, comparisons, logic, and conditionals

Fix parsing regressions, make parsing 2.5x faster than the old pest p…

b1c49fe

…arser, and clean up the math-parser rewrite

Keavon force-pushed the Math branch 2 times, most recently from c2f0c11 to b1c49fe Compare July 16, 2026 02:34

		@@ -2,13 +2,21 @@ use crate::value::{Number, Value};
		use lazy_static::lazy_static;

Uh oh!

Uh oh!

Conversation

urisinger commented Jun 3, 2025

Uh oh!

Keavon commented Jun 20, 2025

Uh oh!

RegenJacob commented Oct 5, 2025

Uh oh!

urisinger commented Oct 6, 2025

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Annonnymmousss commented Apr 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Annonnymmousss commented Apr 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cubic-dev-ai Bot left a comment •

edited

Loading