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

Allow deprecating a re-export #30827

Open
seanmonstar opened this issue Jan 11, 2016 · 11 comments
Open

Allow deprecating a re-export #30827

seanmonstar opened this issue Jan 11, 2016 · 11 comments
Labels
A-stability Area: `#[stable]`, `#[unstable]` etc. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-help-wanted Call for participation: Help is requested to fix this issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@seanmonstar
Copy link
Contributor

This would allow a module to rename an identifier without breaking users, and provide a message that they should change.

Example:

mod foo {
    #[deprecated(reason = "use Foo instead")]
    pub use self::Foo as Bar;

    pub struct Foo(pub i32);
}

use self::foo::Bar; // <- deprecrated, use Foo instead
@steveklabnik steveklabnik added A-lang C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jan 26, 2016
@steveklabnik steveklabnik added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed A-lang labels Mar 24, 2017
@Mark-Simulacrum Mark-Simulacrum added the A-stability Area: `#[stable]`, `#[unstable]` etc. label Jun 19, 2017
@steveklabnik
Copy link
Member

Triage: the final syntax for deprecated uses note instead of reason:

mod foo {
    #[deprecated(note = "use Foo instead")]
    pub use self::Foo as Bar;

    pub struct Foo(pub i32);
}

use self::foo::Bar; // <- deprecrated, use Foo instead

however, this still reproduces. With no comments in two years, I'm not sure if this will ever be implemented. @rust-lang/lang , is this feature desired?

@joshtriplett
Copy link
Member

I'd like to see this; deprecated should work on any declaration.

@Centril
Copy link
Contributor

Centril commented Sep 24, 2018

I agree with @joshtriplett.

@eddyb
Copy link
Member

eddyb commented Sep 24, 2018

IIUC, in the current system, by the time stability/deprecation checks run, information about where you got something from, is lost (it's transient during name resolution).
cc @petrochenkov

@petrochenkov petrochenkov added the E-help-wanted Call for participation: Help is requested to fix this issue. label Sep 24, 2018
@petrochenkov
Copy link
Contributor

E-help-wanted
I'm unlikely to work on this in the next couple of years, and have no desire to mentor on this specific issue (that is sill significant work).
Someone just needs to do this thankless job and 1) (preferable) move stability/deprecation checking to resolve phase or 2) (not preferable) keep relevant data in HIR to perform stability/deprecation checking where it's currently performed.

@petrochenkov
Copy link
Contributor

Same issue - #23937.

@eddyb
Copy link
Member

eddyb commented Sep 26, 2018

@petrochenkov Would it be a good idea to integrate stability / deprecation with visibility a bit more?
That is, the result of path / associated item / field / etc. name resolution can be:

  • accessible
    • optionally deprecated
  • not accessible, because it's:
    • private
    • unstable

And these would compose across the entire import chain to that destination.

However, one complicated aspect is how hierarchical stability is - if something is not annotated, one of its parents might be instead, and that's where it's inherited from.
OTOH, (path) name resolution should already have access to the DefId tree, for... privacy checking!

So we'd be doing the "reverse" computation (walking up parents), which shouldn't be significantly more expensive, if we're caching everything computed along the way.

@petrochenkov
Copy link
Contributor

@eddyb
Merging visibility and stability into a single thing literally is probably not a good idea.
Visibilities are also important during import resolution and affect how far things are reexported (with globs or not), while stability/deprecation is a single post-processing check not interacting with anything else.
That said, the error "trying to access something unstable" is probably going to be reported in the same place as "trying to access something private" despite the differences, after resolving a path segment.

@bstrie
Copy link
Contributor

bstrie commented Feb 22, 2021

Note that this caused actual problems in the standard library itself, where two items that were supposedly deprecated were silently failing to trigger the deprecation warning for several years: #82080

Would making the attribute itself error when applied to a use be easier than making this work properly? This would still be better than the current behavior of silently failing to apply the attribute.

@bstrie
Copy link
Contributor

bstrie commented Mar 18, 2021

From #83269 (comment) :

I wonder if we could tweak things slightly so that it does work - also see #83248 (comment).

The information of which imports you went through should exist during name resolution, and we'd only have to keep track of one DefId (the "nearest" pub use to the import) for deprecation checking purposes.

alyssais added a commit to alyssais/vm-virtio that referenced this issue Sep 14, 2022
As pointed out by Sebastien Boeuf [1], the kernel headers are
backwards compatible, so there's no need to maintain the bindings for
multiple kernel versions.  And doing so will make updating the
bindings easier, as it won't require adding a new feature each time.

It's unfortunately not possible for us to emit a deprecation warning
for the features, as in Rust deprecation warnings have to be applied
to functions/constants/etc. (not modules or re-exports [2]), so any
deprecation warning in this crate would mean editing the generated
bindings files.  But it doesn't really do any harm to keep the feature
declarations in the Cargo.toml around indefinitely.

[1]: rust-vmm/virtio-bindings#35 (comment)
[2]: rust-lang/rust#30827

Signed-off-by: Alyssa Ross <[email protected]>
lauralt pushed a commit to rust-vmm/vm-virtio that referenced this issue Sep 14, 2022
As pointed out by Sebastien Boeuf [1], the kernel headers are
backwards compatible, so there's no need to maintain the bindings for
multiple kernel versions.  And doing so will make updating the
bindings easier, as it won't require adding a new feature each time.

It's unfortunately not possible for us to emit a deprecation warning
for the features, as in Rust deprecation warnings have to be applied
to functions/constants/etc. (not modules or re-exports [2]), so any
deprecation warning in this crate would mean editing the generated
bindings files.  But it doesn't really do any harm to keep the feature
declarations in the Cargo.toml around indefinitely.

[1]: rust-vmm/virtio-bindings#35 (comment)
[2]: rust-lang/rust#30827

Signed-off-by: Alyssa Ross <[email protected]>
@davidhewitt
Copy link
Contributor

Hey, I'd find the ability to deprecate re-exports very useful for maintaining PyO3, so I'm interested in working on this.

Someone just needs to do this thankless job and 1) (preferable) move stability/deprecation checking to resolve phase

@petrochenkov is the above statement still true, and option (1) preferred? From a quick scan of the code it looks like that is the case. Appreciate that you previously said you're not interested in mentoring this issue. Is there a Zulip room which would be appropriate for me to ask questions about direction / implementation of this?

phip1611 added a commit to phip1611/uefi-rs that referenced this issue Jul 27, 2024
As deprecated re-exports are unsupported [0], we go the hard way and
just make it a breaking change.

Along the way, I reordered some statements in some files to follow our typical
order of:
- pub mod
- private mod
- pub use
- private use

[0] rust-lang/rust#30827
tgross35 added a commit to tgross35/rust that referenced this issue Sep 24, 2024
rustdoc: inherit parent's stability where applicable

It is currently not possible for a re-export to have a different stability (rust-lang#30827). Therefore the standard library uses a hack when moving items like `std::error::Error` or `std::net::IpAddr` into `core` by marking the containing module (`core::error` / `core::net`) as unstable or stable in a later version than the items the module contains.

Previously, rustdoc would always show the *stability as declared* for an item rather than the *stability as publicly reachable* (i.e. the features required to actually access the item), which could be confusing when viewing the docs. This PR changes it so that we show the stability of the first unstable parent or the most recently stabilized parent instead, to hopefully make things less confusing.

fixes rust-lang#130765

screenshots:
![error in std](https://github.com/user-attachments/assets/2ab9bdb9-ed81-4e45-a832-ac7d3ba1be3f) ![error in core](https://github.com/user-attachments/assets/46f46182-5642-4ac5-b92e-0b99a8e2496d)
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 25, 2024
Rollup merge of rust-lang#130798 - lukas-code:doc-stab, r=notriddle

rustdoc: inherit parent's stability where applicable

It is currently not possible for a re-export to have a different stability (rust-lang#30827). Therefore the standard library uses a hack when moving items like `std::error::Error` or `std::net::IpAddr` into `core` by marking the containing module (`core::error` / `core::net`) as unstable or stable in a later version than the items the module contains.

Previously, rustdoc would always show the *stability as declared* for an item rather than the *stability as publicly reachable* (i.e. the features required to actually access the item), which could be confusing when viewing the docs. This PR changes it so that we show the stability of the first unstable parent or the most recently stabilized parent instead, to hopefully make things less confusing.

fixes rust-lang#130765

screenshots:
![error in std](https://github.com/user-attachments/assets/2ab9bdb9-ed81-4e45-a832-ac7d3ba1be3f) ![error in core](https://github.com/user-attachments/assets/46f46182-5642-4ac5-b92e-0b99a8e2496d)
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Sep 25, 2024
rustdoc: inherit parent's stability where applicable

It is currently not possible for a re-export to have a different stability (rust-lang/rust#30827). Therefore the standard library uses a hack when moving items like `std::error::Error` or `std::net::IpAddr` into `core` by marking the containing module (`core::error` / `core::net`) as unstable or stable in a later version than the items the module contains.

Previously, rustdoc would always show the *stability as declared* for an item rather than the *stability as publicly reachable* (i.e. the features required to actually access the item), which could be confusing when viewing the docs. This PR changes it so that we show the stability of the first unstable parent or the most recently stabilized parent instead, to hopefully make things less confusing.

fixes rust-lang/rust#130765

screenshots:
![error in std](https://github.com/user-attachments/assets/2ab9bdb9-ed81-4e45-a832-ac7d3ba1be3f) ![error in core](https://github.com/user-attachments/assets/46f46182-5642-4ac5-b92e-0b99a8e2496d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-stability Area: `#[stable]`, `#[unstable]` etc. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-help-wanted Call for participation: Help is requested to fix this issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants