Skip to content

Cranelift: simplify DataValue::eq - #13953

Closed
dev-er1 wants to merge 1 commit into
bytecodealliance:mainfrom
dev-er1:simplify-datavalue-partialeq
Closed

Cranelift: simplify DataValue::eq#13953
dev-er1 wants to merge 1 commit into
bytecodealliance:mainfrom
dev-er1:simplify-datavalue-partialeq

Conversation

@dev-er1

@dev-er1 dev-er1 commented Jul 24, 2026

Copy link
Copy Markdown

This simplifies the PartialEq implementation for DataValue by replacing the repeated ... => false match arms with a single wildcard arm.

No functional changes intended.

@dev-er1
dev-er1 requested a review from a team as a code owner July 24, 2026 12:56
@dev-er1
dev-er1 requested review from cfallin and removed request for a team July 24, 2026 12:56

@cfallin cfallin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! Happy to merge once the rustfmt issue is fixed.

@alexcrichton

Copy link
Copy Markdown
Member

One possible note here is that this is an intentional idiom you'll find throughout Wasmtime in a number of places. The intentional part is that if a new variant is added the compiler's exhaustive-match checks flag the match as "gotta add more arms here". With a catch-all like proposed in this PR adding a new variant will get no such warnings and it'd be some bug later own down the road to force adding the match. Personally I find that more valuable than removing lines, but that's also a subjective call

@cfallin

cfallin commented Jul 24, 2026

Copy link
Copy Markdown
Member

Ah, yeah, sorry, I should've registered that here -- brain saw wildcard -> wildcard and I missed that. Thanks; @dev-er1 we'll close this as above.

@cfallin cfallin closed this Jul 24, 2026
@dev-er1
dev-er1 deleted the simplify-datavalue-partialeq branch July 24, 2026 14:55
@dev-er1

dev-er1 commented Jul 24, 2026

Copy link
Copy Markdown
Author

In any case, thanks for the review.

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.

3 participants