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

Add #[rustc_per_edition] for edition-dependent type aliases. #82489

Closed
wants to merge 2 commits into from

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented Feb 24, 2021

This will make it possible to have e.g. std::ops::Range refer to different types depending on the edition of the code using that alias.

Marking this as experimental for now, because I don't know yet if this is something we want.

Thanks @eddyb for pointing out this can be done relatively easily with type aliases.

cc @sfackler, since you're working on the new Range types, for which this could be useful.

As this PR shows, the change to rustc itself is pretty simple. But rustdoc (and racer/rls and rust-analyzer) would also need special support for this, which I haven't looked into yet.

r? @ghost

@m-ou-se m-ou-se added A-typesystem Area: The type system T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. labels Feb 24, 2021
@rust-log-analyzer

This comment has been minimized.

@eddyb
Copy link
Member

eddyb commented Feb 24, 2021

One very important design decision is that pub use std::ops::Range as MyRange; still gives you the "edition-polymorphic" alias, while pub type MyRange<T> = std::ops::Range<T>; forces an edition to be picked.

It might be more desirable to have imports also force choosing one of the alternatives, and if we can do it at the import level, it would work for other items as well, but it is trickier.

Something I was thinking might work is have a setup like this:

#[rustc_per_edition]
pub use self::rust_0000::Range;

mod rust_2015 { pub use crate::iter::Range; }
mod rust_2018 { pub use crate::iter::Range; }
mod rust_2021 { pub struct Range<T> {...} ... }

Where rustc_resolve rewrites rust_0000 in the path depending on the edition, when the attribute is present (this happens in the crate this is written, i.e. libcore, as paths themselves are not kept cross-crate).

(Other than replacing an identifier, we could instead inject an extra path component just before the last one or w/e, but that feels "more magical")

While it would resolve to rust_2021 inside libcore itself, we could record an Export that contains per-edition resolutions, so that cross-crate rustc_resolve can just pull the right set of resolutions for wherever that #[rustc_per_edition] core::ops::Range is imported.

cc @petrochenkov

@petrochenkov
Copy link
Contributor

Could someone explain what are the goals exactly?

  1. libstd is built with edition set in stone (now 2018, soon 2021).
  2. Some other crate is built in an arbitrary edition e and uses libstd.
  3. ??? Range ???

@m-ou-se
Copy link
Member Author

m-ou-se commented Feb 25, 2021

I'd like to find a way to make it possible to use editions for small breaking changes in the standard library as well, instead of only in the language. (E.g. for renaming/moving items.) This seems to be a way to achieve that without making things too complicated.

For Range specifically: a..b currently expands to a value of type std::ops::Range, which is an Iterator and is not Copy. We now think it might be better if a..b would've expanded to a type that is Copy, and implements IntoIterator. We can already change the compiler/language to expand a..b to a different type in a new edition, but ideally, this new Copy type would be called std::ops::Range in the new edition too (and we'd move the iterator type to std::iter::Range or something).

Basically: It'd be nice if in Edition 2021 code, std::ops::Range would refer to a Copy type, while in older editions, it still refers to the Iterator type like it does now (which we might put under std::iter::Range).

@m-ou-se
Copy link
Member Author

m-ou-se commented Feb 25, 2021

We already did something like this for panic: std::panic!() 'resolves' to either panic_2015!() or panic_2021!(). (That's now implemented as a builtin macro. But maybe a more general mechanism would be possible.)

@Dylan-DPC
Copy link
Member

@m-ou-se any updates on this?

@bors
Copy link
Contributor

bors commented Jun 3, 2022

☔ The latest upstream changes (presumably #97694) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 3, 2022
@JohnCSimon
Copy link
Member

@m-ou-se
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Nov 27, 2022
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-typesystem Area: The type system S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants