Since bincode is unmaintained use wincode instead. by ratmice · Pull Request #646 · softdevteam/grmtools

ratmice · 2026-07-16T04:28:42Z

This is the final patch in the series, this should fix #616. Serialization with our complex bounds is quite a bit less straight forward with wincode than it was with bincode. Because the Configuration is parameterized in the serialization bounds.
Thus we end up having to put bounds on multiple serialization configurations in order to serialize data with that configuration.
At least that was the only way I could figure out how to implement it.

This doesn't provide any CTParserBuilder mechanisms for configuring the encoding used, so it currently always defaults to the VarInt encoding, but the plumbing is all there to do so.

This is part of a series of patches to various crates moving from the bincode crate to the wincode crate.
I believe the reverse order of dependencies is:

packedvec
sparsevec
vob
grmtools

I'm currently leaving how we want to deal with semver up in the air. Since these do change trait impls around,
It may be that as we release these we need to fiddle with the version numbers in Cargo.toml too.

ratmice · 2026-07-16T04:35:25Z

-                )
+                // We have to call reconstitute like this because the config parameter takes a trait
+                // which uses const generics. Thus the two config parameters here are not actually of the same type.
+                match __ENCODING_CONFIG {


Since in #631 this was calling this function in hot loop,
I should try and put this match statement in the closure body I think.

Should be fixed in d74ca12

ratmice · 2026-07-16T05:02:23Z

This was tested with local versions of all the crates using the following Cargo.toml

diff --git a/Cargo.toml b/Cargo.toml
index c1f3ce15..662a5ae8 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -48,3 +48,8 @@ proc-macro2 = "1.0"
 prettyplease = "0.2.31"
 syn = "2.0"
 yaml-rust2 = "0.10.1"
+
+[patch.crates-io]
+vob = {path = "../vob", version = "3.0.4"}
+sparsevec = {path = "../sparsevec", version = "0.2.2"}
+packedvec = {path = "../packedvec", version = "1.2"}

ratmice · 2026-07-16T05:50:20Z

+        + Hash
+        + PrimInt
+        + SchemaWrite<FixIntConfig, Src = LexerTypesT::StorageT>
+        + SchemaWrite<VarIntConfig, Src = LexerTypesT::StorageT>


It is a bit unfortunate, but as far as I can tell it doesn't appear that we can just do something like:

LexerTypesT::StorageT: SchemaWrite<Config, Src = LexerTypesT::StorageT> + ...; Config: for<c: wincode::config::Config>, c,

Because higher ranked trait bounds are limited to lifetimes.
Config here is also not dyn compatible because of the associated types and consts.
So if there are other wincode configurations we do want to try encoding with afaik we have to add a bounds
here for each of those.

ratmice · 2026-07-16T06:39:52Z

Working on the follow up commit that configures encoding between variable, and fixed...

ratmice

Still needs some work to prepare it for follow up patches.

ratmice · 2026-07-16T06:44:59Z

            visibility: Visibility::Private,
            rust_edition: RustEdition::Rust2021,
            inspect_rt: None,
+            encoding_config: EncodingConfiguration::VariableSizedInteger,


Should be None at this point like yacckind and recoverer, above.
Otherwise we can't differentiate between the default value, and user having set it.

I went ahead and fixed this in f0da9ba

Which contains the whole configuration scheme, I did some looking at the generated sources,
and the variable in encoding files are both about half the size of the fixed int encodings,
but the fixed int encodings should should load faster.

If we don't want that, parts of the first patch can be simplified (Like the multiple Schema bounds on different configurations)

ltratt · 2026-07-16T08:34:13Z

@ratmice Let me handle vob and sparsevec to save you the effort,

ratmice · 2026-07-16T08:48:32Z

Woops, think I might have already pushed to my branch, but feel free.

ratmice · 2026-07-16T09:45:35Z

🤞 other than version bumping, I think this one is ready for being looked at whenever you've got time.

ltratt · 2026-07-16T09:46:29Z

I'm working my way through. I must admit that I didn't anticipate wincode having quite such an impact! Still, this is definitely the right thing to do.

ltratt · 2026-07-16T09:56:41Z

I have a few minor comments and one bikeshed: do we think it's obvious what "encoding_config" will be to users? I'm somewhat neutral on this: I did wonder if "serialisation_format" (ok, ok, I could live with a "z" in there :)) might be clearer, but I'm not sure.

ltratt · 2026-07-16T11:01:07Z

I think all of the necessary dependencies are now on crates.io.

ratmice · 2026-07-16T11:59:07Z

I went with SerialisationFormat in 2822302 so at least we're consistently british spelling!

ratmice · 2026-07-16T12:13:25Z

 }

+impl<T> Value<T> {
+    pub fn primary_location(&self) -> &T {


This is a pretty awkward API, the thing going on is that we've parameterized between
logical locations (command line arguments in nimbleparse, builder parameters in CTParserBuilder, and source locations read from %grmtools sections).

In the first two cases we have Value<Span> while in the second we have Value<Location>.

We could return a Vec<T> where T: Clone here, but in all the Value<Location> areas, this is going to be extremely unhelpful, and in the Span case, PrimaryLocation will still return at least one relevant location.

So we may drop some information regarding source locations when dealing with Span,
but we also avoid emitting redundant locations, when all those locations are like CommandLine

So, it's kind of like we're either going to have this information loss, where we drop "secondary" locations,
or we're going to have a bunch of redundancy. So I think picking one of the locations is ok.

Since we've got a major version bump coming up, we could consider an API change if it would help?

I hadn't given it a whole lot of thought, In that the whole mixing of spans and logical error locations is kind of new territory for me, so I've kind of been winging it. One thing we could consider is the way that the AST Value<T> is currently an enum which has the pair of e.g. String, and Span, but also String, and Location.

We could consider making it a

enum Setting { Num(u64, usize) Flag(bool, usize), Array(Vec<Setting>, usize, usize) } struct ValueRepr<T> { value: Value, locations: Vec<T> }

With that the Location::CommandLine and Location::Other("From<...>") could just push one location on the locations stack, and we'd use the usize indexes to point to that one location.

With spans, each of these could refer to different locations on the locations stack.

I think one of the questions is whether we could get the lrpar/cttests/grmtools_section.test to emit that structure.
Because we test that the AST structure is exactly the same for the handwritten parser, and the generated parser in the testsuite. It's hard for me to imagine how this structure representation could be emitted from the generated parser's rules. I'll try and give it a think overnight.

Hmm, perhaps the current muddle is the best we can do right now.

Well, I did think of one more place that could be cleaned up in e8a5dab, Edit: the other thing to note is that this is all #[doc(hidden)] too.

Good point!

ratmice · 2026-07-16T13:01:01Z

Also let me know if you want me to split this one back into two separate PRs.
I just didn't want to get into a situation where some choice made in the first patch meant we had to make some incompatible change to move forward on making the serialization_fromat configurable.

I'm pretty sure though now that isn't the case.

ltratt · 2026-07-16T15:09:58Z

I went with SerialisationFormat in 2822302 so at least we're consistently british spelling!

Sorry :)

ltratt · 2026-07-16T15:13:14Z

I'm fine with this as one PR.

ltratt · 2026-07-17T07:55:46Z

I think this is ready for squashing?

Since bincode is unmaintained use wincode instead.

1ea0efc

ratmice commented Jul 16, 2026

View reviewed changes

Move block into closure/out of hot path

d74ca12

ratmice commented Jul 16, 2026

View reviewed changes

Rename bincode features to wincode

5d73cc7

Add builder and grmtools section configuration of EncodingConfiguration

f0da9ba

ltratt reviewed Jul 16, 2026

View reviewed changes

Comment thread lrlex/src/lib/ctbuilder.rs

Comment thread lrpar/src/lib/ctbuilder.rs

Comment thread lrpar/src/lib/ctbuilder.rs Outdated

Comment thread lrpar/src/lib/ctbuilder.rs

ratmice added 3 commits July 16, 2026 04:39

Clean up TryFrom impls, improve docs

af75298

Rename EncodingConfig to SerialisationFormat, etc

2822302

Bump dependency versions

29166e4

ratmice commented Jul 16, 2026

View reviewed changes

Comment thread cfgrammar/src/lib/header.rs Outdated

Make error messages more accurate

b8d30c1

Use primary_location() for RecoveryKind

e8a5dab

Uh oh!

Conversation

ratmice commented Jul 16, 2026

Uh oh!

ratmice Jul 16, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

ratmice commented Jul 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ratmice commented Jul 16, 2026

Uh oh!

ratmice left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ltratt commented Jul 16, 2026

Uh oh!

ratmice commented Jul 16, 2026

Uh oh!

ratmice commented Jul 16, 2026

Uh oh!

ltratt commented Jul 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ltratt commented Jul 16, 2026

Uh oh!

ltratt commented Jul 16, 2026

Uh oh!

ratmice commented Jul 16, 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!

ratmice Jul 16, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

ratmice Jul 17, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ratmice commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ltratt commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ltratt commented Jul 16, 2026

Uh oh!

ltratt commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ratmice Jul 16, 2026 •

edited

Loading

ratmice Jul 16, 2026 •

edited

Loading

ratmice Jul 17, 2026 •

edited

Loading

ratmice commented Jul 16, 2026 •

edited

Loading

ltratt commented Jul 16, 2026 •

edited

Loading