Skip to content

Implement MIN/MAX constants for non-zero integers #93293

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Mar 11, 2022

Conversation

nvzqz
Copy link
Contributor

@nvzqz nvzqz commented Jan 25, 2022

This adds the associated MIN/MAX constants to NonZero{U,I}{8,16,32,64,128,size}, requested in #89065.

This reimplements #89077 due that PR being stagnant for 4 months. I am fine with closing this in favor of that one if the author revisits it. If so, I'd like to see that PR have the docs link to the $Int's constants.

@rust-highfive
Copy link
Contributor

r? @scottmcm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 25, 2022
@nvzqz
Copy link
Contributor Author

nvzqz commented Jan 25, 2022

r? @joshtriplett

@camelid camelid added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 3, 2022
@camelid
Copy link
Member

camelid commented Mar 3, 2022

triage: @nvzqz could you address Josh's review?

Was accidentally placed on unsigned integers, where it is not relevant.
@nvzqz
Copy link
Contributor Author

nvzqz commented Mar 10, 2022

@joshtriplett good catch on the mixup. Fixed and sorry for the delay.

@joshtriplett
Copy link
Member

joshtriplett commented Mar 10, 2022

I may be missing something, but is it possible to implement this without calling unwrap? This lives in the same module as the struct definition; can it just do NonZeroXYZ(1)? Or is there a reason why that can't work?

@nvzqz
Copy link
Contributor Author

nvzqz commented Mar 10, 2022

I had tried but there's this error:

error[E0133]: initializing type with rustc_layout_scalar_valid_range attr is unsafe and requires unsafe block

I did not want to add unsafe where none is really needed.

@joshtriplett
Copy link
Member

@nvzqz Ah, interesting. I didn't realize that rustc treated that as unsafe.

I suppose that in practice the optimizer will eliminate the unwrap here.

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 11, 2022

📌 Commit ecb7927 has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 11, 2022
@nvzqz
Copy link
Contributor Author

nvzqz commented Mar 11, 2022

@joshtriplett you actually don't need to rely on the optimizer here because it's evaluated as a const. Because of that, using 0 here would be a compile error. So it makes no runtime impact 😊

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2022
Rollup of 5 pull requests

Successful merges:

 - rust-lang#93293 (Implement `MIN`/`MAX` constants for non-zero integers)
 - rust-lang#94356 (Rename unix::net::SocketAddr::from_path to from_pathname and stabilize it)
 - rust-lang#94765 (Rename is_{some,ok,err}_with to is_{some,ok,err}_and.)
 - rust-lang#94819 (configure: don't serialize empty array elements)
 - rust-lang#94826 (Improve doc wording for retain on some collections)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d58c69a into rust-lang:master Mar 11, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 11, 2022
@nvzqz nvzqz deleted the nonzero-min-max branch March 12, 2022 13:12
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 11, 2023
…=dtolnay

Stabilize `nonzero_min_max`

## Overall

Stabilizes `nonzero_min_max` to allow the "infallible" construction of ordinary minimum and maximum `NonZero*` instances.

The feature is fairly straightforward and already matured for some time in stable toolchains.

```rust
let _ = NonZeroU8::MIN;
let _ = NonZeroI32::MAX;
```

## History

* On 2022-01-25, implementation was [created](rust-lang#93293).

## Considerations

* This report is fruit of the inanition observed after two unsuccessful attempts at getting feedback.
* Other constant variants discussed at rust-lang#89065 (comment) are orthogonal to this feature.

Fixes rust-lang#89065
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 11, 2023
…=dtolnay

Stabilize `nonzero_min_max`

## Overall

Stabilizes `nonzero_min_max` to allow the "infallible" construction of ordinary minimum and maximum `NonZero*` instances.

The feature is fairly straightforward and already matured for some time in stable toolchains.

```rust
let _ = NonZeroU8::MIN;
let _ = NonZeroI32::MAX;
```

## History

* On 2022-01-25, implementation was [created](rust-lang#93293).

## Considerations

* This report is fruit of the inanition observed after two unsuccessful attempts at getting feedback.
* Other constant variants discussed at rust-lang#89065 (comment) are orthogonal to this feature.

Fixes rust-lang#89065
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Jun 1, 2023
Stabilize `nonzero_min_max`

## Overall

Stabilizes `nonzero_min_max` to allow the "infallible" construction of ordinary minimum and maximum `NonZero*` instances.

The feature is fairly straightforward and already matured for some time in stable toolchains.

```rust
let _ = NonZeroU8::MIN;
let _ = NonZeroI32::MAX;
```

## History

* On 2022-01-25, implementation was [created](rust-lang/rust#93293).

## Considerations

* This report is fruit of the inanition observed after two unsuccessful attempts at getting feedback.
* Other constant variants discussed at rust-lang/rust#89065 (comment) are orthogonal to this feature.

Fixes rust-lang/rust#89065
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants