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

A new prelude for the 2021 edition (trait-only edition) #3114

Merged
merged 4 commits into from
May 25, 2021

Conversation

djc
Copy link
Contributor

@djc djc commented Apr 26, 2021

As requested by the Libs team (via @m-ou-se), here's a smaller, trait-only 2021 prelude RFC. This also addresses some of the feedback from RFC 3090 and includes a more detailed migration lint design.

Rendered

@djc djc force-pushed the prelude-2021-traits branch from 981eb6d to b88bcf0 Compare April 26, 2021 20:42
@djc djc mentioned this pull request Apr 26, 2021
Copy link
Member

@m-ou-se m-ou-se left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

A few comments:

text/0000-prelude-2021-traits.md Outdated Show resolved Hide resolved
text/0000-prelude-2021-traits.md Outdated Show resolved Hide resolved
text/0000-prelude-2021-traits.md Outdated Show resolved Hide resolved
text/0000-prelude-2021-traits.md Outdated Show resolved Hide resolved
@m-ou-se m-ou-se added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Apr 28, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Apr 28, 2021

There's a few comments and small details that need to be addressed before we can merge this. But I'm starting the libs FCP process now to not delay this any further. We already discussed these additions in a meeting earlier this year, but we still need formal approval for these prelude additions.

Specifically, this RFC proposes to make the prelude dependent on the edition, and to add to add three traits to the Rust 2021 prelude:

  • TryInto
  • TryFrom
  • FromIterator

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 28, 2021

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Apr 28, 2021
@yaahc
Copy link
Member

yaahc commented Apr 29, 2021

Looks good to me.

Conditional based on the existing comments all being resolved:

@rfcbot reviewed

djc and others added 3 commits April 30, 2021 22:12
Co-authored-by: Kornel <kornel@geekhood.net>
Co-authored-by: Mara Bos <m-ou.se@m-ou.se>
@Lokathor
Copy link
Contributor

Nit Pick: The inherent impl not method on bool was already rejected from being added to core on the grounds that people would get an unused import warning, and so we'll have to wait for some inherent traits proposal to be accepted and stabilized (which could take far longer than the 2021 edition)

@djc
Copy link
Contributor Author

djc commented Apr 30, 2021

Oh, that's good to know. Do you have a reference for that?

@Lokathor
Copy link
Contributor

rust-lang/rust#82786

So, yeah, we should just put Not in the darn prelude in 2021 and get all this nonsense over with.

@CryZe
Copy link

CryZe commented Apr 30, 2021

Putting just Not in and not the various other unary operators or even all operators would be pretty inconsistent. Since it's also potentially a pretty strong idiomatic shift even (with many people adopting it and many not), it seems too controversial to just slip into an RFC of common not so controversial imports, especially if this would only be a workaround to the path ahead of adding inherent trait impls.

@Lokathor
Copy link
Contributor

pretty strong idiomatic shift

this feels like an overstatement of the situation. It's a single method.

@m-ou-se
Copy link
Member

m-ou-se commented May 3, 2021

@nikomatsakis Can you sign off on the migration plan?

@BurntSushi @dtolnay @sfackler If any of you have time to review this; would be nice to start the FCP soon. (Sorry for the rush, but it'd be nice if we can finish the list of 2021 edition changes before publishing the blog post. This is the last one that still needs fcp. :) )

@rfcbot rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label May 3, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented May 3, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label May 3, 2021
@BurntSushi
Copy link
Member

LGTM. Thanks for putting this together!

With respect to Not, I agree with @CryZe's perspective here.

With respect to naming, I personally would prefer rust2021 instead of rust_2021. I find the former far more aesthetically pleasing, to the point that it outweighs any rigorous consistency argument with naming elsewhere. With that said, I don't believe that these are names that folks will be uttering or even reading frequently, so this is a pretty minor point.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve the migration lint description =)

@m-ou-se
Copy link
Member

m-ou-se commented May 3, 2021

With respect to naming, I personally would prefer rust2021 instead of rust_2021. I find the former far more aesthetically pleasing, to the point that it outweighs any rigorous consistency argument with naming elsewhere. With that said, I don't believe that these are names that folks will be uttering or even reading frequently, so this is a pretty minor point.

Some data: There are zero matches for rust20\d\d in compiler/, but 53 matches for rust_20\d\d. We already have user-facing names like #![warn(rust_2018_idioms)] with underscores. Internally in rustc it's written like that too for functions like span.rust_2018().

@BurntSushi
Copy link
Member

@m-ou-se Ah okay. That's enough to change my preference then. Thanks for that data!

@rfcbot rfcbot added to-announce and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels May 13, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented May 13, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@m-ou-se m-ou-se merged commit 8f1618a into rust-lang:master May 25, 2021
@m-ou-se
Copy link
Member

m-ou-se commented May 25, 2021

Huzzah! The @rust-lang/libs team has decided to accept this RFC.

To track further discussion, subscribe to the tracking issue here: rust-lang/rust#85684

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 2, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 14, 2021
…joshtriplett

Stabilize {std, core}::prelude::rust_*.

This stabilizes the `{core, std}::prelude::{rust_2015, rust_2018, rust_2021}` modules.

The usage of these modules as the prelude in those editions was already stabilized. This just stabilizes the modules themselves, making it possible for a user to explicitly refer to them.

Tracking issue: rust-lang#85684

FCP on the RFC that included this finished here: rust-lang/rfcs#3114 (comment)
@camsteffen
Copy link

Maybe I'm too late, but I want to suggest adding Deref/DerefMut. It seems odd that I need to import in order to write something like x.deref().to_owned().

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 14, 2021
…joshtriplett

Stabilize {std, core}::prelude::rust_*.

This stabilizes the `{core, std}::prelude::{rust_2015, rust_2018, rust_2021}` modules.

The usage of these modules as the prelude in those editions was already stabilized. This just stabilizes the modules themselves, making it possible for a user to explicitly refer to them.

Tracking issue: rust-lang#85684

FCP on the RFC that included this finished here: rust-lang/rfcs#3114 (comment)
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 15, 2021
…joshtriplett

Stabilize {std, core}::prelude::rust_*.

This stabilizes the `{core, std}::prelude::{rust_2015, rust_2018, rust_2021}` modules.

The usage of these modules as the prelude in those editions was already stabilized. This just stabilizes the modules themselves, making it possible for a user to explicitly refer to them.

Tracking issue: rust-lang#85684

FCP on the RFC that included this finished here: rust-lang/rfcs#3114 (comment)
fee1-dead added a commit to fee1-dead-contrib/rust that referenced this pull request Jun 22, 2021
…_lint, r=nikomatsakis

Add `future_prelude_collision` lint

Implements rust-lang#84594. (RFC rust-lang/rfcs#3114 ([rendered](https://github.com/rust-lang/rfcs/blob/master/text/3114-prelude-2021.md))) Not entirely complete but wanted to have my progress decently available while I finish off the last little bits.

Things left to implement:

* [x] UI tests for lints
* [x] Only emit lint for 2015 and 2018 editions
* [ ] Lint name/message bikeshedding
* [x] Implement for `FromIterator` (from best I can tell, the current approach as mentioned from [this comment](rust-lang#84594 (comment)) won't work due to `FromIterator` instances not using dot-call syntax, but if I'm correct about this then that would also need to be fixed for `TryFrom`/`TryInto`)*
* [x] Add to `rust-2021-migration` group? (See rust-lang#85512) (added to `rust-2021-compatibility` group)
* [ ] Link to edition guide in lint docs

*edit: looked into it, `lookup_method` will also not be hit for `TryFrom`/`TryInto` for non-dotcall syntax. If anyone who is more familiar with typecheck knows the equivalent for looking up associated functions, feel free to chime in.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 22, 2021
…int, r=nikomatsakis

Add `future_prelude_collision` lint

Implements rust-lang#84594. (RFC rust-lang/rfcs#3114 ([rendered](https://github.com/rust-lang/rfcs/blob/master/text/3114-prelude-2021.md))) Not entirely complete but wanted to have my progress decently available while I finish off the last little bits.

Things left to implement:

* [x] UI tests for lints
* [x] Only emit lint for 2015 and 2018 editions
* [ ] Lint name/message bikeshedding
* [x] Implement for `FromIterator` (from best I can tell, the current approach as mentioned from [this comment](rust-lang#84594 (comment)) won't work due to `FromIterator` instances not using dot-call syntax, but if I'm correct about this then that would also need to be fixed for `TryFrom`/`TryInto`)*
* [x] Add to `rust-2021-migration` group? (See rust-lang#85512) (added to `rust-2021-compatibility` group)
* [ ] Link to edition guide in lint docs

*edit: looked into it, `lookup_method` will also not be hit for `TryFrom`/`TryInto` for non-dotcall syntax. If anyone who is more familiar with typecheck knows the equivalent for looking up associated functions, feel free to chime in.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC. to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants