allow(deprecated) is too coarse-grained, should take a path

[RalfJung]

https://github.com/rust-lang/rust/commit/007d87f1719e2fcf2ff36aef8b6dc866fa276386 added allow(deprecated) in a few places because mem::uninitialized is still used. Unfortunately, this allows all deprecated items, so by the time someone gets around to fix that, other uses of deprecated items could have crept in.

It would be much better if that could be allow(deprecated(mem::uninitialized)) or so, to only allow the one method without also allowing everything else.

The same applies to deprecated_in_future.

I think I saw @Mark-Simulacrum ask for this somewhere recently? Or was it someone else?

[Centril]

Unfortunately, this allows all deprecated items, so by the time someone gets around to fix that, other uses of deprecated items could have crept in.

In most of these cases the allow(deprecated) lint check directive was applied on a specific statement or it was applied inside small tests. Only in the case of sgx and cloudabi was #![allow(deprecated)] applied inside a larger whole module. This is due to the highly specialized nature of the libstd codebase in terms of platform support.

I would like to see stronger justification for changing the grammar of allow(...) and friends, what other circumstances other than deprecated this would apply to, and whether this is a sufficiently large problem outside this repository.

[petrochenkov]

ask for this somewhere recently? Or was it someone else?

Me? https://github.com/rust-lang/rust/pull/61879#issuecomment-508413106

In most of these cases the allow(deprecated) lint check directive was applied on a specific statement

Deprecation lints reported early (e.g. in https://github.com/rust-lang/rust/pull/62042) don't currently have have the necessary infra to suppress them locally and have to be allowed at the crate level.

[petrochenkov]

and whether this is a sufficiently large problem outside this repository.

I don't think this problem is specific to rustc. Some uses of deprecated features are hard to get rid of, or maintainer may lack resources for any non-trivial work, and it may take weeks or months to address and they have to be allowed in the meantime. At the same time, you want to be aware of other new deprecations, that may be trivial to fix.

deprecated(mem::uninitialized)

I'd prefer a feature name, or something like that. With paths you immediately get the problem of resolving them.

[Centril]

Deprecation lints reported early (e.g. in #62042) don't currently have have the necessary infra to suppress them locally and have to be allowed at the crate level.

But in these specific cases these deprecation lints are not reported early, right? (Otherwise they would have had no effect in the #[allow(deprecated)] cases in the referenced commit.)

While from a complexity perspective (compiler + user facing) an extension to allow(...) is not too large, on first inspection it seems to me that what seems like a compiler bug should be fixed before considering language changes.

[petrochenkov]

But in these specific cases these deprecation lints are not reported early, right? (Otherwise they would have had no effect in the #[allow(deprecated)] cases in the referenced commit.)

Hm, the only allows I see in that PR are at the crate level (namely, in doc tests - each doc test is built as a separate crate).

[Centril]

Hm, the only allows I see in that PR are at the crate level (namely, in doc tests - each doc test is built as a separate crate).

That's strange, the commit aforementioned has e.g.:

                #[allow(deprecated)]
                let mut loses_info: llvm::Bool = ::std::mem::uninitialized();

Some uses of deprecated features are hard to get rid of, or maintainer may lack resources for any non-trivial work, and it may take weeks or months to address and they have to be allowed in the meantime. At the same time, you want to be aware of other new deprecations, that may be trivial to fix.

No doubt, but using #[allow(deprecated)] on the function or statement level seems like a decently an adequate way of achieving this even if it requires some copy pasting. It's not as fast as #![allow(deprecated(path::to::thing))] but it at least encourages you to fix some of the easy ones sooner.

I'd prefer a feature name, or something like that. With paths you immediately get the problem of resolving them.

I'm against any mention of feature names in the stable language for the same reason I was opposed to #[cfg(rust_feature = "async_await)] as compared to #[cfg(accessible(::path::to::thing))]. The feature names are neither stable nor intuitive and are hastily made up throw-away symbols invented when RFCs or PRs are written.

[petrochenkov]

That's strange, the commit aforementioned has e.g.:

Ah, we are talking about different PRs, I was talking about https://github.com/rust-lang/rust/pull/62042.

[RalfJung]

@petrochenkov ah right, it was you. :) Sorry.

In most of these cases the allow(deprecated) lint check directive was applied on a specific statement or it was applied inside small tests.

Another instance of (the equivalent of) a crate-level allow:

https://github.com/rust-lang/rust/blob/baab1914ec9a9742776a8147780947b48fddf54d/src/libstd/lib.rs#L208

If we had finer-grained control, that could have been

#![warn(deprecated_in_future)]
#![allow(deprecated_in_future(mem::uninitialized)]

Generally, when updating rustc (even for your own crate) I feel it can be hard to find all the right places for where to locally allow(deprecated) in a well-targeted way that minimizes accidentally allowing more. Just being able to allow all deprecated methods, to me, feels just as if we only were able to allow all warnings. There's a reason we have allow(some_lint) even though people could just very locally add #[allow] in the statements where they do something that's linted against. The same reason applies to allow(deprecated(path)).

[Centril]

@RalfJung In the interim, deprecated_in_future is rustc specific so we can allow this in an unstable fashion just for rustc internals without an RFC.

[RalfJung]

Not sure what you mean? That lint can also be used by user crates.

But user crates can indeed not mark items as "deprecated in the future". Still, I don't know what it is you are proposing.

[Centril]

Still, I don't know what it is you are proposing.

I'm not really actively proposing it, but if you want to... we can add a rustc internal unstable feature gate under which you can do allow(deprecated_in_future(mem::uninitialized)) .