Skip to content

EXPERIMENTAL: Typestate buffer builder#362

Open
chrysn wants to merge 4 commits intolake-rs:mainfrom
chrysn-pull-requests:typestate-buffer-builder
Open

EXPERIMENTAL: Typestate buffer builder#362
chrysn wants to merge 4 commits intolake-rs:mainfrom
chrysn-pull-requests:typestate-buffer-builder

Conversation

@chrysn
Copy link
Member

@chrysn chrysn commented May 23, 2025

As a result of #361, the pushes to the buffers in early message building (first few bytes, where we know things will fit) are very .unwrap()py. That is still OK there because it's an improvement over the explicit setting into content (that could also panic) and manual length setting (that could go wrong), but it's ugly.

Either kind of panic should be easy to optimize away, and for hax to prove, but there are still downsides:

  • It's a visible panic that the developer has to convince themselves that it's OK.
  • hax only runs on a particular size setting; can't cover all size choices in CI.

This approach is different in that it uses const generics (and occasionally typenum where const generics don't cut it, but for most parts that are user visible it's const generics) to perform a const assertion that the parts that are supposed to be infallible really are. (There's still a panic in there later, but if we trust the const generics we might even make that unsafe_unreachable, and at any rate, it's just one place to reason about).

This PR is currently in "initially run this by CI" stage, and not marked as a draft because I'd like feedback on the approach and whether we should follow this.

Small bycatch is that some buffers should really be tad bigger than the messages, and that we keep mixing up the buffer types as they're just type aliases and all the same.

@chrysn
Copy link
Member Author

chrysn commented May 27, 2025

The branches this was based on have changed. If you review now, only 66cc89e matters (and I'll rebase it on top of what then will be main if we want to go that route)

@chrysn chrysn force-pushed the typestate-buffer-builder branch 2 times, most recently from b99d855 to 2e06363 Compare June 5, 2025 13:36
@chrysn
Copy link
Member Author

chrysn commented Jun 5, 2025

Rebased (and thus blocked by) #359. That already has the idiom of taking our const { blocks and hiding them from hax. All the other const'nesses in this PR are hopefully understood by hax.

@chrysn chrysn force-pushed the typestate-buffer-builder branch from 2e06363 to 77be3c4 Compare June 10, 2025 09:40
@chrysn
Copy link
Member Author

chrysn commented Jun 16, 2025

This conflicts with #379 (hax checks) -- not just in the simple merge-conflict style, but because hax is unhappy with a lot of things in typenum, and can't do const generic expressions (which is an unstable feature anyway).

We might still offer the API under cfg(not(hax)), of offer an alternative (runtime panicking, but hax checkable) drop-in replacement.

@chrysn chrysn force-pushed the typestate-buffer-builder branch from 77be3c4 to 2e9b37e Compare June 16, 2025 10:40
chrysn added 3 commits June 16, 2025 16:38
This allows operations that are known to be infallible by construction
of the minimal message size to be checked at buidl time.
@chrysn chrysn force-pushed the typestate-buffer-builder branch from 2e9b37e to fe7e776 Compare June 16, 2025 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant