-
Notifications
You must be signed in to change notification settings - Fork 1.6k
RFC: Allow #[ignore(Trait)]
on field to ignore it when deriving Trait
#3869
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This can and should probably be an ACP, not an RFC. CC rust-lang/libs-team#334 from >1 year ago which proposed something very similar for Similarly, starting to interpret your proposed (We got away with the addition of |
Published it as an RFC that is what was suggested in this comment Validation: The following doesn't compile due to a deny-by-default future incompatibility lint which is going to be promoted to a hard error as part of this RFC: use derive as ignore;
struct Foo {
// syntax that will gain meaning because of this RFC
#[ignore()]
hello: ()
} This does not compile because of ambiguity. Whereas if use derive as ignore;
struct Foo {
#[ignore]
hello: ()
} So then giving this meaning should be fine: use derive as ignore;
struct Foo {
#[ignore(Foo, Bar)]
hello: ()
} struct Foo {
#[ignore(Foo, Bar)]
hello: ()
} Because it would not have compiled (without turning off the deny-by-default lint which says that we will promote it into a hard error in the future) This currently does compile, with a warn-by-default lint. And in the RFC I'll clarify that this will continue to compile, and its meaning will not be changed. struct Foo {
#[ignore]
hello: ()
} I'm not super familiar with terminology, but this isn't a new helper attribute - it's the same attribute Summary: If any other name was chosen, then it could be potentially breaking. But because If there are any possible breaking changes that I've missed, I would appreciate an example |
Ah, I completely forgot that Still, it would "need to become" a helper attribute but that would clash with the built-in attribute, or rather the built-in attribute However, I'm not an expert in those resolution rules, so maybe it's a non-issue, so I'm happy to be corrected by someone more knowledgeable. I'm wondering if we can properly discern helper and built-in attribute |
CC https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/namespacing.20macro.20attrs.20to.20reduce.20conflicts.20with.20new.20adds/ (from Jul–Sep '25) (don't quite remember if this is only tangentially related or indeed fully) |
Co-authored-by: Jacob Lifshay <programmerjake@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really elegant solution with transforming #[ignore(Xyz)]
-> #[ignore]
!
…not support `ignore`
Does this feature's benefits outweigh implementation complexity?
This could be replaced with manual implementations of std::hash:Hash, Eq and PartialEq on the id field. This works if there is only 1 or 2 fields used for the trait implementation. If there is a single field that needs ignoring it can be newtyped and given manual trait implementations. For example
now OpaqueRng can be used in any struct without needing this RFC. This covers the case where there is only a few fields that should be ignored. Are there any motivating cases in the middle where neither approach works well? |
I think it does because of how common it is to want to ignore some fields, that imo outweighs all the additional compiler complexity. |
Currently when you |
If, while deriving |
I don't want to start bikeshedding war but for the attribute-syntax shouldn't we move more in the direction of |
This is not an issue for If there are some parts you think could be clarified please let me know! |
(Sidenote: If you think the discussion is getting out of hand and prefer to have it inside some MR line comments to not clutter the main comment section, please let me know and we can move it there) You are of course right that it already reserved and so technically no breakage should occur. But the fact that it is used for testing is actually a good reason to not add overload to its meaning! Also, I really think more discussion should be placed on potential future-proofing. The single I think the section "How about #[debug(ignore)]" does not provide enough justification for the proposed syntax right now. For starter, it doesn't really explain why we shouldn't adopt the already-existing "api" of Let me also disagree with the claim "people have to learn each crate's own way of skipping fields" - even with Going further, similarly to my musings about future derive possibilities, proc macros might have much richer API for handling a given field than just "ignoring" it and trying to split this between this might turn out to be a worse design than "per-crate attribute namespace". For example I definitely would not want to see a mix of Given this I still think attribute format in the form of I however see the point in the current fragmentation of ecosystem where some crates use "skip" vs. some "ignore" and might use different syntax. More standardization in this direction should be made but maybe it should not be forced upon you by the language as |
When someone adds a derive to the list of
If you support the
Compare the 2 overloads of the
I think overloading Name Calling this anything-goes configuration attribute What if we chose a different name, e.g.
I would suggest to use helper attributes, i.e. I think this discussion can be simplified into a single question: Is it worth to have a dedicated attribute The RFC answers this, but as a summary:
Thank you for taking the time to bring this up. |
This RFC proposes that the
#[ignore]
attribute can now be applied to fields.Its purpose is to tell derive macros to ignore the field when generating code.
For the above struct
User
, derivesPartialEq
andHash
will ignore thename
andage
fileds.Code like this is generated:
Rendered