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 const_swap_nonoverlapping #133668

Open
1 of 4 tasks
RalfJung opened this issue Nov 30, 2024 · 1 comment
Open
1 of 4 tasks

Tracking Issue for const_swap_nonoverlapping #133668

RalfJung opened this issue Nov 30, 2024 · 1 comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC I-lang-nominated Nominated for discussion during a lang team meeting. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Nov 30, 2024

Feature gate: #![feature(const_swap_nonoverlapping)]

This is a tracking issue for making swap_nonoverlapping a const fn.

Public API

mod ptr {
    pub const unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize);
}

Steps / History

Blocking Issues

ptr::swap_nonoverlapping has a limitation currently where it can fail when the data-to-swap contains pointers that cross the "element boundary" of such a swap (i.e., count > 0 and the pointer straddles the boundary between two T). Here's an example of code that unexpectedly fails:

    const {
        let mut ptr1 = &1;
        let mut ptr2 = &666;

        // Swap ptr1 and ptr2, bytewise.
        unsafe {
            ptr::swap_nonoverlapping(
                ptr::from_mut(&mut ptr1).cast::<u8>(),
                ptr::from_mut(&mut ptr2).cast::<u8>(),
                mem::size_of::<&i32>(),
            );
        }

        // Make sure they still work.
        assert!(*ptr1 == 666);
        assert!(*ptr2 == 1);
    };

The proper way to fix this is to implement rust-lang/const-eval#72.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@RalfJung RalfJung added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 30, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 1, 2024
…lnay

Move some functions out of const_swap feature gate

- `swap_unchecked` is still unstable as a regular fn, so that feature gate can also cover its constness.
- `swap_nonoverlapping` isn't ready to be stabilized yet, so make it a different feature gate.

Part of rust-lang#83163, rust-lang#88539, rust-lang#133668
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 1, 2024
…lnay

Move some functions out of const_swap feature gate

- `swap_unchecked` is still unstable as a regular fn, so that feature gate can also cover its constness.
- `swap_nonoverlapping` isn't ready to be stabilized yet, so make it a different feature gate.

Part of rust-lang#83163, rust-lang#88539, rust-lang#133668
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 1, 2024
Rollup merge of rust-lang#133669 - RalfJung:const_swap_splitup, r=dtolnay

Move some functions out of const_swap feature gate

- `swap_unchecked` is still unstable as a regular fn, so that feature gate can also cover its constness.
- `swap_nonoverlapping` isn't ready to be stabilized yet, so make it a different feature gate.

Part of rust-lang#83163, rust-lang#88539, rust-lang#133668
@RalfJung
Copy link
Member Author

RalfJung commented Dec 23, 2024

I guess the alternative is that we could just error when there is a pointer straddling the boundary of one of the to-be-swapped chunks. That is what happens today, and it doesn't seem entirely unreasonable? This is probably a very rare situation, after all.

@rust-lang/wg-const-eval @rust-lang/lang @rust-lang/opsem do you think it is acceptable for the example code above to error? See here for what the error looks like, and here for a variant of the code that does work since the pointer is entirely contained within a single chunk. (The type used for the chunk does not matter, only its size is relevant.) Operations that read/write a part of a pointer generally don't work in const-eval. For ptr::copy and ptr::copy_nonoverlapping we made it work even when a pointer is spread across multiple of the "chunks" being copied, but doing the same for ptr::swap_nonoverlapping is tricky since we would have to allocate some temporary storage somewhere to hold the data from one side while the other side is being copied over.

The other option would be to add full support for "pointer fragments" to const-eval, so that parts a pointer can indeed be copied around and reassambled to a fully usable pointer. That's more attractive honestly (solving the problem properly rather than papering over it with a special-case intrinsic), but also is even more work and risks slowing down the common case where we don't have such pointer fragments. (See rust-lang/const-eval#72.)

@RalfJung RalfJung added the I-lang-nominated Nominated for discussion during a lang team meeting. label Dec 23, 2024
Zalathar added a commit to Zalathar/rust that referenced this issue Dec 24, 2024
core: fix const ptr::swap_nonoverlapping when there are pointers at odd offsets

Ensure that the pointer gets swapped correctly even if it is not stored at an aligned offset. This rules out implementations that copy things in a `usize` loop -- so our implementation needs to be adjusted to avoid such a loop when running in const context.

Part of rust-lang#133668
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 24, 2024
Rollup merge of rust-lang#134689 - RalfJung:ptr-swap-test, r=oli-obk

core: fix const ptr::swap_nonoverlapping when there are pointers at odd offsets

Ensure that the pointer gets swapped correctly even if it is not stored at an aligned offset. This rules out implementations that copy things in a `usize` loop -- so our implementation needs to be adjusted to avoid such a loop when running in const context.

Part of rust-lang#133668
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC I-lang-nominated Nominated for discussion during a lang team meeting. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

1 participant