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

Add LowerExp and UpperExp implementations to NonZero #131377

Merged
merged 1 commit into from
Nov 3, 2024

Conversation

rick-de-water
Copy link
Contributor

Adds LowerExp and UpperExp trait implementations to NonZero, as discussed in rust-lang/libs-team#458.

I had to modify the macro to mark the new impls with a different rust version. Let me know if this is the right way to do it (first timer here!)

@rustbot
Copy link
Collaborator

rustbot commented Oct 7, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @scottmcm (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 7, 2024
@rust-log-analyzer

This comment has been minimized.

@fmease fmease changed the title Add LowerExp and UpperExp implementations Add LowerExp and UpperExp implementations to NonZero Oct 7, 2024
@@ -110,8 +110,8 @@ impl_zeroable_primitive!(
pub struct NonZero<T: ZeroablePrimitive>(T::NonZeroInner);

macro_rules! impl_nonzero_fmt {
($Trait:ident) => {
#[stable(feature = "nonzero", since = "1.28.0")]
($Trait:ident, $Feature:literal, $Since:literal) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, would it perhaps make sense to use a :meta matcher here?

That plus a repeat in the macro could allow something like

impl_nonzero_fmt!{
    #[stable(feature = "nonzero", since = "1.28.0")]
    Debug

    #[stable(feature = "nonzero", since = "1.28.0")]
    Display

    #[stable(feature = "nonzero_fmt_exp", since = "CURRENT_RUSTC_VERSION")]
    LowerExp
}

which I think would be nice to make it more obvious to people rging and such what the attributes are on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the macro

@scottmcm
Copy link
Member

scottmcm commented Oct 8, 2024

While you're getting CI happy,
@rustbot author

Then please squash the commits and rustbot review it.

@rustbot rustbot added 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 Oct 8, 2024
@scottmcm scottmcm added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels Oct 8, 2024
Mark the new fmt impls with the correct rust version

Clean up the fmt macro and format the tests
@rick-de-water
Copy link
Contributor Author

rick-de-water commented Oct 8, 2024

Applied feedback and squashed commits, please
@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2024
@m-ou-se m-ou-se removed the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Oct 15, 2024
@m-ou-se
Copy link
Member

m-ou-se commented Oct 15, 2024

@rfcbot merge

See rust-lang/libs-team#458:

LowerExp and UpperExp format traits implementations are missing from the NonZero integer types. All other (Binary, *Hex, Octal, etc) have been implemented, so it looks like an oversight, especially since the integer primitives themselves do implement LowerExp and UpperExp.

@rfcbot
Copy link

rfcbot commented Oct 15, 2024

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 15, 2024
@m-ou-se m-ou-se removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels Oct 15, 2024
@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Oct 23, 2024
@rfcbot
Copy link

rfcbot commented Oct 23, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Nov 2, 2024
@rfcbot
Copy link

rfcbot commented Nov 2, 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.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dtolnay
Copy link
Member

dtolnay commented Nov 3, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Nov 3, 2024

📌 Commit fead1d5 has been approved by dtolnay

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 3, 2024
@dtolnay dtolnay assigned dtolnay and unassigned scottmcm Nov 3, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Nov 3, 2024
Add LowerExp and UpperExp implementations to NonZero

Adds `LowerExp` and `UpperExp` trait implementations to `NonZero`, as discussed in rust-lang/libs-team#458.

I had to modify the macro to mark the new impls with a different rust version. Let me know if this is the right way to do it (first timer here!)
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 3, 2024
…kingjubilee

Rollup of 15 pull requests

Successful merges:

 - rust-lang#129329 (Implement `From<&mut {slice}>` for `Box/Rc/Arc<{slice}>`)
 - rust-lang#131377 (Add LowerExp and UpperExp implementations to NonZero)
 - rust-lang#132275 (Register `~const` preds for `Deref` adjustments in HIR typeck)
 - rust-lang#132393 (Docs: added brief colon explanation)
 - rust-lang#132437 (coverage: Regression test for inlining into an uninstrumented crate)
 - rust-lang#132499 (unicode_data.rs: show command for generating file)
 - rust-lang#132503 (better test for const HashMap; remove const_hash leftovers)
 - rust-lang#132520 (NFC add known bug nr to test)
 - rust-lang#132522 (make codegen help output more consistent)
 - rust-lang#132523 (Added regression test for generics index out of bounds)
 - rust-lang#132528 (Use `*_opt` typeck results fns to not ICE in fallback suggestion)
 - rust-lang#132537 (PassWrapper: adapt for llvm/llvm-project@5445edb5d)
 - rust-lang#132540 (Do not format generic consts)
 - rust-lang#132543 (add and update some crashtests)
 - rust-lang#132550 (compiler: Continue introducing rustc_abi to the compiler)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 3, 2024
Add LowerExp and UpperExp implementations to NonZero

Adds `LowerExp` and `UpperExp` trait implementations to `NonZero`, as discussed in rust-lang/libs-team#458.

I had to modify the macro to mark the new impls with a different rust version. Let me know if this is the right way to do it (first timer here!)
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 3, 2024
…iaskrgr

Rollup of 16 pull requests

Successful merges:

 - rust-lang#129329 (Implement `From<&mut {slice}>` for `Box/Rc/Arc<{slice}>`)
 - rust-lang#131377 (Add LowerExp and UpperExp implementations to NonZero)
 - rust-lang#132393 (Docs: added brief colon explanation)
 - rust-lang#132437 (coverage: Regression test for inlining into an uninstrumented crate)
 - rust-lang#132499 (unicode_data.rs: show command for generating file)
 - rust-lang#132503 (better test for const HashMap; remove const_hash leftovers)
 - rust-lang#132511 (stabilize const_arguments_as_str)
 - rust-lang#132520 (NFC add known bug nr to test)
 - rust-lang#132522 (make codegen help output more consistent)
 - rust-lang#132523 (Added regression test for generics index out of bounds)
 - rust-lang#132528 (Use `*_opt` typeck results fns to not ICE in fallback suggestion)
 - rust-lang#132537 (PassWrapper: adapt for llvm/llvm-project@5445edb5d)
 - rust-lang#132540 (Do not format generic consts)
 - rust-lang#132543 (add and update some crashtests)
 - rust-lang#132544 (Use backticks instead of single quotes for library feature names in diagnostics)
 - rust-lang#132550 (compiler: Continue introducing rustc_abi to the compiler)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 3, 2024
…iaskrgr

Rollup of 15 pull requests

Successful merges:

 - rust-lang#129329 (Implement `From<&mut {slice}>` for `Box/Rc/Arc<{slice}>`)
 - rust-lang#131377 (Add LowerExp and UpperExp implementations to NonZero)
 - rust-lang#132393 (Docs: added brief colon explanation)
 - rust-lang#132437 (coverage: Regression test for inlining into an uninstrumented crate)
 - rust-lang#132499 (unicode_data.rs: show command for generating file)
 - rust-lang#132503 (better test for const HashMap; remove const_hash leftovers)
 - rust-lang#132511 (stabilize const_arguments_as_str)
 - rust-lang#132520 (NFC add known bug nr to test)
 - rust-lang#132522 (make codegen help output more consistent)
 - rust-lang#132523 (Added regression test for generics index out of bounds)
 - rust-lang#132528 (Use `*_opt` typeck results fns to not ICE in fallback suggestion)
 - rust-lang#132537 (PassWrapper: adapt for llvm/llvm-project@5445edb5d)
 - rust-lang#132540 (Do not format generic consts)
 - rust-lang#132543 (add and update some crashtests)
 - rust-lang#132550 (compiler: Continue introducing rustc_abi to the compiler)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b9f9727 into rust-lang:master Nov 3, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 3, 2024
@bors
Copy link
Contributor

bors commented Nov 3, 2024

⌛ Testing commit fead1d5 with merge 7028d93...

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 3, 2024
Rollup merge of rust-lang#131377 - rick-de-water:nonzero-exp, r=dtolnay

Add LowerExp and UpperExp implementations to NonZero

Adds `LowerExp` and `UpperExp` trait implementations to `NonZero`, as discussed in rust-lang/libs-team#458.

I had to modify the macro to mark the new impls with a different rust version. Let me know if this is the right way to do it (first timer here!)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
finished-final-comment-period The final comment period is finished for this PR / Issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. 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. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants