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

feat: implement {Div,Rem}Assign<NonZero<X>> on X #121952

Merged
merged 3 commits into from
Mar 18, 2024

Conversation

JarvisCraft
Copy link
Contributor

Description

This PR implements DivAssign<X> and RemAssign<X> on X as suggested in rust-lang/libs-team#346.

Since this is just a trait implementation on an already stable type, for which non-assign operator traits are already stable, I suggest that it is an insta-stable feature.

@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2024

r? @workingjubilee

rustbot has assigned @workingjubilee.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Mar 3, 2024
@JarvisCraft JarvisCraft force-pushed the master branch 2 times, most recently from c3d7969 to f0fc331 Compare March 3, 2024 21:16
@workingjubilee workingjubilee added S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 3, 2024
@joshtriplett
Copy link
Member

Since these are insta-stable, the ACP alone doesn't suffice, so starting an FCP:

@rfcbot merge

@rfcbot

This comment was marked as off-topic.

@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 Mar 5, 2024
@m-ou-se m-ou-se added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 5, 2024
@m-ou-se

This comment was marked as resolved.

@rfcbot

This comment was marked as resolved.

@rfcbot rfcbot 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. labels Mar 5, 2024
@m-ou-se

This comment was marked as resolved.

@rfcbot
Copy link

rfcbot commented Mar 5, 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 Mar 5, 2024
@m-ou-se m-ou-se added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. labels Mar 5, 2024
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 5, 2024
@rfcbot
Copy link

rfcbot commented Mar 5, 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 Mar 15, 2024
@workingjubilee
Copy link
Contributor

r=me with the doc fixup.

@bors delegate

@bors
Copy link
Contributor

bors commented Mar 16, 2024

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

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

@workingjubilee workingjubilee 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-fcp Status: PR is in FCP and is awaiting for FCP to complete. labels Mar 16, 2024
@bors
Copy link
Contributor

bors commented Mar 17, 2024

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

These were only relevant for the unsafe-containing implementations

Signed-off-by: Petr Portnov <[email protected]>
@JarvisCraft
Copy link
Contributor Author

Thanks for your hint, I've updated the PR by echoing the comments and rebasing on master.

@bors r=@workingjubilee

@bors
Copy link
Contributor

bors commented Mar 17, 2024

📌 Commit e7d3970 has been approved by workingjubilee

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 Mar 17, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 17, 2024
feat: implement `{Div,Rem}Assign<NonZero<X>>` on `X`

# Description

This PR implements `DivAssign<X>` and `RemAssign<X>` on `X` as suggested in rust-lang/libs-team#346.

Since this is just a trait implementation on an already stable type, for which non-assign operator traits are already stable, I suggest that it is an insta-stable feature.
@bors
Copy link
Contributor

bors commented Mar 17, 2024

⌛ Testing commit e7d3970 with merge a83caa2...

@bors
Copy link
Contributor

bors commented Mar 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 Mar 17, 2024
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@workingjubilee
Copy link
Contributor

Seems spurious?

@bors retry

@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 Mar 17, 2024
@bors
Copy link
Contributor

bors commented Mar 18, 2024

⌛ Testing commit e7d3970 with merge 80e5694...

@bors
Copy link
Contributor

bors commented Mar 18, 2024

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing 80e5694 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 18, 2024
@bors bors merged commit 80e5694 into rust-lang:master Mar 18, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 18, 2024
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
##[endgroup]
Starting download for Cargo-lock
Directory structure has been setup for the artifact
Total number of files that will be downloaded: 1
Artifact Cargo-lock was downloaded to /home/runner/work/rust/rust
##[group]Run actions/download-artifact@v3
with:
  name: cargo-updates
---
##[endgroup]
Starting download for cargo-updates
Directory structure has been setup for the artifact
Total number of files that will be downloaded: 1
Skipping download validation.
Artifact cargo-updates was downloaded to /home/runner/work/rust/rust
##[group]Run echo "${COMMIT_MESSAGE}" > commit.txt
echo "${COMMIT_MESSAGE}" > commit.txt
echo "${COMMIT_MESSAGE}" > commit.txt
cat cargo_update.log >> commit.txt

echo "${PR_MESSAGE}" > body.md
echo '```txt' >> body.md
cat cargo_update.log >> body.md
echo '```' >> body.md
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
  RUSTC_BOOTSTRAP: 1
  PR_TITLE: Weekly `cargo update`
  PR_MESSAGE: Automation to keep dependencies in `Cargo.lock` current.
following is the output from `cargo update`:
  COMMIT_MESSAGE: cargo update 
##[endgroup]
##[group]Run git config user.name github-actions
git config user.name github-actions
git config user.email [email protected]
git switch --force-create cargo_update
git add ./Cargo.lock
git commit --no-verify --file=commit.txt
---
##[endgroup]
Switched to a new branch 'cargo_update'
On branch cargo_update
Untracked files:
  (use "git add <file>..." to include in what will be committed)
 body.md
 commit.txt


nothing added to commit but untracked files present (use "git add" to track)
##[error]Process completed with exit code 1.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (80e5694): 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)
2.3% [2.2%, 2.4%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-3.4% [-3.4%, -3.4%] 1
Improvements ✅
(secondary)
-2.4% [-3.3%, -0.6%] 6
All ❌✅ (primary) 0.4% [-3.4%, 2.4%] 3

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)
-5.8% [-11.4%, -1.4%] 7
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 668.349s -> 668.534s (0.03%)
Artifact size: 312.73 MiB -> 312.79 MiB (0.02%)

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants