Skip to content

Tracking issue for std::ptr::NonNull::cast #47653

Closed

Description

@SimonSapin

ptr::NonNull<T> is similar to *mut T. Its cast method added in #47631 is similar to raw pointer casting with the as operator.

impl<T: ?Sized> NonNull<T> {
    pub fn cast<U>(self) -> NonNull<U> {
        unsafe {
            NonNull::new_unchecked(self.as_ptr() as *mut U)
        }
    }
}

Note that T is ?Sized but U is not (or we’d get a E0606 "vtable kinds may not match" error on as)

Activity

  1. joshlf commented on Jan 22, 2018

    @joshlf
    Contributor

    Would it be possible to add a cast_unsized or similar method that converts into a fat pointer? This is probably less urgent - the necessity of cast comes from the fact that NonNull should have feature parity with *mut, and *mut has as - but it's probably worth at least discussing.

  2. SimonSapin commented on Jan 22, 2018

    @SimonSapin
    ContributorAuthor

    How would that work? How does this method you pick a vtable or a slice length? The only way I know in safe Rust (other than ad-hoc APIs like String::into_boxed_str) of creating fat pointers is from thin ones through CoerceUnsized, which NonNull already supports:

    #![feature(box_into_raw_non_null)]
    use std::ptr::NonNull;
    
    fn main() {
        // Creating thin pointers
        let a: NonNull<[u8; 10]> = Box::into_raw_non_null(Box::new([42; 10]));
        let b: NonNull<String> = Box::into_raw_non_null(Box::new(String::new()));
    
        // Coercing to fat pointers to dynamically-sized types
        let _a: NonNull<[u8]> = a;
        let _b: NonNull<std::fmt::Display> = b;
    }
  3. joshlf commented on Jan 22, 2018

    @joshlf
    Contributor

    I suppose I was thinking particularly about statically typed but dynamically sized fat pointers (e.g., to slices). I'm not sure how you'd answer the question about vtables. So, concretely, converting between, e.g., NonNull<[u32]> and NonNull<[u8]>, where the length field can just be maintained because they're both the same number of bytes (or multiplied by four? are fat pointer length fields byte size or element count?).

  4. SimonSapin commented on Jan 22, 2018

    @SimonSapin
    ContributorAuthor

    Turns out you can cast a raw slice to another. This leaves the length unchanged regardless of the item type, which is often wrong. For example, this compiles and prints random (stack?) memory.

    fn main() {
        let b = [1, 2, 3, 4, 5];
        let ptr = cast_slice::<u8, u64>(&b);
        for x in unsafe { &*ptr } {
            println!("{:016x}", x)
        }
    }
    
    fn cast_slice<T, U>(s: *const [T]) -> *const [U] {
        s as _
    }

    However that’s the case where the types on both sides of as are known to be raw slices. In the fully-generic ?Sized case the compiler doesn’t know what code to emit.

  5. joshlf commented on Jan 22, 2018

    @joshlf
    Contributor
  6. SimonSapin commented on Jan 22, 2018

    @SimonSapin
    ContributorAuthor

    This compiles, however wrong it may be:

    fn main() {
        struct Foo(u8, [u8]);
        struct Bar(String, [u64]);
        let ptr: *const Foo = unimplemented!();
        ptr as *const Bar;
    }

    But this is still slice to slice. If trait objects are involved, they need to be the same trait.

    At least that’s the current implementation of as. I don’t have a strong opinion on how it should be.

  7. added
    T-libs-api[DEPRECATED; DO NOT USE]
    B-unstableBlocker: Implemented in the nightly compiler and unstable.
    on Jan 22, 2018
  8. joshlf commented on Jan 22, 2018

    @joshlf
    Contributor

    OK, probably worth it to just stick with cast for the time being and only address this if it becomes an issue for someone.

  9. added
    C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFC
    on Jan 23, 2018
  10. SimonSapin commented on Mar 4, 2018

    @SimonSapin
    ContributorAuthor

    This one-line method landed in Nightly on 2018-02-07. If stabilized in the current 1.26 cycle, it will reach Stable on 2018-05-11 after four months. @aturon, is this enough baking time to start FCP?

  11. SimonSapin commented on Apr 8, 2018

    @SimonSapin
    ContributorAuthor

    #49669 adds a Void extern type (probably to be renamed) and uses it with NonNull. I’ve found myself trying to use this method to try to cast to NonNull<Void>, which doesn’t work because cast requires the destination type to be Sized. We can’t fully generalize to ?Sized because the method wouldn’t know how to make up a vtable pointer or slice length. It’s unfortunate that today’s type system can’t precisely express the actual requirement of a pointer being thin, but I think we can live with it. #49669 ends up adding a specific NonNull::as_void method.

    TR;DR: Despite limitations I think we should stabilize this as-is.

    @rfcbot fcp merge

  12. rfcbot commented on Apr 8, 2018

    @rfcbot

    Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

    No concerns currently listed.

    Once a majority of reviewers approve (and none object), 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.

  13. added
    proposed-final-comment-periodProposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off.
    final-comment-periodIn the final comment period and will be merged soon unless new substantive objections are raised.
    and removed
    proposed-final-comment-periodProposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off.
    on Apr 8, 2018
  14. rfcbot commented on Apr 11, 2018

    @rfcbot

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

  15. added a commit that references this issue on Apr 18, 2018
    ac3c228
  16. jethrogb commented on Aug 28, 2018

    @jethrogb
    Contributor

    Ran into the issue where I can't cast between NonNull slice types.

  17. SimonSapin commented on Aug 28, 2018

    @SimonSapin
    ContributorAuthor
    fn main() {
        let a: *const [u8] = b"foo";
        println!("{:04x?}", unsafe { &*(a as *const [u16]) })
    } 

    Playground, output:

    [6f66, 0a6f, 0000]
    

    The as operator will happily cast a raw slice to another raw slice type by copying the length without consideration of the sizes of the item types, in this case creating a buffer overflow. This is such a footgun that I’m not sure NonNull not supporting it is a bad thing.

    You can use NonNull::from(slice::from_raw_parts(ptr, len)) after considering whether len needs some computation. If alignment is also an issue, [T]::align_to might help as well.

  18. jethrogb commented on Aug 28, 2018

    @jethrogb
    Contributor

    I understand that you can do unsafe (and unsound) things when casting pointers. NonNull is advertised as “*mut T but non-zero and covariant.” It should be able to do everything a regular pointer can do. Of course there are workarounds, but they're just that. I ended up not using NonNull in my code because it was too cumbersome compared to raw pointers.

  19. SimonSapin commented on Aug 28, 2018

    @SimonSapin
    ContributorAuthor

    It’s already discussed above why we can’t just add U: ?Sized to NonNull<T>::cast::<U>(). Do you think it would be useful to add NonNull<[T]>::cast_slice::<U>(self) -> NonNull<[U]> that “dumbly” copies the length like as does?

  20. joshlf commented on Aug 28, 2018

    @joshlf
    Contributor

    It’s already discussed above why we can’t just add U: ?Sized to NonNull<T>::cast::<U>(). Do you think it would be useful to add NonNull<[T]>::cast_slice::<U>(self) -> NonNull<[U]> that “dumbly” copies the length like as does?

    That actually might be useful if only to give us a place to document the footgun that is casting slice pointers. Since it's a keyword, as doesn't have documentation like methods do.

  21. jethrogb commented on Aug 28, 2018

    @jethrogb
    Contributor

    It’s already discussed above why we can’t just add U: ?Sized to NonNull::cast::(). Do you think it would be useful to add NonNull<[T]>::cast_slice::(self) -> NonNull<[U]> that “dumbly” copies the length like as does?

    Yes, except I think it should just be called cast

  22. SimonSapin commented on Aug 28, 2018

    @SimonSapin
    ContributorAuthor

    @joshlf rustdoc does support documenting keywords, these days:
    https://doc.rust-lang.org/std/#keywords
    https://github.com/rust-lang/rust/blob/1.28.0/src/libstd/keyword_docs.rs#L11

    @jethrogb Re calling it cast I see what you mean, but in generics today we don’t have a way to accept Sized types and slices but not trait objects, or to encode that the return type can only be a slice if the input type is as well. Or maybe we could with a dedicated unsafe trait?

  23. SimonSapin commented on Aug 28, 2018

    @SimonSapin
    ContributorAuthor

    Hmm I half-expected an impl coherence error, but this compiles. I’m not sure how I feel about potentially adding it to the standard library though.

    unsafe trait Cast<T: ?Sized> {
        fn cast(ptr: *mut Self) -> *mut T;
    }
    
    // implied: T: Sized
    unsafe impl<T, U: ?Sized> Cast<T> for U {
        fn cast(ptr: *mut Self) -> *mut T {
            ptr as _
        }
    }
    
    unsafe impl<T, U> Cast<[T]> for [U] {
        fn cast(slice: *mut Self) -> *mut [T] {
            unsafe {
                let ptr = (*slice).as_mut_ptr();
                let len = (*slice).len();
                std::slice::from_raw_parts_mut(ptr as _, len)
            }
        }
    }
  24. jethrogb commented on Aug 28, 2018

    @jethrogb
    Contributor

    No need for a trait:

    struct Ptr<T: ?Sized>(*const T);
    
    impl<T> Ptr<T> {
        fn cast<U>(self) -> Ptr<U> {
            Ptr(self.0 as *const _)
        }
    }
    
    impl<T> Ptr<[T]> {
        fn cast<U>(self) -> Ptr<[U]> {
            Ptr(self.0 as *const _)
        }
    }
  25. SimonSapin commented on Aug 28, 2018

    @SimonSapin
    ContributorAuthor

    @jethrogb That code doesn’t have T: ?Sized on the first impl though (to allow e.g. extracting the data pointer of a raw trait object, which NonNull::cast supports today), and adding it makes the two impls conflict.

  26. jethrogb commented on Aug 28, 2018

    @jethrogb
    Contributor

    Yes, you're right, I didn't look at the docs in detail enough to see that existing bound

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    B-unstableBlocker: Implemented in the nightly compiler and unstable.C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCT-libs-api[DEPRECATED; DO NOT USE]final-comment-periodIn the final comment period and will be merged soon unless new substantive objections are raised.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions