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 generic NonZero #120257

Closed
20 of 21 tasks
reitermarkus opened this issue Jan 23, 2024 · 34 comments
Closed
20 of 21 tasks

Tracking Issue for generic NonZero #120257

reitermarkus opened this issue Jan 23, 2024 · 34 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@reitermarkus
Copy link
Contributor

reitermarkus commented Jan 23, 2024

Feature gate: #![feature(generic_nonzero)]

This is a tracking issue for replacing the distinct NonZero* types with a generic NonZero<T> type. This allows using NonZero with FFI type aliases instead of having weird type names like NonZero_c_ulonglong.

This replaces the following tracking issues:

Public API

use core::num::{NonZero, NonZeroU8};

assert_eq!(NonZero::new(33u8), NonZeroU8::new(33));

Steps / History

Unresolved Questions

  • None yet.

Footnotes

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

@reitermarkus reitermarkus added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 23, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jan 30, 2024
…=dtolnay

Remove `raw_os_nonzero` feature.

This feature is superseded by a generic `NonZero` type: rust-lang#120257

Closes rust-lang#82363.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 30, 2024
Rollup merge of rust-lang#120295 - reitermarkus:remove-ffi-nonzero, r=dtolnay

Remove `raw_os_nonzero` feature.

This feature is superseded by a generic `NonZero` type: rust-lang#120257

Closes rust-lang#82363.
@reitermarkus
Copy link
Contributor Author

@dtolnay, given the documentation issue, making NonZero::new and other constructors generic can really only be done during/after stabilization, so the only blocker I can see right now is stabilization itself. Can this enter FCP, or do we want to wait for #120486?

@dtolnay dtolnay added the I-libs-api-nominated The issue / PR has been nominated for discussion during a libs-api team meeting. label Jan 31, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 3, 2024
…ructors, r=dtolnay,oli-obk

Make `NonZero` constructors generic.

This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax.

Tracking issue: rust-lang#120257

~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~

```rust
101 |         if n == T::ZERO {
    |            ^^^^^^^^^^^^ expected `host`, found `true`
    |
    = note: expected constant `host`
               found constant `true`
```

r? `@dtolnay`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 4, 2024
…ructors, r=dtolnay,oli-obk

Make `NonZero` constructors generic.

This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax.

Tracking issue: rust-lang#120257

~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~

```rust
101 |         if n == T::ZERO {
    |            ^^^^^^^^^^^^ expected `host`, found `true`
    |
    = note: expected constant `host`
               found constant `true`
```

r? ``@dtolnay``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 5, 2024
…ructors, r=dtolnay,oli-obk

Make `NonZero` constructors generic.

This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax.

Tracking issue: rust-lang#120257

~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~

```rust
101 |         if n == T::ZERO {
    |            ^^^^^^^^^^^^ expected `host`, found `true`
    |
    = note: expected constant `host`
               found constant `true`
```

r? ```@dtolnay```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 5, 2024
…ructors, r=dtolnay,oli-obk

Make `NonZero` constructors generic.

This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax.

Tracking issue: rust-lang#120257

~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~

```rust
101 |         if n == T::ZERO {
    |            ^^^^^^^^^^^^ expected `host`, found `true`
    |
    = note: expected constant `host`
               found constant `true`
```

r? ````@dtolnay````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 5, 2024
…ructors, r=dtolnay,oli-obk

Make `NonZero` constructors generic.

This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax.

Tracking issue: rust-lang#120257

~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~

```rust
101 |         if n == T::ZERO {
    |            ^^^^^^^^^^^^ expected `host`, found `true`
    |
    = note: expected constant `host`
               found constant `true`
```

r? `````@dtolnay`````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 6, 2024
…ructors, r=dtolnay,oli-obk

Make `NonZero` constructors generic.

This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax.

Tracking issue: rust-lang#120257

~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~

```rust
101 |         if n == T::ZERO {
    |            ^^^^^^^^^^^^ expected `host`, found `true`
    |
    = note: expected constant `host`
               found constant `true`
```

r? `@dtolnay`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 6, 2024
…ructors, r=dtolnay,oli-obk

Make `NonZero` constructors generic.

This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax.

Tracking issue: rust-lang#120257

~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~

```rust
101 |         if n == T::ZERO {
    |            ^^^^^^^^^^^^ expected `host`, found `true`
    |
    = note: expected constant `host`
               found constant `true`
```

r? ``@dtolnay``
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 7, 2024
…ctors, r=dtolnay,oli-obk

Make `NonZero` constructors generic.

This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax.

Tracking issue: rust-lang#120257

~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~

```rust
101 |         if n == T::ZERO {
    |            ^^^^^^^^^^^^ expected `host`, found `true`
    |
    = note: expected constant `host`
               found constant `true`
```

r? `@dtolnay`
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 8, 2024
…ctors, r=dtolnay

Make `NonZero` constructors generic.

This makes `NonZero` constructors generic, so that `NonZero::new` can be used without turbofish syntax.

Tracking issue: rust-lang#120257

~~I cannot figure out how to make this work with `const` traits. Not sure if I'm using it wrong or whether there's a bug:~~

```rust
101 |         if n == T::ZERO {
    |            ^^^^^^^^^^^^ expected `host`, found `true`
    |
    = note: expected constant `host`
               found constant `true`
```

r? `@dtolnay`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 9, 2024
…ructors, r=Nilstrieb

Use `transmute_unchecked` in `NonZero::new`.

Tracking issue: rust-lang#120257

See rust-lang#120521 (comment).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 9, 2024
…ructors, r=Nilstrieb

Use `transmute_unchecked` in `NonZero::new`.

Tracking issue: rust-lang#120257

See rust-lang#120521 (comment).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 9, 2024
…ructors, r=Nilstrieb

Use `transmute_unchecked` in `NonZero::new`.

Tracking issue: rust-lang#120257

See rust-lang#120521 (comment).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 9, 2024
…ructors, r=Nilstrieb

Use `transmute_unchecked` in `NonZero::new`.

Tracking issue: rust-lang#120257

See rust-lang#120521 (comment).
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 9, 2024
Rollup merge of rust-lang#120809 - reitermarkus:generic-nonzero-constructors, r=Nilstrieb

Use `transmute_unchecked` in `NonZero::new`.

Tracking issue: rust-lang#120257

See rust-lang#120521 (comment).
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Feb 10, 2024
…r=Nilstrieb

Use `transmute_unchecked` in `NonZero::new`.

Tracking issue: rust-lang/rust#120257

See rust-lang/rust#120521 (comment).
oli-obk added a commit to oli-obk/rust that referenced this issue Feb 13, 2024
…r=dtolnay

Use generic `NonZero` internally.

Tracking issue: rust-lang#120257
oli-obk added a commit to oli-obk/rust that referenced this issue Feb 13, 2024
…r=dtolnay

Make `NonZero::get` generic.

Tracking issue: rust-lang#120257

Depends on rust-lang#120521.

r? `@dtolnay`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
Make `NonZero::get` generic.

Tracking issue: rust-lang/rust#120257

Depends on rust-lang/rust#120521.

r? `@dtolnay`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
Use generic `NonZero` in tests.

Tracking issue: rust-lang/rust#120257

r? `@dtolnay`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
…,wesleywiser

Move generic `NonZero` `rustc_layout_scalar_valid_range_start` attribute to inner type.

Tracking issue: rust-lang/rust#120257

r? `@dtolnay`
@tmccombs
Copy link
Contributor

tmccombs commented Apr 10, 2024

Just a thought:

I think this could potentially be made user-extensable if there was an API that looked like:

pub trait Zeroable: Copy + StructuralPartialEq + Sized {
  const ZERO: Self;
}

pub struct NonZero<T: Zeroable>(T);

impl<T: Zeroable> NonZero<T> {
  const fn new(v: T) -> Option<Self> {
    if (v == T::ZERO) {
      None
    } else {
      Some(NonZero(v))
    }
  }
  // ...
}

@kennytm
Copy link
Member

kennytm commented Apr 10, 2024

@tmccombs For all built-in integer types, Option<NonZero<T>> is layout-compatible with T (same size_of). Unless there is a way to specify the niche without #[rustc_layout_scalar_valid_range_{start,end}], such advantage cannot translate into user-extensible types.

@tmccombs
Copy link
Contributor

Oh, I forgot to mention that the compiler would need to be able to treat rhe value of ZERO as a niche for the NonZero type. I'm not sure how feasible that is.

@Alonely0
Copy link

Alonely0 commented Apr 11, 2024

Just a thought:

I think this could potentially be made user-extensable if there was an API that looked like:

pub trait Zeroable: Copy + StructuralPartialEq + Sized {
  const ZERO: Self;
}

pub struct NonZero<T: Zeroable>(T);

impl<T: Zeroable> NonZero<T> {
  const fn new(v: T) -> Option<Self> {
    if (v == T::ZERO) {
      None
    } else {
      Some(NonZero(v))
    }
  }
  // ...
}

Could we instead have it just be an unsafe marker auto trait (without the associated const), and later on have a WithNiche trait that'd be a generalization in which the niche may or may not be a zeroed bit pattern? Your proposal seems a bit weird to me, since the compiler should be able to create a zeroed bit pattern for any known type. Furthermore, this would allow us to move this particular issue forward while we deal with the complexity of user-defined niches in a separate RFC.

Bikeshed:

use core::mem::MaybeUninit;

unsafe auto trait InvalidZeroed: WithNiche {}
unsafe trait WithNiche {
    const NICHE: Self;
}

unsafe impl<T: InvalidZeroed> WithNiche for T {
    const NICHE: T = unsafe { MaybeUninit::zeroed().assume_init() };
}

@reitermarkus
Copy link
Contributor Author

reitermarkus commented Apr 11, 2024

the compiler should be able to create a zeroed bit pattern for any known type

The problem is not creating the bit pattern, it's ensuring that pattern is used as a niche, which currently can only be done with #[rustc_layout_scalar_valid_range_{start,end}], i.e. not in user code.

Furthermore, this would allow us to move this particular issue forward

Creating a user-implementable Zeroable trait is not part of this tracking issue, so this is not a blocker in any case.

@RalfJung
Copy link
Member

Indeed, any discussion of having user-defined NonZero types is off-topic in this tracking issue.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 11, 2024
@rfcbot
Copy link

rfcbot commented Apr 11, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@tmccombs
Copy link
Contributor

Indeed, any discussion of having user-defined NonZero types is off-topic in this tracking issue.

I probably should have been more clear, but the reason I brought it up is I'm not sure if it is merged as is, if it would be possible to change it to allow user-defined NonZero types in the future in a backwards compatible way.

@reitermarkus
Copy link
Contributor Author

if it would be possible to change it to allow user-defined NonZero types in the future in a backwards compatible way.

Yes, the ZeroablePrimitive trait is perma-unstable and sealed in order to avoid any compatibility issues.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 22, 2024
…e, r=dtolnay

Stabilize generic `NonZero`.

Tracking issue: rust-lang#120257

r? `@dtolnay`
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 23, 2024
Rollup merge of rust-lang#124230 - reitermarkus:generic-nonzero-stable, r=dtolnay

Stabilize generic `NonZero`.

Tracking issue: rust-lang#120257

r? `@dtolnay`
@dtolnay dtolnay closed this as completed Apr 23, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Apr 23, 2024
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Make `NonZero::get` generic.

Tracking issue: rust-lang/rust#120257

Depends on rust-lang/rust#120521.

r? `@dtolnay`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Use generic `NonZero` in tests.

Tracking issue: rust-lang/rust#120257

r? `@dtolnay`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
…,wesleywiser

Move generic `NonZero` `rustc_layout_scalar_valid_range_start` attribute to inner type.

Tracking issue: rust-lang/rust#120257

r? `@dtolnay`
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Apr 29, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 8, 2024
…r=dtolnay

Generic `NonZero` post-stabilization changes.

Tracking issue: rust-lang#120257

r? `@dtolnay`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 8, 2024
…r=dtolnay

Generic `NonZero` post-stabilization changes.

Tracking issue: rust-lang#120257

r? ``@dtolnay``
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 8, 2024
Rollup merge of rust-lang#124587 - reitermarkus:use-generic-nonzero, r=dtolnay

Generic `NonZero` post-stabilization changes.

Tracking issue: rust-lang#120257

r? ``@dtolnay``
github-actions bot pushed a commit to rust-lang/miri that referenced this issue May 11, 2024
Generic `NonZero` post-stabilization changes.

Tracking issue: rust-lang/rust#120257

r? ``@dtolnay``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. 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