Skip to content

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented Jun 4, 2022

This is only the intrinsics, which aren't exposed anywhere, so I think we can just do this without an FCP.

The behaviour of these is well understood, have been implemented for ages, and don't offer any new functionality to const fns -- it would be completely legal for them to be implemented via wrapping_add, since we don't promise to catch all UB.

(Sending this because I was working on something else where I wanted to use unchecked_sub for a length in a const fn, and it complained at me.)

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 4, 2022
@rust-highfive
Copy link
Contributor

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 r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. 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-highfive
Copy link
Contributor

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 Jun 4, 2022
@scottmcm scottmcm force-pushed the const-stabilize-int-unchecked branch from b8cf2b1 to d23dee8 Compare June 4, 2022 23:25
@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum
Copy link
Member

r? @oli-obk for const-eval(?) decision here

@scottmcm scottmcm force-pushed the const-stabilize-int-unchecked branch from d23dee8 to 801334a Compare June 5, 2022 04:59
@bors
Copy link
Collaborator

bors commented Jun 8, 2022

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

This is only the intrinsics, which aren't exposed anywhere, so I think we can just do this without an FCP.

The behaviour of these is well understood, have been implemented for ages, and don't offer any new functionality to `const fn`s -- it would be completely legal for them to be implemented via `wrapping_add`, since we don't promise to catch all UB.
@scottmcm scottmcm force-pushed the const-stabilize-int-unchecked branch from 801334a to d8198ca Compare June 8, 2022 17:07
@oli-obk
Copy link
Contributor

oli-obk commented Jun 13, 2022

We have never const-stabilized an intrinsic without also stabilizing a function using it before. As per the big banner at the top of the file, intrinsics need to be signed off by the lang team, and I don't have a good argument for stabilizing it without a const fn that needs it.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 21, 2022

Closing this as per my comment above

@oli-obk oli-obk closed this Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants