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 extending null() and null_mut() to non-Sized thin pointers #93959

Closed
3 tasks
SimonSapin opened this issue Feb 13, 2022 · 6 comments · Fixed by #94954
Closed
3 tasks

Tracking Issue for extending null() and null_mut() to non-Sized thin pointers #93959

SimonSapin opened this issue Feb 13, 2022 · 6 comments · Fixed by #94954
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. 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.

Comments

@SimonSapin
Copy link
Contributor

SimonSapin commented Feb 13, 2022

Feature gate: maybe none? There’s no obvious way to use #[unstable] for this kind of library change.

This is a tracking issue for extending ptr::null, ptr::null_mut, and NonNull::dangling to work with any thin pointers. This change was accepted as part of RFC 2580 Pointer Metadata.

As of Rust 1.58:

  • These functions can only return pointer to T: Sized types. (This bound is implied by the lack of T: ?Sized pseudo-bound in their respective signature.)
  • extern types are the only kind of types that are not Sized but pointers to them are still “thin”. (As opposed to pointers to slices or trait objects, that are “wide” and store a length or a vtable pointer as pointer metadata.)

So this description could be rephrased as extending ptr::null, ptr::null_mut, and NonNull::dangling to extern types. However future language proposals could potentially add new kinds of !Sized types whose pointers are thin. This change should then apply to such types too.

Motivation

These functions deliberately do not support slices or trait objects. What length or vtable would be used in the new pointers? More generally, their implementation is only obvious for thin pointers. That is, when pointer metadata is zero-size, such as for Sized types.

However extern types add an intermediate kind of type that is !Sized but still still has thin pointers. The new Pointee trait from the Pointer Metadata RFC allows expressing more precise bounds within the trait system. That same RFC proposed the change tracked here and was accepted, but that change is not implemented yet so it remains an open question for extern types.

Public API

These existing stable APIs of core::ptr / std::ptr:

pub const fn null<T>() -> *const T {}
pub const fn null_mut<T>() -> *mut T {}

Should be changed to

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

… where Thin is an existing trait alias:

pub trait Thin = Pointee<Metadata = ()>;
pub trait Pointee {
    type Metadata: Copy + Send + Sync + Ord + Hash + Unpin;
}

In 1.58, Pointee and Thin are both unstable and tracked at #81513. Pointee is automatically implemented for all types. For Sized and extern types, Metadata is (). So Sized implies Thin.

Because Sized implies Thin, this proposed change to the signatures of stable functions should be backward-compatible.

Having an unstable trait involved in a bound of a stable function is unusual but not unprecedented. (For example Pattern in str::find.)

Steps / History

  • Implementation: #...
  • Final comment period (FCP)
  • Stabilization PR (?)

Implementation strategy and related language change

null, null_mut, and dangling are implemented by converting an integer (zero or align_of()) to a raw pointer with the as operator. In Rust 1.58, as only allows such a conversion if the target type is a pointer to a Sized type. The obvious way to implement this proposed extension of null and friends would be to make the same extension to as. However this change in language semantics is not part of an accepted RFC. It should be approved by the language team, perhaps with its own RFC.

To summarize, the proposed language change is:

Cast expressions integer as *const T and integer as *mut T would change from being legal only if T: Sized, to being legal only if T: ?Sized + Thin.

Update: ptr::from_raw_parts could be used instead of as.

Unresolved Questions

  • Is the language change to the as operator described above desirable?
  • Should it separately go through the RFC process?
@SimonSapin SimonSapin added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Feb 13, 2022
@SimonSapin
Copy link
Contributor Author

As @scottmcm pointed out on Zulip, instead of of extending as the implementation of null could look like this: ptr::from_raw_parts(0 as *const ()). So I’m withdrawing the language change proposal. (I someone else wants to propose the same elsewhere, feel free.)

@SimonSapin
Copy link
Contributor Author

In terms of its own signature NonNull::dangling seems like it could be extended to all thin pointers, but in terms of its behavior it relies of align_of. What should align_of (or align_of_val_raw) be for extern types? Let’s… not get into that here. So not touching dangling and only extending null and null_mut.

@SimonSapin SimonSapin changed the title Tracking Issue for extending null, null_mut, dangling to non-Sized thin pointers Tracking Issue for extending null, null_mut, ~dangling~ to non-Sized thin pointers Feb 13, 2022
@SimonSapin SimonSapin changed the title Tracking Issue for extending null, null_mut, ~dangling~ to non-Sized thin pointers Tracking Issue for extending null() and null_mut() to non-Sized thin pointers Feb 13, 2022
@SimonSapin
Copy link
Contributor Author

Because Sized implies Thin,

We know that, but the compiler doesn’t (yet)

this proposed change to the signatures of stable functions should be backward-compatible.

… so this wrong. I in core itself I get only one error, but it likely affects every user of null or null_mut in a generic context where T: Sized holds but T: Thin doesn’t without an explicit bound:

error[E0271]: type mismatch resolving `<T as Pointee>::Metadata == ()`
   --> library/core/src/sync/atomic.rs:177:24
    |
177 |         AtomicPtr::new(crate::ptr::null_mut())
    |                        ^^^^^^^^^^^^^^^^^^^^ expected `()`, found associated type
    |
    = 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-traits.html
note: required by a bound in `ptr::null_mut`
   --> library/core/src/ptr/mod.rs:232:35
    |
232 | pub const fn null_mut<T: ?Sized + Thin>() -> *mut T {
    |                                   ^^^^ required by this bound in `ptr::null_mut`

I tried encoding the desired implication with this:

--- library/core/src/marker.rs
+++ library/core/src/marker.rs
@@ -89,7 +90,7 @@ impl<T: ?Sized> !Send for *mut T {}
 )]
 #[fundamental] // for Default, for example, which requires that `[T]: !Default` be evaluatable
 #[rustc_specialization_trait]
-pub trait Sized {
+pub trait Sized: ptr::Thin {
     // Empty.
 }

But this caused many errors like:

error[E0271]: type mismatch resolving `<Cell<T> as Pointee>::Metadata == ()`
   --> library/core/src/cell.rs:260:18
    |
260 | impl<T: Default> Default for Cell<T> {
    |                  ^^^^^^^ expected associated type, found `()`
    |
    = note: expected associated type `<Cell<T> as Pointee>::Metadata`
                     found unit type `()`
    = help: consider constraining the associated type `<Cell<T> as Pointee>::Metadata` to `()`
    = note: for more information, visit https://doc.rust-lang.org/book/ch19-03-advanced-traits.html

I think this is because Default is defined as:

pub trait Default: Sized {

So impl Default for Cell<T> requires Cell<T>: Thin.

Since #81172, when asked specifically for <Cell<T> as Pointee>::Metadata the compile knows to resolve it to (). But trait resolution doesn’t seem to be aware of the "equality".

@Kixunil
Copy link
Contributor

Kixunil commented Feb 14, 2022

I don't see a problem with defining Sized: Thin since the trait is implemented by the compiler anyway. Is there any obstacle I don't see?

@SimonSapin
Copy link
Contributor Author

The obstacle is that things break when I do it, as described in my previous comment. It looks like #93977 might fix that.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 14, 2022
…ata, r=wesleywiser

Type params and assoc types have unit metadata if they are sized

Extend the logic in `Pointee` projection to ensure that we can satisfy `<T as Pointee>::Metadata = ()` if `T: Sized`.

cc: `@SimonSapin` and rust-lang#93959
bors added a commit to rust-lang-ci/rust that referenced this issue May 25, 2022
Extend ptr::null and null_mut to all thin (including extern) types

Fixes rust-lang#93959

This change was accepted in https://rust-lang.github.io/rfcs/2580-ptr-meta.html

Note that this changes the signature of **stable** functions. The change should be backward-compatible, but it is **insta-stable** since it cannot (easily, at all?) be made available only through a `#![feature(…)]` opt-in.

The RFC also proposed the same change for `NonNull::dangling`, which makes sense it terms of its signature but not in terms of its implementation. `dangling` uses `align_of()` as an address. But what `align_of()` should be for extern types or whether it should be allowed at all remains an open question.

This commit depends on rust-lang#93977, which is not yet part of the bootstrap compiler. So `#[cfg]` is used to only apply the change in stage 1+. As far a I know bounds cannot be made conditional with `#[cfg]`, so the entire functions are duplicated. This is unfortunate but temporary.

Since this duplication makes it less obvious in the diff, the new definitions differ in:

* More permissive bounds (`Thin` instead of implied `Sized`)
* Different implementation
* Having `rustc_allow_const_fn_unstable(const_fn_trait_bound)`
* Having `rustc_allow_const_fn_unstable(ptr_metadata)`
@bors bors closed this as completed in 7ccc09b May 25, 2022
@madsmtm
Copy link
Contributor

madsmtm commented May 26, 2022

Thanks for doing this @SimonSapin!

workingjubilee pushed a commit to tcdi/postgrestd that referenced this issue Sep 15, 2022
Fixes rust-lang/rust#93959

This change was accepted in https://rust-lang.github.io/rfcs/2580-ptr-meta.html

Note that this changes the signature of **stable** functions.
The change should be backward-compatible, but it is **insta-stable**
since it cannot (easily, at all?) be made available only
through a `#![feature(…)]` opt-in.

The RFC also proposed the same change for `NonNull::dangling`,
which makes sense it terms of its signature but not in terms of its implementation.
`dangling` uses `align_of()` as an address. But what `align_of()` should be for
extern types or whether it should be allowed at all remains an open question.

This commit depends on rust-lang/rust#93977, which is not yet
part of the bootstrap compiler. So `#[cfg]` is used to only apply the change in
stage 1+. As far a I know bounds cannot be made conditional with `#[cfg]`, so the
entire functions are duplicated. This is unfortunate but temporary.

Since this duplication makes it less obvious in the diff,
the new definitions differ in:

* More permissive bounds (`Thin` instead of implied `Sized`)
* Different implementation
* Having `rustc_allow_const_fn_unstable(ptr_metadata)`
workingjubilee pushed a commit to tcdi/postgrestd that referenced this issue Sep 15, 2022
Extend ptr::null and null_mut to all thin (including extern) types

Fixes rust-lang/rust#93959

This change was accepted in https://rust-lang.github.io/rfcs/2580-ptr-meta.html

Note that this changes the signature of **stable** functions. The change should be backward-compatible, but it is **insta-stable** since it cannot (easily, at all?) be made available only through a `#![feature(…)]` opt-in.

The RFC also proposed the same change for `NonNull::dangling`, which makes sense it terms of its signature but not in terms of its implementation. `dangling` uses `align_of()` as an address. But what `align_of()` should be for extern types or whether it should be allowed at all remains an open question.

This commit depends on rust-lang/rust#93977, which is not yet part of the bootstrap compiler. So `#[cfg]` is used to only apply the change in stage 1+. As far a I know bounds cannot be made conditional with `#[cfg]`, so the entire functions are duplicated. This is unfortunate but temporary.

Since this duplication makes it less obvious in the diff, the new definitions differ in:

* More permissive bounds (`Thin` instead of implied `Sized`)
* Different implementation
* Having `rustc_allow_const_fn_unstable(const_fn_trait_bound)`
* Having `rustc_allow_const_fn_unstable(ptr_metadata)`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. 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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants