-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Hide deprecation warnings inside derive expansions #58994
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
I'd prefer to fix this in some other way.
I think the usual approach "do not report warnings in macros from other crates" should work here, derive macros are always "from other crates" (there is a chance that built-in derives may need to be special-cased though). |
Alternatively, |
I don't think we should do this; deprecation may be part of an internal refactoring within a large application or crate. This happens e.g. in rustc with methods sometimes. |
The method would still get linted about if used from an impl for another type. It's not a crate wide "allow", but just an allow for impls. |
I understand that the method would still be linted, but my point was not specific to methods, it applies equally to impl blocks, e.g. #[deprecated]
struct Foo;
impl Foo { ... } // Not necessarily in the same file or part of a large file. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@petrochenkov would you be alright with just backporting the first commit, but putting the rest into nightly? @Centril I removed the conroversial "don't lint on impls of deprecated types" change. The only functional changes now are
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@oli-obk That's pretty good; the change to pattern matching seems like an UX improvement. Tho... for the purposes of beta-backport it seems a bit large. ^^ |
Yeah, seems ok as a temporary measure for backport. |
☔ The latest upstream changes (presumably #59044) made this pull request unmergeable. Please resolve the merge conflicts. |
4f6bcdd
to
2151ffb
Compare
@oli-obk |
Oh, right. Labels. Thanks for the reminder, I'll set those from now on. |
Sorry to be pushy, but is there an ETA on this? We have some work which is currently blocked on this being fixed (namely, we can't commit some code that exercises this bug). Happy to help if there's anything I can do to help move it along! |
I'll get to reviewing it this weekend or maybe tomorrow. |
Awesome thanks @petrochenkov ! Sorry again about being pushy. |
☔ The latest upstream changes (presumably #58140) made this pull request unmergeable. Please resolve the merge conflicts. |
@bors p=8 |
I should have reviewed this sooner, sorry. To summarize, this PR does three things:
|
Performing deprecation checking in the way it's performed now certainly has problems, multiple of which are shared with stability checking - import deprecations are ignored, intermediate path segments are not checked (#15702), macros are not checked for deprecation at all (#49912). The solution to all those problems is to organize stability/deprecation checking exactly like name privacy checking is organized now. The move of deprecation checking to a late lint pass doesn't make it closer to that goal, so if possible I'd prefer not to do that move and leave the code as is. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors retry arm segfault |
Hide deprecation warnings inside derive expansions Fixes #58822
☀️ Test successful - checks-travis, status-appveyor |
discussed in T-compiler meeting. accepting for beta-backport. |
[beta] Rollup backports Cherry-picked: * #58021: Fix fallout from #57667 * #59599: Updated RELEASES.md for 1.34.0 * #59587: Remove #[doc(hidden)] from Error::type_id * #58994: Hide deprecation warnings inside derive expansions * #58015: Expand docs for `TryFrom` and `TryInto`. * #59770: ci: pin android emulator to 28.0.23 * #59704: ci: Update FreeBSD tarball downloads * #59257: Update CI configuration for building Redox libraries * #59724: Function arguments should never get promoted r? @ghost
[beta] Rollup backports Cherry-picked: * #58021: Fix fallout from #57667 * #59599: Updated RELEASES.md for 1.34.0 * #59587: Remove #[doc(hidden)] from Error::type_id * #58994: Hide deprecation warnings inside derive expansions * #58015: Expand docs for `TryFrom` and `TryInto`. * #59770: ci: pin android emulator to 28.0.23 * #59704: ci: Update FreeBSD tarball downloads * #59257: Update CI configuration for building Redox libraries * #59724: Function arguments should never get promoted * #59499: Fix broken download link in the armhf-gnu image * #58330: Add rustdoc JS non-std tests * #58848: Prevent cache issues on version updates r? @ghost
Fixes #58822