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

Regression: this #[deprecated] annotation has no effect #79304

Closed
kpcyrd opened this issue Nov 22, 2020 · 7 comments
Closed

Regression: this #[deprecated] annotation has no effect #79304

kpcyrd opened this issue Nov 22, 2020 · 7 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-stability Area: `#[stable]`, `#[unstable]` etc. regression-from-stable-to-beta Performance or correctness regression from stable to beta.

Comments

@kpcyrd
Copy link

kpcyrd commented Nov 22, 2020

Code

I tried this code:

struct X {}

#[deprecated(since="0.2.2", note="Use the `AsRef` or `AsMut` implementation instead")]
impl ::std::ops::Index<::std::ops::RangeFull> for X {
    type Output = [u8];
    fn index(&self, _index: ::std::ops::RangeFull) -> &[u8] {
        todo!();
    }
}

fn main() {
    println!("Hello, world!");
}

This code currently compiles on stable rust, but stops working on nightly and beta. The example code is a reduced case from sodiumoxide which is currently broken on nightly and beta.

% cargo check      
    Checking regress v0.1.0 (/<redacted>)
    Finished dev [unoptimized + debuginfo] target(s) in 0.62s
% cargo +beta check
    Checking regress v0.1.0 (/<redacted>)
error: this `#[deprecated]` annotation has no effect
 --> src/main.rs:3:1
  |
3 | #[deprecated(since="0.2.2", note="Use the `AsRef` or `AsMut` implementation instead")]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the unnecessary deprecation attribute
  |
  = note: `#[deny(useless_deprecated)]` on by default

error: aborting due to previous error

error: could not compile `regress`

To learn more, run the command again with --verbose.

Version it worked on

It most recently worked on: 1.48.0

Version with regression

rustc --version --verbose:

rustc 1.49.0-beta.1 (21dea46d8 2020-11-18)
binary: rustc
commit-hash: 21dea46d8347c8b4117c5567949703f0fbb51649
commit-date: 2020-11-18
host: x86_64-unknown-linux-gnu
release: 1.49.0-beta.1
@rustbot rustbot added regression-from-stable-to-beta Performance or correctness regression from stable to beta. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 22, 2020
@jonas-schievink
Copy link
Contributor

I believe this is intended behavior added in #78626. It shouldn't cause any issues when using sodiumoxide as a dependency, since all lints are silenced in that case.

cc @m-ou-se

@petrochenkov
Copy link
Contributor

petrochenkov commented Nov 22, 2020

If #[deprecated] is displayed by rustdoc (like #[stable]/#[unstable]), then #78626 is a regression.
Even if stability attributes are not checked on impls now (but they may be checked in the future), they are still useful as documentation.

@jyn514
Copy link
Member

jyn514 commented Nov 22, 2020

This should be a warning, not an error I think.

@jyn514 jyn514 added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-stability Area: `#[stable]`, `#[unstable]` etc. labels Nov 22, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 22, 2020

@petrochenkov deprecated is not shown by rustdoc on impls: image

So the attribute does have no effect.

@camelid
Copy link
Member

camelid commented Nov 22, 2020

Should this be closed as intended behavior then?

bors bot added a commit to sodiumoxide/sodiumoxide that referenced this issue Nov 23, 2020
443: Remove useless deprecation annotation r=kpcyrd a=kpcyrd

This should fix our broken CI.

The change in rustc was caused by rust-lang/rust#78626, further discussion in rust-lang/rust#79304.

Co-authored-by: kpcyrd <git@rxv.cc>
bors bot added a commit to sodiumoxide/sodiumoxide that referenced this issue Nov 23, 2020
443: Remove useless deprecation annotation r=kpp a=kpcyrd

This should fix our broken CI.

The change in rustc was caused by rust-lang/rust#78626, further discussion in rust-lang/rust#79304.

Co-authored-by: kpcyrd <git@rxv.cc>
@kpcyrd
Copy link
Author

kpcyrd commented Nov 24, 2020

Since this is silenced for dependencies and we've already fixed this on our master branch this issue is resolved for me (thanks for the help!). I'm not closing it myself in case you still want to discuss this until the 1.49.0 release, but feel free to do so.

@m-ou-se
Copy link
Member

m-ou-se commented Nov 24, 2020

@kpcyrd Thanks for the report and the update.

As mentioned above, this is "working as intended" in the sense that this was expected outcome of #78626. That does not mean it's necessarily the best behaviour. Maybe a warning would make more sense here, depending on how much trouble this change causes.

this issue is resolved for me

Okay, closing this for now then.

Please re-open the issue or leave a comment if you (anybody) think something needs to be done here.

@m-ou-se m-ou-se closed this as completed Nov 24, 2020
@jyn514 jyn514 removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Feb 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-stability Area: `#[stable]`, `#[unstable]` etc. regression-from-stable-to-beta Performance or correctness regression from stable to beta.
Projects
None yet
Development

No branches or pull requests

7 participants