Skip to content

Commit

Permalink
Rollup merge of #118665 - dtolnay:signedness, r=Nilstrieb
Browse files Browse the repository at this point in the history
Consolidate all associated items on the NonZero integer types into a single impl block per type

**Before:**

```rust
#[repr(transparent)]
#[rustc_layout_scalar_valid_range_start(1)]
pub struct NonZeroI8(i8);

impl NonZeroI8 {
    pub const fn new(n: i8) -> Option<Self> ...
    pub const fn get(self) -> i8 ...
}

impl NonZeroI8 {
    pub const fn leading_zeros(self) -> u32 ...
    pub const fn trailing_zeros(self) -> u32 ...
}

impl NonZeroI8 {
    pub const fn abs(self) -> NonZeroI8 ...
}
...
```

**After:**

```rust
#[repr(transparent)]
#[rustc_layout_scalar_valid_range_start(1)]
pub struct NonZeroI8(i8);

impl NonZeroI8 {
    pub const fn new(n: i8) -> Option<Self> ...
    pub const fn get(self) -> i8 ...
    pub const fn leading_zeros(self) -> u32 ...
    pub const fn trailing_zeros(self) -> u32 ...
    pub const fn abs(self) -> NonZeroI8 ...
    ...
}
```

Having 6-7 different impl blocks per type is not such a problem in today's implementation, but becomes awful upon the switch to a generic `NonZero<T>` type (context: #82363 (comment)).

In the implementation from #100428, there end up being **67** impl blocks on that type.

<img src="https://github.com/rust-lang/rust/assets/1940490/5b68bd6f-8a36-4922-baa3-348e30dbfcc1" width="200"><img src="https://github.com/rust-lang/rust/assets/1940490/2cfec71e-c2cd-4361-a542-487f13f435d9" width="200"><img src="https://github.com/rust-lang/rust/assets/1940490/2fe00337-7307-405d-9036-6fe1e58b2627" width="200">

Without the refactor to a single impl block first, introducing `NonZero<T>` would be a usability regression compared to today's separate pages per type. With all those blocks expanded, Ctrl+F is obnoxious because you need to skip 12&times; past every match you don't care about. With all the blocks collapsed, Ctrl+F is useless. Getting to a state in which exactly one type's (e.g. `NonZero<u32>`) impl blocks are expanded while the rest are collapsed is annoying.

After this refactor to a single impl block, we can move forward with making `NonZero<T>` a generic struct whose docs all go on the same rustdoc page. The rustdoc will have 12 impl blocks, one per choice of `T` supported by the standard library. The reader can expand a single one of those impl blocks e.g. `NonZero<u32>` to understand the entire API of that type.

Note that moving the API into a generic `impl<T> NonZero<T> { ... }` is not going to be an option until after `NonZero<T>` has been stabilized, which may be months or years after its introduction. During the period while generic `NonZero` is unstable, it will be extra important to offer good documentation on all methods demonstrating the API being used through the stable aliases such as `NonZeroI8`.

This PR follows a `key = $value` syntax for the macros which is similar to the macros we already use for producing a single large impl block on the integer primitives.

https://github.com/rust-lang/rust/blob/1dd4db50620fb38a6382c22456a96ed7cddeff83/library/core/src/num/mod.rs#L288-L309

Best reviewed one commit at a time.
  • Loading branch information
matthiaskrgr authored Jan 19, 2024
2 parents 1bd42be + cdee1fe commit 122b3f9
Showing 1 changed file with 1,211 additions and 1,265 deletions.
Loading

0 comments on commit 122b3f9

Please sign in to comment.