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.

Sign up for GitHub

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

Jump to bottom

Use generic NonZero internally. #120486

Merged
merged 2 commits into from
Feb 16, 2024
Merged

Use generic NonZero internally. #120486

merged 2 commits into from
Feb 16, 2024

Conversation

reitermarkus
Copy link
Contributor

@reitermarkus reitermarkus commented Jan 30, 2024 •
edited
Loading

Tracking issue: #120257

@rustbot
Copy link
Collaborator

rustbot commented Jan 30, 2024

r? @cuviper

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc O-hermit Operating System: Hermit O-SGX Target: SGX O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface O-wasm Target: WASM (WebAssembly), http://webassembly.org/ O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 30, 2024
@reitermarkus reitermarkus marked this pull request as ready for review January 30, 2024 05:05
@rustbot
Copy link
Collaborator

rustbot commented Jan 30, 2024

The Miri subtree was changed

cc @rust-lang/miri

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino, @ouz-a

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor

oli-obk commented Jan 30, 2024

What's the motivation for this? We can't remove the type specific versions from libstd anyway, so we could just keep using them unless some code needs to be generic over the integer.

@reitermarkus
Copy link
Contributor Author

We can't remove the type specific versions from libstd anyway

Yes, we can't remove it. My assumption is that the type aliases will be deprecated at some point once generic NonZero is stable.

Even if they are not deprecated, the examples in the docs will be changed to show generic NonZero, so it makes sense to me to just use the generic version everywhere.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cuviper
Copy link
Member

cuviper commented Jan 30, 2024

IMO, it's not worth the churn until the generic NonZero is actually stabilized.

And even then, I think it would be better to split the PR across different components for separate review.

@reitermarkus
Copy link
Contributor Author

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 15, 2024
@reitermarkus
Copy link
Contributor Author

@bors r=dtolnay

@bors
Copy link
Contributor

bors commented Feb 15, 2024

📌 Commit a90cc05 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 15, 2024
dtolnay
dtolnay approved these changes Feb 15, 2024
View reviewed changes
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.

Thank you!

Sorry about the 1-week bors queue 🙁 that is unusually long.

oli-obk added a commit to oli-obk/rust that referenced this pull request Feb 15, 2024
@oli-obk
Rollup merge of rust-lang#120486 - reitermarkus:use-generic-nonzero, …
eacf4b7 
…r=dtolnay

Use generic `NonZero` internally.

Tracking issue: rust-lang#120257
@oli-obk oli-obk mentioned this pull request Feb 15, 2024
Closed
@bors
Copy link
Contributor

bors commented Feb 16, 2024

⌛ Testing commit a90cc05 with merge 1be4688...

@bors
Copy link
Contributor

bors commented Feb 16, 2024

☀️ Test successful - checks-actions
Approved by: dtolnay
Pushing 1be4688 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 16, 2024
@bors bors merged commit 1be4688 into rust-lang:master Feb 16, 2024
12 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 16, 2024
This was referenced Feb 16, 2024
Merged
Draft
Merged
Open
Closed
Open
Merged
Closed
Open
Merged
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1be4688): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -1.4% [-1.4%, -1.4%] 1
Improvements ✅
(secondary)		 -2.9% [-2.9%, -2.9%] 1
All ❌✅ (primary) -1.4% [-1.4%, -1.4%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 637.219s -> 637.782s (0.09%)
Artifact size: 306.29 MiB -> 306.29 MiB (-0.00%)

@reitermarkus reitermarkus deleted the use-generic-nonzero branch February 18, 2024 00:18
@dtolnay dtolnay assigned dtolnay and unassigned cuviper Mar 24, 2024
@dtolnay dtolnay mentioned this pull request Aug 16, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtolnay dtolnay dtolnay approved these changes

@cuviper cuviper Awaiting requested review from cuviper

Assignees

@dtolnay dtolnay

Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. O-hermit Operating System: Hermit O-itron Operating System: ITRON O-SGX Target: SGX O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface O-wasm Target: WASM (WebAssembly), http://webassembly.org/ O-windows Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.78.0
Development

Successfully merging this pull request may close these issues.
9 participants
@reitermarkus @rustbot @rust-log-analyzer @oli-obk @cuviper @dtolnay @celinval @bors @rust-timer