feat: Introducing `fzn-rs`, a better FlatZinc parser that is easier to re-use across projects #238

maartenflippo · 2025-07-06T07:33:06Z

Our current FlatZinc parser implementation works, but it has a few flaws:

It is difficult to support both the original fzn format, as well as the new JSON-based format.
The error messages have very little information. They lack any indication of where the error happened, which can make for difficult debugging.
We write a lot of code to resolve variables, constants, etc. All that code has to be repeated when implementing other tools that consume FlatZinc (e.g. a proof processor or proof checker).

That is why this PR introduces a new FlatZinc parsing crate, which provides the following:

A new AST that is heavily inspired by flatzinc-serde. It improves the AST we obtain from flatzinc mainly because we do not attempt to infer type information of identifiers in constraint arguments.
The AST tracks, for every node, where it originated in the source. This allows for more descriptive error messages.
Derive macros that allow for easy specification of the shape of constraints that are supported by the consumer of the crate. No more resolving constants/variables/arrays. That is all done in generated code. Look at the crate documentation for fzn-rs for an idea of what that looks like.

fzn-rs-derive/src/annotation.rs

fzn-rs/src/lib.rs

fzn-rs/src/ast.rs

fzn-rs/src/fzn/mod.rs

fzn-rs-derive/tests/utils.rs

Review re-requested

fzn-rs-derive/src/lib.rs

fzn-rs/src/ast.rs

Review re-requested

ImkoMarijnissen

Similar error:

thread 'main' panicked at pumpkin-solver/src/bin/pumpkin-solver/flatzinc/compiler/mod.rs:29:61:
handle errors: IncorrectNumberOfArguments { expected: 3, actual: 4, span: Span { start: 7182213, end: 7182263 } }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

maartenflippo · 2025-08-13T21:03:58Z

Similar error:

thread 'main' panicked at pumpkin-solver/src/bin/pumpkin-solver/flatzinc/compiler/mod.rs:29:61:
handle errors: IncorrectNumberOfArguments { expected: 3, actual: 4, span: Span { start: 7182213, end: 7182263 } }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@ImkoMarijnissen what instance is this?

maartenflippo · 2025-09-10T08:29:04Z

I updated this branch to fix. I can now run the last 5 years of the minizinc challenge without getting errors. Sadly we do incur a performance penalty, as the "lex then parse" approach in the parser is not as fast as I would have hoped.

On challenge years 2020-2024, we get the following score:

Main	With fzn-rs
179	163

All in all I believe that, while the impact on score is not great, it is still a step forward. If we agree that the crate itself looks good, then I move to merge this PR and hopefully improve the performance of the flatzinc parser later. The reason for this is that there are many branches that depend on this already (it really makes it so much easier to consume flatzinc). Since the performance change is completely internal to the crate, we can change it (hopefully) without hurting consumers of this crate.

Please re-examine #258 to see how this changes pumpkin-solver.

Review re-requested

EmirDe · 2025-09-10T12:58:40Z

Quick question: when you compare the new version and the old version, specifically on benchmarks where we can find optimal solution, are the results consistent? Just wondering if there could be some problems with the parsing!

maartenflippo · 2025-09-10T14:23:38Z

Yes the solutions are consistent

maartenflippo · 2025-09-11T14:43:58Z

I looked some more into the results, and I cannot really pinpoint where the slowdown comes from. In both configurations, if I run a profiler on instances that take a long time to parse, the main hotspot is NotificationEngine::watch_all, with anywhere from 20-40% of the runtime.

What is also curious is that the instances that show a slowdown in initTime (which is the statistic that measures how long it takes to set up the solver to start solving) does not correlate with flatzinc model size. So large models may be fast in one configuration, and slow in another. This makes sense if we account for the slow IO on the cluster, but it makes it very difficult to track down the issues locally in the profiler.

What I did figure out is that the impact of lexing the source before parsing (which is not done in the main branch currently) is actually negligible. Lexing does not show up in a single one of my 10 profiles. I profiled the 10 instances which took the longest on DelftBLue.

@ImkoMarijnissen could you also have a look at this PR again and give your feedback on it and the apparent slowdown?

EmirDe · 2025-09-12T14:25:50Z

I looked some more into the results, and I cannot really pinpoint where the slowdown comes from. In both configurations, if I run a profiler on instances that take a long time to parse, the main hotspot is NotificationEngine::watch_all, with anywhere from 20-40% of the runtime.

What is also curious is that the instances that show a slowdown in initTime (which is the statistic that measures how long it takes to set up the solver to start solving) does not correlate with flatzinc model size. So large models may be fast in one configuration, and slow in another. This makes sense if we account for the slow IO on the cluster, but it makes it very difficult to track down the issues locally in the profiler.

What I did figure out is that the impact of lexing the source before parsing (which is not done in the main branch currently) is actually negligible. Lexing does not show up in a single one of my 10 profiles. I profiled the 10 instances which took the longest on DelftBLue.

@ImkoMarijnissen could you also have a look at this PR again and give your feedback on it and the apparent slowdown?

Quick questions: when you tests and profiling, do you stop the solver after parsing? Is slow parsing something only on DelftBlue or also locally?

Let us discuss this in the solver meeting!

EmirDe · 2025-10-02T12:50:48Z

Macro can type check? Then from_ast can say that everything is type-checked.
Solve -> SolveItem, SolveToken?
example in the library documentation

…d solve items

…d solve

…he solve item in TypedInstance

…ation

… error

…tion variants

The Rc is not Send, but popular crates like anyhow do expect errors to be Send. Since these are errors, we pay the price to allocate a String instead.

…zn parser" This reverts commit 44cbb24.

ImkoMarijnissen · 2025-10-03T00:11:14Z

fzn-rs-derive/src/constraint.rs

@@ -0,0 +1,107 @@
+use quote::quote;
+
+/// Construct a token stream that initialises a value with name `value_type` and the arguments


This appears to be the exact same documentation as used in annotation.rs, perhaps it could be clarified what the difference is?

ImkoMarijnissen · 2025-10-03T00:14:26Z

fzn-rs/src/lib.rs

+//!
+//! /// The `TypedInstance` is parameterized by the constraint type, as well as any annotations you
+//! /// may need to parse.
+//! type MyInstance = TypedInstance<i64, MyConstraints>;


It's a bit unclear from this documentation what the i64 signifies here

ImkoMarijnissen · 2025-10-03T00:16:34Z

Cargo.toml

@@ -1,5 +1,5 @@
 [workspace]
-members = ["./pumpkin-solver", "./drcp-format", "./pumpkin-solver-py", "./pumpkin-macros", "./drcp-debugger", "./pumpkin-crates/*"]
+members = ["./pumpkin-solver", "./drcp-format", "./pumpkin-solver-py", "./pumpkin-macros", "./drcp-debugger", "./pumpkin-crates/*", "./fzn-rs", "./fzn-rs-derive"]


I think the pipeline is not running the tests for these crates if they are not part of default-members, do all test cases pass for these crates?

ImkoMarijnissen · 2025-10-03T00:17:24Z

fzn-rs/src/lib.rs

+//! fn parse_flatzinc(source: &str) -> MyInstance {
+//!     // First, the source string is parsed into a structured representation.
+//!     //
+//!     // Note: the `fzn_rs::fzn` module is only available with the `fzn` feature enabled.


Is it explained anywhere what this feature is?

maartenflippo requested a review from ImkoMarijnissen July 6, 2025 07:33

maartenflippo force-pushed the feat/improved-fzn-parser branch 2 times, most recently from 6e4bd9e to efde93c Compare July 10, 2025 14:24

ImkoMarijnissen previously requested changes Jul 11, 2025

View reviewed changes

maartenflippo force-pushed the feat/improved-fzn-parser branch from b226ff1 to 0281a3c Compare July 24, 2025 09:53

maartenflippo marked this pull request as ready for review July 25, 2025 13:29

maartenflippo requested a review from ImkoMarijnissen July 25, 2025 13:51

maartenflippo mentioned this pull request Jul 25, 2025

refactor(pumpkin-solver): Use the fzn-rs package instead of flatzinc #258

Open

2 tasks

ImkoMarijnissen previously requested changes Jul 28, 2025

View reviewed changes

fzn-rs-derive/src/lib.rs Show resolved Hide resolved

fzn-rs-derive/src/lib.rs Show resolved Hide resolved

fzn-rs/src/ast.rs Outdated Show resolved Hide resolved

fzn-rs/src/ast.rs Show resolved Hide resolved

ImkoMarijnissen self-requested a review July 28, 2025 10:59

ImkoMarijnissen approved these changes Jul 28, 2025

View reviewed changes

ImkoMarijnissen previously requested changes Jul 28, 2025

View reviewed changes

maartenflippo force-pushed the feat/improved-fzn-parser branch from 945b37e to 438aa3b Compare August 13, 2025 20:16

maartenflippo requested a review from ImkoMarijnissen September 10, 2025 08:29

