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

*const T has quite a few methods that seem like they don't need to require T: Sized #42847

Open
Mark-Simulacrum opened this issue Jun 23, 2017 · 13 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Mark-Simulacrum
Copy link
Member

Not sure which exactly, but we should evaluate and relax bounds where possible.

See also #24794, #36952.

@Mark-Simulacrum Mark-Simulacrum added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jun 23, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 23, 2017
@smmalis37
Copy link
Contributor

Adding to this, std::ptr::null & std::ptr::null_mut. Technically not methods on *const T but has these bounds too.

@ExpHP
Copy link
Contributor

ExpHP commented Feb 21, 2018

All inherent methods and trait impls for *const T and *mut T now have a T: ?Sized bound thanks to #44932 and #46094.

Following the paper trail, it appears that there may be good reason that std::ptr::{null,null_mut} do not support T: ?Sized.

@shepmaster
Copy link
Member

Should I open a new issue to argue about std::ptr::null / std::ptr::null_mut? Seems wrong when used with extern types:

extern {
    type foo_t;
}

fn fake_foo_new() -> *const foo_t {
    std::ptr::null()
}
error[E0277]: the trait bound `foo_t: std::marker::Sized` is not satisfied
  --> src/main.rs:15:5
   |
15 |     std::ptr::null()
   |     ^^^^^^^^^^^^^^ `foo_t` does not have a constant size known at compile-time
   |
   = help: the trait `std::marker::Sized` is not implemented for `foo_t`
   = note: required by `std::ptr::null`

@ExpHP
Copy link
Contributor

ExpHP commented Apr 30, 2018

Do mind that the conclusion drawn in the last thread I linked seemed to be that null vtable pointers (even on raw pointers) are currently considered to be UB in rust.

(I'm not saying that you necessarily shouldn't; just pointing out. Maybe new facts and use cases may bring a new perspective)

Edit: wow, it just occurred to me what you're showing in that example. I never realized that rust had any unsized types that use thin pointers!

@shepmaster
Copy link
Member

I'll be that person and ping in @dtolnay, @eddyb, and @mikeyhew for some more discussion around my last comment

@kennytm
Copy link
Member

kennytm commented May 5, 2018

Unless there's a trait to distinguish between thin+unsized and fat+unsized pointers, we cannot relax the Sized bounds. In particular, trait object pointers *const dyn Trait must not have a null vtable (unless we have impl Trait for ! for all traits), and thus null() should not be defined for trait objects.

When the custom DST traits are included we could define

fn null<T: ?Sized>() -> *const T
where
    T::Meta: Default,
{
    /* assemble a custom DST from { ptr = 0, meta = T::Meta::default() } */
}

@shepmaster
Copy link
Member

shepmaster commented May 5, 2018

I can certainly see that a null vtable would be a bad thing, but I also feel that the complete inability to create a NULL or uninitialized extern type is going to be a real blocker to a lot of usage of extern types:

#![feature(extern_types)]

extern {
    type foo_t;
    
    fn foo_init(f: *mut foo_t);
}

fn main() {
    let mut a = std::mem::uninitialized();
    unsafe { foo_init(&mut a) };
}
error[E0277]: the trait bound `foo_t: std::marker::Sized` is not satisfied
  --> src/main.rs:10:9
   |
10 |     let mut a = std::mem::uninitialized();
   |         ^^^^^ `foo_t` does not have a constant size known at compile-time
   |
   = help: the trait `std::marker::Sized` is not implemented for `foo_t`
   = note: all local variables must have a statically known size

Edit — that was a dumb example because of course a can't be on the stack, it's unsized. The real example of that would work:

#![feature(extern_types)]

extern "C" {
    type foo_t;

    fn foo_init(f: *mut *mut foo_t);
}

fn main() {
    unsafe {
        let mut a = std::mem::uninitialized();
        foo_init(&mut a);
    }
}

The corresponding equivalent with null still doesn't:

#![feature(extern_types)]

extern "C" {
    type foo_t;

    fn foo_init(f: *mut *mut foo_t);
}

fn main() {
    unsafe {
        let mut a = std::ptr::null_mut();
        foo_init(&mut a);
    }
}
error[E0277]: the trait bound `foo_t: std::marker::Sized` is not satisfied
  --> src/main.rs:11:21
   |
11 |         let mut a = std::ptr::null_mut();
   |                     ^^^^^^^^^^^^^^^^^^ `foo_t` does not have a constant size known at compile-time
   |
   = help: the trait `std::marker::Sized` is not implemented for `foo_t`
   = note: required by `std::ptr::null_mut`

@dtolnay
Copy link
Member

dtolnay commented May 5, 2018

As a workaround for now you can get a null pointer to extern type through as:

#![feature(extern_types)]

extern "C" {
    type foo_t;

    fn foo_take_null(f: *mut foo_t);
}

fn main() {
    let a = 0 as *mut foo_t;
    unsafe {
        foo_take_null(a);
    }
}

@eddyb
Copy link
Member

eddyb commented Jul 31, 2018

We should add a private/unstable marker trait and use it to add a bound that's weaker than Sized.

It should be directly equivalent to T::Meta == (). The implementation can use unions and an assert that the size of the pointer matches that of a regular pointer (which it always would for this trait).

cc @retep998

@SimonSapin
Copy link
Contributor

Is null and null_mut for extern types the only remaining thing tracked in this issue?


RFC 2580 (implementation PR: #81172) add public APIs for pointer metadata and a pub trait Thin = Pointee<Metadata = ()>;.

Building on top of that, I hoped that fixing this issue would be as simple as

-pub const fn null<T>() -> *const T {
+pub const fn null<T: ?Sized + Thin>() -> *const T {

-pub const fn null_mut<T>() -> *mut T {
+pub const fn null_mut<T: ?Sized + Thin>() -> *mut T {

However this gives two kinds of errors when compiling libcore:

  • error[E0606]: casting `usize` as `*const T` is invalid
       --> library/core/src/ptr/mod.rs:210:5
        |
    210 |     0 as *const T
        |     ^^^^^^^^^^^^^
    
    error[E0606]: casting `usize` as `*mut T` is invalid
       --> library/core/src/ptr/mod.rs:228:5
        |
    228 |     0 as *mut T
        |     ^^^^^^^^^^^

    Possible fix: in compiler/rustc_typeck/src/check/cast.rs, in FnCtxt::pointer_kind, in the ty::Param case: if a Pointee<Metadata = ()> bound is in scope for that parameter return Ok(Some(PointerKind::Thin)). This might be similar to type_is_known_to_be_sized_modulo_regions which for type parameters presumably checks if a (possibly implicity) Sized bound is in scope.

  • error[E0271]: type mismatch resolving `<T as Pointee>::Metadata == ()`
       --> library/core/src/sync/atomic.rs:171:24
        |
    171 |         AtomicPtr::new(crate::ptr::null_mut())
        |                        ^^^^^^^^^^^^^^^^^^^^ expected `()`, found associated type
        |
       ::: library/core/src/ptr/mod.rs:227:35
        |
    227 | pub const fn null_mut<T: ?Sized + Thin>() -> *mut T {
        |                                   ---- required by this bound in `ptr::null_mut`
        |
        = note:    expected unit type `()`
                found associated type `<T as Pointee>::Metadata`
        = help: consider constraining the associated type `<T as Pointee>::Metadata` to `()`
        = note: for more information, visit https://doc.rust-lang.org/book/ch19-03-advanced>

    Possible fix? “Somehow” teach the trait resolver that T: Sized implies T: Pointee<Metadata = ()>. Maybe in the compiler itself, as changing the library definition to trait Sized: Thin {} causes many instances of error[E0271]: type mismatch resolving `<Something as Pointee>::Metadata == ()` .

@Ericson2314
Copy link
Contributor

I hope we can do trait Sized: Thin {} in the library in addition to whatever compiler tricks are needed, as the supertrait is "morally correct" and good for documentation.

@madsmtm
Copy link
Contributor

madsmtm commented Jun 15, 2021

AtomicPtr specifically should probably have the same T: ?Sized + Thin bound, so the issue would go away there - but I agree with making trait Sized: Thin {}

madsmtm added a commit to madsmtm/rust-objc that referenced this issue Jun 15, 2021
To see what changes we'd have to make in the library to use `extern type` (RFC-1861) (when it's is stabilized).

Unfortunately had to change some usage of `ptr::null[_mut]` to `0 as *const/mut X`, see rust-lang/rust#42847.
@jplatte
Copy link
Contributor

jplatte commented Jun 21, 2021

Related: pointer::cast requires the generic parameter U to be Sized. Is this intentional? I would expect it to be a valid way to cast a pointer to a #[repr(transparent)] type to a pointer to its containing type or the other way around, same as an as cast or mem::transmute::<*const ReprTransparentType, *const InnerType>.

madsmtm added a commit to madsmtm/objc2 that referenced this issue Nov 3, 2021
To see what changes we'd have to make in the library to use `extern type` (RFC-1861) (when it's is stabilized).

Unfortunately had to change some usage of `ptr::null[_mut]` to `0 as *const/mut X`, see rust-lang/rust#42847.
madsmtm added a commit to madsmtm/objc2 that referenced this issue Dec 22, 2021
To see what changes we'd have to make in the library to use `extern type` (RFC-1861) (when it's is stabilized).

Unfortunately had to change some usage of `ptr::null[_mut]` to `0 as *const/mut X`, see rust-lang/rust#42847.
madsmtm added a commit to madsmtm/objc2 that referenced this issue Dec 22, 2021
To see what changes we'd have to make in the library to use `extern type` (RFC-1861) (when it's is stabilized).

Unfortunately had to change some usage of `ptr::null[_mut]` to `0 as *const/mut X`, see rust-lang/rust#42847.
madsmtm added a commit to madsmtm/objc2 that referenced this issue Dec 22, 2021
To see what changes we'd have to make in the library to use `extern type` (RFC-1861) (when it's is stabilized).

Unfortunately had to change some usage of `ptr::null[_mut]` to `0 as *const/mut X`, see rust-lang/rust#42847.
madsmtm added a commit to madsmtm/objc2 that referenced this issue Dec 22, 2021
To see what changes we'd have to make in the library to use `extern type` (RFC-1861) (when it's is stabilized).

Unfortunately had to change some usage of `ptr::null[_mut]` to `0 as *const/mut X`, see rust-lang/rust#42847.
madsmtm added a commit to madsmtm/objc2 that referenced this issue Dec 22, 2021
To see what changes we'd have to make in the library to use `extern type` (RFC-1861) (when it's is stabilized).

Unfortunately had to change some usage of `ptr::null[_mut]` to `0 as *const/mut X`, see rust-lang/rust#42847.
madsmtm added a commit to madsmtm/objc2 that referenced this issue Dec 22, 2021
To see what changes we'd have to make in the library to use `extern type` (RFC-1861) (when it's is stabilized).

Unfortunately had to change some usage of `ptr::null[_mut]` to `0 as *const/mut X`, see rust-lang/rust#42847.
madsmtm added a commit to madsmtm/objc2 that referenced this issue Dec 22, 2021
To see what changes we'd have to make in the library to use `extern type` (RFC-1861) (when it's is stabilized).

Unfortunately had to change some usage of `ptr::null[_mut]` to `0 as *const/mut X`, see rust-lang/rust#42847.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. 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