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

Make ptr range iterable #89900

Closed
wants to merge 1 commit into from
Closed

Make ptr range iterable #89900

wants to merge 1 commit into from

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Oct 15, 2021

This PR adds std::iter::Step impls for *const T and *mut T, enabling iteration over a range of pointers, such as produced by <[T]>::as_ptr_range and <[T]>::as_mut_ptr_range.

It is common for systems interacting with C++ to need to use pointer ranges for iteration, as begin/end pointer pairs are a design pattern in the C++ standard library.

// Designed to be called from C++ or C.
#[no_mangle]
unsafe extern "C" fn demo(start: *const u16, end: *const u16) {
    for ptr in start..end {
        println!("{}", *ptr);
    }
}

fn main() {
    let slice = &[1u16, 2, 3];
    let range = slice.as_ptr_range();
    unsafe { demo(range.start, range.end); }
}

@dtolnay dtolnay added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Oct 15, 2021
@rust-highfive
Copy link
Collaborator

r? @kennytm

(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 Oct 15, 2021
@dtolnay dtolnay added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Oct 15, 2021
@leonardo-m
Copy link

Isn't it better to convert the pair of pointers coming from C or C++ into something like a slice, for Rust usage? What do you need all those pointers for in Rust code?

@kennytm
Copy link
Member

kennytm commented Oct 15, 2021

If end - start does not wholly divide size_of::<T>(), its forward may never hit end. In particular it will crash with `Step` invariants not upheld in case like:

let start = !1 as *const [u8; 2];
let end = !0 as *const [u8; 2];
(start..end).next();

@dtolnay
Copy link
Member Author

dtolnay commented Oct 26, 2021

@leonardo-m:

Isn't it better to convert the pair of pointers coming from C or C++ into something like a slice, for Rust usage?

In FFI, I definitely don't think so. Materializing a reference (to a slice, or even to individual elements) is more prone to UB in Rust than directly working with the foreign pointers as pointers. One basically has to be an expert in Rust aliasing rules, which are not necessarily cogently written down at this point yet, in order to do it correctly. Whereas someone with basic C++ knowledge can write the pointer-based code and have it be correct.

Compare:

let start: *mut T = ...;
let end: *mut T = ...;
for ptr in start..end {
    // Only the preconditions of the C++ callee come into play, which a C++ dev
    // is already going to be comfortable reasoning about. Nothing about stacked
    // borrows, concurrency, initializedness, inhabitedness, alignedness, or other
    // Rust-isms is relevant.
    unsafe { cpp(ptr); }
}
let mut slice = std::slice::from_raw_parts_mut(...); // suddenly a pile of extremely subtle
                                                     // invariants in order for this to be allowed
for elem in slice {
    unsafe { cpp(elem as *mut T); } // probably UB
}

@dtolnay
Copy link
Member Author

dtolnay commented Oct 26, 2021

@kennytm:

If end - start does not wholly divide size_of::<T>(), its forward may never hit end. In particular it will crash with `Step` invariants not upheld

Good call. Hopefully this is fixable within the constraints of the Step trait design. The mental model I want for the behavior of the iterator is that beginT..endT produces the same sequence of numeric values as (beginT as usize..endT as usize).step_by(size_of::<T>()).

Copy link
Member Author

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

`Step` invariants not upheld

It turns out the desired semantics for pointer ranges is incompatible with the current requirements of Step, so I've opened #91000 with a different approach. Closing this PR in favor of that one.

@dtolnay dtolnay closed this Nov 18, 2021
@dtolnay dtolnay deleted the ptrstep branch November 18, 2021 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-fcp This change is insta-stable, so needs a completed FCP to proceed. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants