Skip to content
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

inherit attributes for generated methods / consts / types #76

Conversation

bachue
Copy link
Contributor

@bachue bachue commented Nov 30, 2021

No description provided.

@LukasKalbertodt
Copy link
Member

Thanks for the PR! Can you briefly describe your motivation for this change? In particular, what kind of attributes would you like to inherit? The change looks good to me and makes sense on first thought, but I'm worried we are missing some attributes that do not make sense to "inherit" in the generated impls.

@bachue
Copy link
Contributor Author

bachue commented Dec 1, 2021

@LukasKalbertodt Ok, in my case, I want to inherit cfg(feature = "xxx") and cfg_attr(feature = "docs", doc(cfg(feature = "xxx"))) attribtues, both of them are used for conditional compilation. Because for my traits, some interfaces are only available when the specified feature is on.

@LukasKalbertodt
Copy link
Member

For cfg, I'm convinced I think. I don't see how this could backfire and I think one always wants to also apply this to the generated items. However, other attributes I'm not sure sure.

You use doc in cfg_attr, but does this really need to apply to the generated item as well? And doc attributes, like doc(hidden) or the simple doc = "comment..." I don't see why they would apply to the generated item as well. Further, we have allow, warn, forbid, deprecated, must_use and tool attributes (e.g. rustfmt::skip). I don't think all of those should always apply to the generated item.

So I don't think I want to accept the PR as is. If you restrict it to cfg attributes, that should be fine, I guess. Also, note that this crate is for the 90% of cases and doesn't aim to work for every possible use case. Because the latter would come at the cost of an increase in complexity.

@bachue bachue force-pushed the features/inherit_attrs_for_generated_methods_const_or_types branch from 1cb7c9c to 8023066 Compare February 12, 2022 13:54
@bachue
Copy link
Contributor Author

bachue commented Feb 12, 2022

@LukasKalbertodt Thanks, PR is updated

@LukasKalbertodt
Copy link
Member

@bachue I'm not really maintaining this crate anymore, I'm sorry. @KodrAus might take this from here!

@KodrAus
Copy link
Member

KodrAus commented May 31, 2022

Thanks for the PR @bachue

It's been a while since I've been through this crate, but this seems reasonable to me. I'll merge this in and take stock of things and look at getting another release out 🙇

@KodrAus KodrAus merged commit 2e6fa94 into auto-impl-rs:master May 31, 2022
@bachue bachue deleted the features/inherit_attrs_for_generated_methods_const_or_types branch May 31, 2022 23:52
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