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

Tracking Issue for making ptr offset methods "const fn" #71499

Closed
2 of 5 tasks
josephlr opened this issue Apr 24, 2020 · 9 comments · Fixed by #93957
Closed
2 of 5 tasks

Tracking Issue for making ptr offset methods "const fn" #71499

josephlr opened this issue Apr 24, 2020 · 9 comments · Fixed by #93957
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@josephlr
Copy link
Contributor

josephlr commented Apr 24, 2020

The feature gate for this is #![feature(const_ptr_offset)]. Miri has been able to handle this for some time. This feature allows code like:

static FOO: u8 = 42;
static PTR1: *const u8 = &FOO as *const u8;
static PTR2: *const u8 = PTR1.wrapping_offset(1);

Steps (based on #69821)

Unresolved Questions

CC: @RalfJung @oli-obk @rust-lang/wg-const-eval

@josephlr josephlr added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Apr 24, 2020
@RalfJung
Copy link
Member

RalfJung commented Apr 24, 2020

Cc @rust-lang/wg-const-eval (unfortunately GH only lets org members ping teams :/ )

Note that Miri already has implementations of the relevant intrinsics but they need to be moved to rustc.

@jonas-schievink jonas-schievink added the A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. label Apr 24, 2020
RalfJung added a commit to RalfJung/rust that referenced this issue May 21, 2020
Make pointer offset methods/intrinsics const

Implements rust-lang#71499 using [the implementations from miri](https://github.com/rust-lang/miri/blob/52f5d202bdcfe8986f0615845f8d1647ab8a2c6a/src/shims/intrinsics.rs#L96-L112). However, unlike the miri implementation, I made the implementation a single function for both `offset` and `arith_offset`.

I added some tests what's allowed and what's UB. Let me know if any other cases should be added.

CC: @RalfJung @oli-obk
RalfJung added a commit to RalfJung/rust that referenced this issue May 29, 2020
Make pointer offset methods/intrinsics const

Implements rust-lang#71499 using [the implementations from miri](https://github.com/rust-lang/miri/blob/52f5d202bdcfe8986f0615845f8d1647ab8a2c6a/src/shims/intrinsics.rs#L96-L112).

I added some tests what's allowed and what's UB. Let me know if any other cases should be added.

CC: @RalfJung @oli-obk
RalfJung added a commit to RalfJung/rust that referenced this issue May 29, 2020
Make pointer offset methods/intrinsics const

Implements rust-lang#71499 using [the implementations from miri](https://github.com/rust-lang/miri/blob/52f5d202bdcfe8986f0615845f8d1647ab8a2c6a/src/shims/intrinsics.rs#L96-L112).

I added some tests what's allowed and what's UB. Let me know if any other cases should be added.

CC: @RalfJung @oli-obk
RalfJung added a commit to RalfJung/rust that referenced this issue May 29, 2020
Make pointer offset methods/intrinsics const

Implements rust-lang#71499 using [the implementations from miri](https://github.com/rust-lang/miri/blob/52f5d202bdcfe8986f0615845f8d1647ab8a2c6a/src/shims/intrinsics.rs#L96-L112).

I added some tests what's allowed and what's UB. Let me know if any other cases should be added.

CC: @RalfJung @oli-obk
RalfJung added a commit to RalfJung/rust that referenced this issue May 29, 2020
Make pointer offset methods/intrinsics const

Implements rust-lang#71499 using [the implementations from miri](https://github.com/rust-lang/miri/blob/52f5d202bdcfe8986f0615845f8d1647ab8a2c6a/src/shims/intrinsics.rs#L96-L112).

I added some tests what's allowed and what's UB. Let me know if any other cases should be added.

CC: @RalfJung @oli-obk
RalfJung added a commit to RalfJung/rust that referenced this issue May 29, 2020
Make pointer offset methods/intrinsics const

Implements rust-lang#71499 using [the implementations from miri](https://github.com/rust-lang/miri/blob/52f5d202bdcfe8986f0615845f8d1647ab8a2c6a/src/shims/intrinsics.rs#L96-L112).

I added some tests what's allowed and what's UB. Let me know if any other cases should be added.

CC: @RalfJung @oli-obk
@RalfJung
Copy link
Member

One concern with these intrinsics that needs to be resolved before stabilization: for offset/add/sub, we cannot actually check during const-eval if the operation overflows, because that depends on the actual integer address the allocation starts at.

@aodhneine
Copy link

Hi, what's the progress on this issue?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 23, 2021

for offset/add/sub, we cannot actually check during const-eval if the operation overflows, because that depends on the actual integer address the allocation starts at.

As decided in the unsafe const RFC, we only need to do a best effort UB check.

I reviewed the methods in question and believe we can stabilize all of them. The only user visible const/runtime differences occur in the presence of UB.

@RalfJung
Copy link
Member

Indeed I think there is no reason to wait longer, we can stabilize this. Does someone want to prepare a stabilization PR?

@m-ou-se m-ou-se added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 9, 2022
@m-ou-se
Copy link
Member

m-ou-se commented Mar 9, 2022

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Mar 9, 2022

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 Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 9, 2022
@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Mar 16, 2022
@rfcbot
Copy link

rfcbot commented Mar 16, 2022

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

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 16, 2022
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Mar 26, 2022
@rfcbot
Copy link

rfcbot commented Mar 26, 2022

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.

This will be merged soon.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Mar 26, 2022
@bors bors closed this as completed in 223b58e Mar 27, 2022
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this 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 a pull request may close this issue.

8 participants