Skip to content

Conversation

reitermarkus
Copy link
Contributor

Tracking issue: #120257

Depends on #120521.

r? @dtolnay

@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 Feb 1, 2024
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.

I reviewed the 3rd commit only (the first 2 are identical to #120521 based on commit hash).

Looks good!

#119452 added codegen tests (@AngelicosPhosphoros thank you 👏 👏) so as long as those are good, I am happy with this PR.

@dtolnay
Copy link
Member

dtolnay commented Feb 2, 2024

@bors delegate+

Please rebase after #120521 lands, so we see just get changes in https://github.com/rust-lang/rust/pull/120563/files, and then approve this yourself. Thanks!

After #120521 is merged and then #120486 is rebased and merged, regardless of whether this PR has been merged by that point, I plan to propose stabilization on #120257. Between the time for the team to approve the stabilization, and land a stabilization PR, and then the nightly and beta period, it will give enough time in my opinion to bail out if anything appears to be going wrong with generic nonzero.

@bors
Copy link
Collaborator

bors commented Feb 2, 2024

✌️ @reitermarkus, you can now approve this pull request!

If @dtolnay told you to "r=me" after making some further change, please make that change, then do @bors r=@dtolnay

@reitermarkus
Copy link
Contributor Author

#119452 added codegen tests

Yes, I specifically ran these tests locally, so hopefully should be fine.

@bors
Copy link
Collaborator

bors commented Feb 8, 2024

☔ The latest upstream changes (presumably #120521) made this pull request unmergeable. Please resolve the merge conflicts.

@reitermarkus
Copy link
Contributor Author

@bors r=dtolnay

@bors
Copy link
Collaborator

bors commented Feb 8, 2024

📌 Commit 24e2cf0 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 Feb 8, 2024
oli-obk added a commit to oli-obk/rust that referenced this pull request Feb 13, 2024
…r=dtolnay

Make `NonZero::get` generic.

Tracking issue: rust-lang#120257

Depends on rust-lang#120521.

r? `@dtolnay`
oli-obk added a commit to oli-obk/rust that referenced this pull request Feb 13, 2024
…r=dtolnay

Make `NonZero::get` generic.

Tracking issue: rust-lang#120257

Depends on rust-lang#120521.

r? ``@dtolnay``
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2024
Rollup of 6 pull requests

Successful merges:

 - rust-lang#120486 (Use generic `NonZero` internally.)
 - rust-lang#120500 (Implement intrinsics with fallback bodies)
 - rust-lang#120530 (Be less confident when `dyn` suggestion is not checked for object safety)
 - rust-lang#120563 (Make `NonZero::get` generic.)
 - rust-lang#120847 (Continue compilation after check_mod_type_wf errors)
 - rust-lang#120959 (Remove good path delayed bugs)

r? `@ghost`
`@rustbot` modify labels: rollup
oli-obk added a commit to oli-obk/rust that referenced this pull request Feb 15, 2024
…r=dtolnay

Make `NonZero::get` generic.

Tracking issue: rust-lang#120257

Depends on rust-lang#120521.

r? ```@dtolnay```
@bors
Copy link
Collaborator

bors commented Feb 16, 2024

⌛ Testing commit 24e2cf0 with merge 439d69c...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 16, 2024
…dtolnay

Make `NonZero::get` generic.

Tracking issue: rust-lang#120257

Depends on rust-lang#120521.

r? `@dtolnay`
@bors
Copy link
Collaborator

bors commented Feb 17, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 17, 2024
@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   Compiling mdbook v0.4.37
[RUSTC-TIMING] elasticlunr test:false 5.487
[RUSTC-TIMING] ammonia test:false 5.021
[RUSTC-TIMING] handlebars test:false 10.790
##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.
##[group]Clock drift check
  local time: Sat Feb 17 00:01:58 UTC 2024
  network time: Sat, 17 Feb 2024 00:01:58 GMT
##[endgroup]
##[endgroup]
Session terminated, killing shell... ...killed.
##[error]The operation was canceled.

@reitermarkus reitermarkus requested a review from dtolnay February 17, 2024 00:50
@dtolnay
Copy link
Member

dtolnay commented Feb 17, 2024

Can't tell what failed.

@bors retry
@bors delegate+

@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 Feb 17, 2024
@bors
Copy link
Collaborator

bors commented Feb 17, 2024

✌️ @reitermarkus, you can now approve this pull request!

If @dtolnay told you to "r=me" after making some further change, please make that change, then do @bors r=@dtolnay

@bors
Copy link
Collaborator

bors commented Feb 17, 2024

⌛ Testing commit 24e2cf0 with merge 4316d0c...

@bors
Copy link
Collaborator

bors commented Feb 17, 2024

☀️ Test successful - checks-actions
Approved by: dtolnay
Pushing 4316d0c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 17, 2024
@bors bors merged commit 4316d0c into rust-lang:master Feb 17, 2024
@rustbot rustbot added this to the 1.78.0 milestone Feb 17, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4316d0c): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

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)
-5.1% [-8.0%, -2.3%] 4
Improvements ✅
(secondary)
-4.5% [-5.1%, -3.8%] 2
All ❌✅ (primary) -5.1% [-8.0%, -2.3%] 4

Cycles

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)
- - 0
Improvements ✅
(secondary)
-8.6% [-8.6%, -8.6%] 1
All ❌✅ (primary) - - 0

Binary size

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.0% [0.0%, 0.2%] 20
Regressions ❌
(secondary)
0.9% [0.8%, 0.9%] 3
Improvements ✅
(primary)
-0.3% [-2.4%, -0.1%] 14
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-2.4%, 0.2%] 34

Bootstrap: 638.214s -> 639.28s (0.17%)
Artifact size: 306.34 MiB -> 306.36 MiB (0.00%)

@reitermarkus reitermarkus deleted the generic-nonzero-get branch February 17, 2024 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants