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

[RalfJung]

007d87f 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? #61879 (comment)

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

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.

[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 #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:

rust/src/libstd/lib.rs

Line 208 in baab191

//#![warn(deprecated_in_future)] // FIXME: std still has quite a few uses of `mem::uninitialized`

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)) .

[peter-lyons-kehl]

1. If having a path in #[allow(deprecated...) or #![allow(deprecated...) is difficult/tricky, how about substring/regex-matching on a note from #[deprecated(note ="...")] and/or on the whole deprecation warning message (as it comes from rustc)?

2. If extending allow is difficult, how about a new attribute, like allow_for or similar?

3. "stronger justification": Concentrated use of deprecated/likely-to-be-deprecated-later/unstable/internal-like libraries (especially depending on nightly features, but not always so), especially at the top level (module level). For example: impl of many traits, type aliases for many const generic instances of the same type, many (module-level) constants using the same deprecated/unstable type/const function...

   The consumer has already agreed to deprecation/unstability - so why repeat this agreement all over? Yes, the consumer, or anyone inspecting/reusing/auditing the consumer crate may want to track/list the extent/number of such unstable imports/use code locations, but she/he may easily do so - just comment out/temporarily remove the allow in question (switchable with a feature and cfg_attr).

   Yes, the consumer may not notice that such an allow would automatically hide potential problems in any new code added later on - but that's her/his tradeoff against high maintenance otherwise (that is, as-is) that means more code, most likely copy-and-paste, which also invites human error.

   So we have human errors on both sides of the tradeoff - how about we let the consumer choose, rather than making the choice for her/him.

[peter-lyons-kehl]

The existing #[allow(deprecated) is as fine-grained as we apply it. The problem is not with granularity, but with possibility (or not) to match/filter only the deprecation(s) that we choose to accept.

Sprinkling the current #[allow(deprecated)] (in a realistic/practical way) around the same file is not equivalent to having path/note/substring/regex-based allow. Most "productive" code makes a new element out of several elements - rather than assigning an existing value to a new constant, re-importing the same functionality under a new name, type alias with no generic/const generic parameters... - those are simple renames (or calls of a function with no arguments, or similar "trivial").

For example, a const value definition, or any expression in general, is usually useful only if it creates something new out of existing element(s). But, each of them may be, or may not be (yet), deprecated, independently.

If we use #[allow(deprecated)] in front (just before) such a definition, that hides any future deprecation on other element(s) that we don't get to hear about (Yes, we'd get a warning on wherever we import those other element(s) with use, but that's irrelevant here. We may also use a full path instead of an import.)

We could have #[allow(deprecated] applied to only parts of the result expression, but that's a pain, and it makes code long/wide and unreadable:

assert_eq!(is_copy(#[allow(deprecated)] TRAIT_FLAGS_NO_COPY_NO_DEFAULT), false);