Skip to content

A small diagnostic improvement for dropping_copy_types #125433

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 1 commit into from
May 29, 2024

Conversation

surechen
Copy link
Contributor

@surechen surechen commented May 23, 2024

For a value m which implements Copy trait, drop(m); does nothing.
We now suggest user to ignore it by a abstract and general note: let _ = ....
I think we can give a clearer note here: let _ = m;

fixes #125189

@rustbot
Copy link
Collaborator

rustbot commented May 23, 2024

r? @petrochenkov

rustbot has assigned @petrochenkov.
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 23, 2024
@surechen surechen force-pushed the fix_125189 branch 2 times, most recently from a84f56a to 1bba58c Compare May 24, 2024 10:05
@surechen surechen requested a review from Urgau May 24, 2024 10:09
@surechen surechen requested a review from Urgau May 24, 2024 11:41
Copy link
Member

@Urgau Urgau left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks.

(you will have to wait for @compiler-errors for the final approval)

@Urgau
Copy link
Member

Urgau commented May 24, 2024

Btw, the same improvement can also be done for the dropping_references, forgetting_copy_types and forgetting_references lints, they have the same note, which should also be a proper suggestion. (feel free to do them in this PR as separate commits or as a follow-up PR if you like, no obligation to do so).

@surechen
Copy link
Contributor Author

Btw, the same improvement can also be done for the dropping_references, forgetting_copy_types and forgetting_references lints, they have the same note, which should also be a proper suggestion. (feel free to do them in this PR as separate commits or as a follow-up PR if you like, no obligation to do so).

Thank you very much, I tend to implement in follow-up PRs.

@compiler-errors
Copy link
Member

@bors r=Urgau

@bors
Copy link
Collaborator

bors commented May 25, 2024

📌 Commit 09c8e39 has been approved by Urgau

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 May 25, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request May 25, 2024
A small diagnostic improvement for dropping_copy_types

For a value `m`  which implements `Copy` trait, `drop(m);` does nothing.
We now suggest user to ignore it by a abstract and general note: `let _ = ...`.
I think we can give a clearer note here: `let _ = m;`

fixes rust-lang#125189

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->
bors added a commit to rust-lang-ci/rust that referenced this pull request May 25, 2024
…kingjubilee

Rollup of 9 pull requests

Successful merges:

 - rust-lang#124080 (Some unstable changes to where opaque types get defined)
 - rust-lang#125271 (use posix_memalign on almost all Unix targets)
 - rust-lang#125433 (A small diagnostic improvement for dropping_copy_types)
 - rust-lang#125498 (Stop using the avx512er and avx512pf x86 target features)
 - rust-lang#125510 (remove proof tree formatting, make em shallow)
 - rust-lang#125513 (Don't eagerly monomorphize drop for types that are impossible to instantiate)
 - rust-lang#125514 (Structurally resolve before `builtin_index` in EUV)
 - rust-lang#125515 ( bootstrap: support target specific config overrides )
 - rust-lang#125527 (Add manual Sync impl for ReentrantLockGuard)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
A small diagnostic improvement for dropping_copy_types

For a value `m`  which implements `Copy` trait, `drop(m);` does nothing.
We now suggest user to ignore it by a abstract and general note: `let _ = ...`.
I think we can give a clearer note here: `let _ = m;`

fixes rust-lang#125189

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->
@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)
Total Installed Size:  1082.96 MiB

:: Proceed with installation? [Y/n] 
:: Retrieving packages...
error: failed retrieving file 'mingw-w64-x86_64-gcc-objc-13.2.0-3-any.pkg.tar.zst.sig' from mirror.umd.edu : Operation too slow. Less than 1 bytes/sec transferred the last 10 seconds
 mingw-w64-x86_64-gcc-13.2.0-3-any downloading...