maartenflippo added 6 commits October 2, 2025 16:21

feat(fzn-rs): Started working on a more ergonomic

0dc4435

feat(fzn-rs): Implement first go at the derive macro

6e313f3

fix(fzn-rs): Don't panic in an error case, but emit a compiler error

b354b53

refactor(fzn-rs): Attach spans to AST in preparation for error messages

02233ea

refactor(fzn-rs): Simplify the code generation code

c38348a

feat(fzn-rs): Improve error messages when parsing instance

a40af1a

maartenflippo added 28 commits October 2, 2025 16:21

feat(fzn-rs): Parse annotations in variables, arrays, constraints, an…

90ebd86

…d solve items

feat(fzn-rs): Ignore predicate declarations in flatzinc

a0ca002

feat(fzn-rs): Allow constraint arguments to be separate structs

6b24dae

docs(fzn-rs): Add example with constraint args as separate struct

c627a51

refactor(fzn-rs): Replace '_' with '-' in crate names

89dd3dc

refactor(fzn-rs): Separate annotations for variables, constraints, an…

e4eef74

…d solve

feat(fzn-rs): Parse annotation arguments in struct

cebb3f7

feat(fzn-rs): Implement RangeList::iter for i64 elements

b7b3a29

feat(fzn-rs): Implement support for i32 as an integer type

32f1011

refactor(fzn-rs): Remove redundent function

717596c

refactor(fzn-rs): Rename VariableArg to VariableExpr, and use it as t…

304a9af

…he solve item in TypedInstance

refactor(fzn-rs): Clean up the implementation into modules + document…

1b00c82

…ation

fix(fzn-rs): Fix numerous compiler issues after previous refactor

e83c6d1

refactor(fzn-rs): Improve documentation of the API

796ffae

refactor(fzn-rs): Remove useless function

195c2a3

fix(fzn-rs): Ignore consequtive lines with comments

68de618

feat(fzn-rs): Implement display for token

019bf10

feat(fzn-rs): Check argument length of constraint and provide span in…

4fdff08

… error

docs(fzn-rs): Correct documentation on Ast and clarify the two annota…

e7e0375

…tion variants

refactor(fzn-rs): Use arrayexpr in annotation arguments

9cf9bb8

chore(fzn-rs): Remove commented code

b1abddd

refactor: Update cargo.lock

a8bdcdf

feat(fzn-rs): Implement parsing of the domain of array elements

8aa7abd

refactor(fzn-rs): Move away from using Rc in errors

39812e0

The Rc is not Send, but popular crates like anyhow do expect errors to be Send. Since these are errors, we pay the price to allocate a String instead.

feat(fzn-rs): Allow access to error values

47e3574

refactor(fzn-rs): Remove the explicit lexing stage from the fzn parser

aac6193

Revert "refactor(fzn-rs): Remove the explicit lexing stage from the f…

8faa08a

…zn parser" This reverts commit 44cbb24.

docs(fzn-rs): Comments and naming of types

1ee9455

maartenflippo force-pushed the feat/improved-fzn-parser branch from cfe2685 to 1ee9455 Compare October 2, 2025 14:38

ImkoMarijnissen reviewed Oct 3, 2025

View reviewed changes

		@@ -0,0 +1,107 @@
		use quote::quote;

		/// Construct a token stream that initialises a value with name `value_type` and the arguments

feat: Introducing fzn-rs, a better FlatZinc parser that is easier to re-use across projects #238

Are you sure you want to change the base?

feat: Introducing fzn-rs, a better FlatZinc parser that is easier to re-use across projects #238

Uh oh!

Conversation

maartenflippo commented Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ImkoMarijnissen left a comment

Choose a reason for hiding this comment

Uh oh!

maartenflippo commented Aug 13, 2025

Uh oh!

maartenflippo commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EmirDe commented Sep 10, 2025

Uh oh!

maartenflippo commented Sep 10, 2025

Uh oh!

maartenflippo commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EmirDe commented Sep 12, 2025

Uh oh!

EmirDe commented Oct 2, 2025

Uh oh!

ImkoMarijnissen Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

ImkoMarijnissen Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

ImkoMarijnissen Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

ImkoMarijnissen Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat: Introducing `fzn-rs`, a better FlatZinc parser that is easier to re-use across projects #238

feat: Introducing `fzn-rs`, a better FlatZinc parser that is easier to re-use across projects #238

maartenflippo commented Jul 6, 2025 •

edited

Loading

maartenflippo commented Sep 10, 2025 •

edited

Loading

maartenflippo commented Sep 11, 2025 •

edited

Loading