Skip to content

Add Fork derive macro for enum forking and unzipping#184

Open
kishore08-07 wants to merge 3 commits intoopen-rmf:mainfrom
kishore08-07:enum
Open

Add Fork derive macro for enum forking and unzipping#184
kishore08-07 wants to merge 3 commits intoopen-rmf:mainfrom
kishore08-07:enum

Conversation

@kishore08-07
Copy link

@kishore08-07 kishore08-07 commented Mar 17, 2026

New feature implementation

Implemented feature

This PR implements #144 , It introduces a procedural macro that enables forking and unzipping for any user-defined enum type, extending the existing support for Result and Option.

Implementation description

  • Implements a Fork derive macro to auto-generate Unzippable for enums.
  • Exposes ForkUnzip operation and related APIs for downstream use.
  • Adds runtime tests for enum forking/unzipping.
  • Includes concise inline documentation and rustdoc for new APIs and macro logic.
  • All changes validated with passing tests.

GenAI Use

We follow OSRA's policy on GenAI tools

  • I used a GenAI tool in this PR.
  • I did not use GenAI

Generated-by:
GPT-4.1

Signed-off-by: kishore08-07 <kishorebsm8@gmail.com>
Copy link
Contributor

@mxgrey mxgrey left a comment

Choose a reason for hiding this comment

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

Some broad thoughts on this: This PR does a good job of capturing what jotted out in #144 regarding a Fork macro that implements the Unzippable trait.

However this doesn't quite cover the original intent of allowing any enum to support the semantics that we have for Result and Option. I think this PR is a good first step in that direction, but I'll probably need to follow up this PR with a more concrete requirements outline for what the Fork or Forkable trait would look like.

One important distinction I expect between Unzippable and Forkable is that Unzippable provides the Outputs as an anonymous sequence, whereas Forkable should refer to specific names. This is especially relevant for Diagrams where users will want to refer to enum variants by name instead of by index numbers, which is what unzip currently uses.

Maybe the best long-term approach is to remove Unzippable entirely and just have a Forkable trait which uses Identifier to label each of its outputs. We would then phase out the unzip operation from diagrams in favor of a more general fork operation that can be used for tuples, Results, Options, all enums, and even structs (by splitting the fields).

I think most of this is beyond the scope of this PR, but I wanted to lay out my thoughts more clearly so we understand the opportunities available for follow-up work.

The one thing that I do think should be changed before merging this is to not allow named fields in a variant of an enum that we're forking. See my inline comment for more detail.

.connect(scope.terminate);
builder
.chain(named)
.map_block(|(left, right)| left - right)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm reluctant to turn named fields of a variant into an anonymous tuple. One reason that named fields get used in a variant is because the names of the fields are important, not the ordering of the fields. If a user changes the order of the named fields in their variant but keep the names the same, then they will expect that nothing is negatively impacted. However if we pack the field values into a tuple like this then changing the ordering of the field will change the meaning of this tuple.

I would rather the Fork macro emit a compilation error if it detects that one of the variants in the enum has more than one named field. If it's just a single named field then I don't mind treating it like the single value variant.

Copy link
Author

Choose a reason for hiding this comment

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

@mxgrey I’ve addressed the inline comment by emitting a compile-time error for variants with multiple named fields, while allowing single-field named variants.Let me know if any further changes are needed.

…ate tests

Signed-off-by: kishore08-07 <kishorebsm8@gmail.com>
@kishore08-07
Copy link
Author

Some broad thoughts on this: This PR does a good job of capturing what jotted out in #144 regarding a Fork macro that implements the Unzippable trait.

However this doesn't quite cover the original intent of allowing any enum to support the semantics that we have for Result and Option. I think this PR is a good first step in that direction, but I'll probably need to follow up this PR with a more concrete requirements outline for what the Fork or Forkable trait would look like.

One important distinction I expect between Unzippable and Forkable is that Unzippable provides the Outputs as an anonymous sequence, whereas Forkable should refer to specific names. This is especially relevant for Diagrams where users will want to refer to enum variants by name instead of by index numbers, which is what unzip currently uses.

Maybe the best long-term approach is to remove Unzippable entirely and just have a Forkable trait which uses Identifier to label each of its outputs. We would then phase out the unzip operation from diagrams in favor of a more general fork operation that can be used for tuples, Results, Options, all enums, and even structs (by splitting the fields).

I think most of this is beyond the scope of this PR, but I wanted to lay out my thoughts more clearly so we understand the opportunities available for follow-up work.

The one thing that I do think should be changed before merging this is to not allow named fields in a variant of an enum that we're forking. See my inline comment for more detail.

Thanks for the detailed thoughts , this really helps clarify the bigger picture. I agree that a Forkable approach with named outputs would be a better long-term direction.

For now, I’ve kept this PR scoped as a first step with Unzippable, and I’ve addressed the review comment.

Looking forward to the follow-up discussion on shaping the Forkable design.

@kishore08-07 kishore08-07 requested a review from mxgrey March 20, 2026 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

2 participants