#[deprecated] lint doesn't trigger when overriding deprecated method

[m-ou-se]

This doesn't give any warnings, even though std::error::Error::description is deprecated:

impl std::error::Error for E {
    fn description(&self) -> &str {
        ":)"
    }
}

It seems like this is a deliberate decision:

rust/compiler/rustc_passes/src/stability.rs

Lines 763 to 764 in 3dcb616

	// Pass `None` to skip deprecation warnings.
	self.tcx.check_stability(def_id, None, impl_item_ref.span, None);

Substituting that first None for Some(impl_item_ref.id.hir_id()) makes it work.

Doing so results in the lint triggering in exactly one place in std:

rust/library/std/src/error.rs

Lines 605 to 609 in 3dcb616

	impl Error for Infallible {
	fn description(&self) -> &str {
	match *self {}
	}
	}

That's because in all other cases, we have #[allow(deprecated)] there:

rust/library/std/src/error.rs

Lines 597 to 602 in 3dcb616

	impl Error for string::FromUtf16Error {
	#[allow(deprecated)]
	fn description(&self) -> &str {
	"invalid utf-16"
	}
	}

Which is suprising, as that lint doesn't tigger there, making that #[allow] unnecessary.

It seems as if it used to be necessary in the past, but now no longer is.

Did #[deprecated] trigger on overrides in the past? Was that behavior changed on purpose? Or is it a bug that it doesn't do that anymore?

[m-ou-se]

cc @rust-lang/libs-api, should the #[deprecated] lint trigger here or not?

[eddyb]

(reposting #98991 (comment) here as well)

I'm not sure what happened here, but the lack of tests for this is worrying (and may explain how this happened, if it was accidental).

EDIT: much more comprehensive analysis posted in #98991 (comment).

[thomaseizinger]

I just ran into this. I'd like to guide my users away from implementing one trait function to new ones in a backwards-compatible way. My thinking was:

• Add new trait functions with default implementations that call the old one.
• Deprecate the old function.

I was expecting to have users (the trait provides a plugin-like functionality, so has many external users outside the crate) to see a deprecation warning that they should no longer implement this function but a new one instead.

[hkBst]

@rustbot ping libs-api

(see #98990 (comment))

[rustbot]

Error: Parsing ping command in comment failed: ...'g libs-api' | error: expected end of command at >| ' (see http'...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[rustbot]

Error: Only Rust team members can ping teams.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[joshtriplett]

Sorry for the long delay in this being talked about!

We talked about this in today's @rust-lang/lang meeting, and those present (myself, @traviscross, and (representing libs-api) @Amanieu) were all generally in favor. We did feel this needs an FCP, though.

Please do provide a PR implementing this, mark it I-lang-nominated, and we'll FCP it.