prost-build: add Config::eq_when_safe to gate auto Eq derives (#30) - #1423
Draft
MukundaKatta wants to merge 1 commit into
Draft
prost-build: add Config::eq_when_safe to gate auto Eq derives (#30)#1423MukundaKatta wants to merge 1 commit into
MukundaKatta wants to merge 1 commit into
Conversation
Implements issue tokio-rs#30 by exposing an explicit configuration knob, `Config::eq_when_safe(bool)`, that controls whether prost emits `#[derive(Eq, Hash)]` on generated message and oneof types when the type is provably free of floating point fields (transitively). The auto-derive logic itself was already in place, but it ran unconditionally. With this change, callers that prefer to opt out (for example to add their own manual `Eq` impls, or to keep generated code minimal) can now do so by calling `config.eq_when_safe(false)` before compiling. The default is `true`, matching prost's pre-existing behavior, so existing users are not affected. Other notes: - `Context::can_message_derive_eq` now memoizes results per fully-qualified message name. Cycles in the message graph (which are already broken at the field-typing layer via `is_nested`) are handled by leaving an in-progress sentinel in the cache. - Added tests: * prost-build unit tests assert that `eq_when_safe(false)` strips `Eq`/`Hash` from the generated derives, and that the default keeps them. * tests/src/derive_eq.{proto,rs} compile a mix of int-only, float-bearing, and composed messages and rely on a `TestEqIsImplemented` bound to confirm that only the safe types satisfy `Eq` while float-bearing types remain only `PartialEq`.
caspermeijn
requested changes
Jul 3, 2026
caspermeijn
left a comment
caspermeijn
left a comment
Member
There was a problem hiding this comment.
What I don't understand is: Why do you want to disable Eq when it is safe? The trait is useful, so why remove it?
| /// // Opt out of automatic `Eq` derives. | ||
| /// config.eq_when_safe(false); | ||
| /// ``` | ||
| pub fn eq_when_safe(&mut self, enable: bool) -> &mut Self { |
Member
There was a problem hiding this comment.
If we want this config API, it should use accept paths, like https://docs.rs/prost-build/latest/prost_build/struct.Config.html#method.btree_map
That way it can be enabled or disabled on specific messages
| @@ -0,0 +1,36 @@ | |||
| syntax = "proto3"; | |||
Member
There was a problem hiding this comment.
I don't understand this diff. The master branch already contains the file: tests/src/derive_eq.proto
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #30. Adds an explicit
Config::eq_when_safe(bool)builder so callers can opt out of automaticEq/Hashderives. Defaulttruepreserves existing behavior.What's added
prost-build/src/config.rs— neweq_when_safefield + builder method.prost-build/src/context.rs— gatescan_message_derive_eq/can_field_derive_eqon the flag, withRefCell<HashMap>memoization and a cycle-break sentinel for recursive types.prost-build/src/lib.rs— unit tests for both flag states.tests/src/derive_eq.protoandtests/src/derive_eq.rswith compile-timeTestEqIsImplemented: Eqbound across all-int, float, double, composed-safe, and composed-unsafe messages.Test plan
cargo fmt --all -- --checkclean.cargo clippy -p prost-build --all-targets --all-features -- -D warningsclean.prost,prost-build,prost-derive,prost-types,tests.