error: failed retrieving file 'mingw-w64-x86_64-gcc-13.2.0-3-any.pkg.tar.zst.sig' from mirror.umd.edu : Operation too slow. Less than 1 bytes/sec transferred the last 10 seconds
error: failed retrieving file 'mingw-w64-x86_64-openssl-3.2.0-1-any.pkg.tar.zst' from mirror.msys2.org : Operation too slow. Less than 1 bytes/sec transferred the last 10 seconds
error: failed retrieving file 'mingw-w64-x86_64-gcc-ada-13.2.0-3-any.pkg.tar.zst' from mirror.msys2.org : Operation too slow. Less than 1 bytes/sec transferred the last 10 seconds
warning: too many errors from mirror.msys2.org, skipping for the remainder of this transaction
warning: failed to retrieve some files

@bors
Copy link
Collaborator

bors commented May 26, 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 May 26, 2024
@jieyouxu
Copy link
Member

@bors r=Urgau

@bors
Copy link
Collaborator

bors commented May 26, 2024

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Collaborator

bors commented May 26, 2024

📌 Commit 09c8e39 has been approved by Urgau

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 May 26, 2024
@surechen
Copy link
Contributor Author

Hi, Thanks to the committers who are working hard to merge this PR.
I submitted a PR(rust-lang/cargo#13964) to rust-lang/cargo to try to fix the CI problem.
I think we can try again after that PR is merged into rust-lang/rust/src/tools/cargo

bors added a commit to rust-lang-ci/rust that referenced this pull request May 27, 2024
A small diagnostic improvement for dropping_copy_types

For a value `m`  which implements `Copy` trait, `drop(m);` does nothing.
We now suggest user to ignore it by a abstract and general note: `let _ = ...`.
I think we can give a clearer note here: `let _ = m;`

fixes rust-lang#125189

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->
@bors
Copy link
Collaborator

bors commented May 27, 2024

⌛ Testing commit 09c8e39 with merge f7bb714...

@rust-log-analyzer
Copy link
Collaborator

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

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

@bors
Copy link
Collaborator

bors commented May 27, 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 May 27, 2024
@Urgau
Copy link
Member

Urgau commented May 29, 2024

rust-lang/cargo#13964 was merged in #125682. Let's retry merging this PR.

@bors r=Urgau

@bors
Copy link
Collaborator

bors commented May 29, 2024

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
  • There's another pull request that is currently being tested, blocking this pull request: Rollup of 8 pull requests #125691

@bors
Copy link
Collaborator

bors commented May 29, 2024

📌 Commit 09c8e39 has been approved by Urgau

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 May 29, 2024
@bors
Copy link
Collaborator

bors commented May 29, 2024

⌛ Testing commit 09c8e39 with merge 5870f1c...

@bors
Copy link
Collaborator

bors commented May 29, 2024

☀️ Test successful - checks-actions
Approved by: Urgau
Pushing 5870f1c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 29, 2024
@bors bors merged commit 5870f1c into rust-lang:master May 29, 2024
7 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 29, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5870f1c): 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 (primary 2.4%, secondary -2.8%)

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.4% [2.4%, 2.4%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.8% [-3.8%, -0.7%] 10
All ❌✅ (primary) 2.4% [2.4%, 2.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: 667.785s -> 667.011s (-0.12%)
Artifact size: 318.10 MiB -> 318.10 MiB (-0.00%)

bors added a commit to rust-lang-ci/rust that referenced this pull request May 29, 2024
…ke_drop_lint, r=Urgau

Make lint: `lint_dropping_references` `lint_forgetting_copy_types` `lint_forgetting_references` give suggestion if possible.

This is a follow-up PR of  rust-lang#125433. When it's merged, I want change lint `dropping_copy_types` to use the same `Subdiagnostic` struct `UseLetUnderscoreIgnoreSuggestion` which is added in this PR.

Hi, Thank you(`@Urgau` ) again for your help in the previous PR.  If your time permits, please also take a look at this one.

r? compiler

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->
epage pushed a commit to epage/cargo that referenced this pull request Jun 3, 2024
As lint dropping_copy_types will give suggsetion in this
situation.(Changed in rust-lang/rust#125433)
# for free to join this conversation on GitHub. Already have an account? # 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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing suggestion for dropping_copy_types
10 participants