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.

Sign up for GitHub

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

Jump to bottom

Constify a few (Partial)Ord impls #92390

Merged
merged 3 commits into from
Jul 26, 2022
Merged

Constify a few (Partial)Ord impls #92390

merged 3 commits into from
Jul 26, 2022

Conversation

fee1-dead
Copy link
Member

@fee1-dead fee1-dead commented Dec 29, 2021 •
edited
Loading

Only a few impls are constified for now, as #92257 has not landed in the bootstrap compiler yet and quite a few impls would need that fix.

This unblocks #92228, which unblocks marking iterator methods as default_method_body_is_const.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 29, 2021
@fee1-dead fee1-dead mentioned this pull request Dec 29, 2021
Closed
3 tasks
@fee1-dead fee1-dead changed the title Constify a few const (Partial)Ord impls Constify a few (Partial)Ord impls Dec 29, 2021
@fee1-dead fee1-dead mentioned this pull request Dec 31, 2021
Open
11 tasks
@RalfJung
Copy link
Member

RalfJung commented Jan 1, 2022 •
edited
Loading

We have to be very careful here -- floating point operations in const are unstable, but impls are insta-stable. We need to make sure that we do not accidentally allow stable code to perform floating point comparisons when const impl becomes stable.
(This PR of course doesn't stabilize anything, but the PR that stabilizes const impl could very easily miss that some of those impls use unstable features that should not be stabilized yet.)

@fee1-dead
Copy link
Member Author

fee1-dead commented Jan 2, 2022 •
edited
Loading

We have to be very careful here -- floating point operations in const are unstable, but impls are insta-stable. We need to make sure that we do not accidentally allow stable code to perform floating point comparisons when const impl becomes stable. (This PR of course doesn't stabilize anything, but the PR that stabilizes const impl could very easily miss that some of those impls use unstable features that should not be stabilized yet.)

Just updated the description on the tracking issue.

@Mark-Simulacrum
Copy link
Member

r? @oli-obk

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 13, 2022
@oli-obk oli-obk added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Mar 2, 2022
@bors
Copy link
Contributor

bors commented Mar 10, 2022

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

@fee1-dead fee1-dead added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2022
@apiraino apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label May 19, 2022
@GKFX GKFX mentioned this pull request Jul 16, 2022
Merged
@rustbot
Copy link
Collaborator

rustbot commented Jul 24, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor

oli-obk commented Jul 25, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jul 25, 2022

📌 Commit 65fca6d has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jul 25, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 25, 2022
@Dylan-DPC
Rollup merge of rust-lang#92390 - fee1-dead-contrib:const_cmp, r=oli-obk
5509e42 
Constify a few `(Partial)Ord` impls

Only a few `impl`s are constified for now, as rust-lang#92257 has not landed in the bootstrap compiler yet and quite a few impls would need that fix.

This unblocks rust-lang#92228, which unblocks marking iterator methods as `default_method_body_is_const`.
@Dylan-DPC Dylan-DPC mentioned this pull request Jul 25, 2022
Closed
@JohnTitor JohnTitor mentioned this pull request Jul 25, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2022
@bors
Auto merge of rust-lang#99735 - JohnTitor:rollup-d93jyr2, r=JohnTitor
a867059 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#92390 (Constify a few `(Partial)Ord` impls)
 - rust-lang#97077 (Simplify some code that depend on Deref)
 - rust-lang#98710 (correct the output of a `capacity` method example)
 - rust-lang#99084 (clarify how write_bytes can lead to UB due to invalid values)
 - rust-lang#99178 (Lighten up const_prop_lint, reusing const_prop)
 - rust-lang#99673 (don't ICE on invalid dyn calls)
 - rust-lang#99703 (Expose size_hint() for TokenStream's iterator)
 - rust-lang#99709 (`Inherited` always has `TypeckResults` available)
 - rust-lang#99713 (Fix sidebar background)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c1647e1 into rust-lang:master Jul 26, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 26, 2022
@leonardo-m
Copy link

This small improvement allows to write many more const functions :)

@oli-obk oli-obk deleted the const_cmp branch July 27, 2022 19:42
@leonardo-m
Copy link

Is #![feature(const_cmp)] needed to compile?

yvt added a commit to r3-os/r3 that referenced this pull request Aug 13, 2022
@yvt
refactor: use <integer>: const Ord
dff2f58 
`[ref:int_const_ord]` has been resolved by [rust-lang/rust#92390][1]. 

[1]: rust-lang/rust#92390
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk left review comments

Assignees

@oli-obk oli-obk

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.