Skip to content

Enhance drop forget ref hint#156395

Open
Kcang-gna wants to merge 2 commits into
rust-lang:mainfrom
Kcang-gna:enhance_drop_forget_ref_hint
Open

Enhance drop forget ref hint#156395
Kcang-gna wants to merge 2 commits into
rust-lang:mainfrom
Kcang-gna:enhance_drop_forget_ref_hint

Conversation

@Kcang-gna
Copy link
Copy Markdown
Contributor

@Kcang-gna Kcang-gna commented May 10, 2026

Replace let _ = ... with let _ = drop_arg
Changed some hint messages

Fixes: #155895

@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 10, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 10, 2026

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 18 candidates

@Urgau
Copy link
Copy Markdown
Member

Urgau commented May 10, 2026

As I said in #155895 (comment), I don't think we should do that, being a bit vague was on purpose.

Further-more the suggestion is not right when there is an explicit borrow, like in https://github.com/rust-lang/rust/pull/156395/changes#diff-caa7b472da1b6aa2b42ea519f4bdd7eef6c5c031a32a342110116db9b83dde97R14.

@rust-log-analyzer
Copy link
Copy Markdown
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
Executing "/scripts/stage_2_test_set1.sh"
+ /scripts/stage_2_test_set1.sh
PR_CI_JOB set; skipping tidy
+ '[' 1 == 1 ']'
+ echo 'PR_CI_JOB set; skipping tidy'
+ SKIP_TIDY='--skip tidy'
+ ../x.py --stage 2 test --skip tidy --skip compiler --skip src
##[group]Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
downloading https://static.rust-lang.org/dist/2026-04-14/rustfmt-nightly-aarch64-unknown-linux-gnu.tar.xz
---
6    |                    |
7    |                    argument has type `i32`
8    |
-    = note: use `let _ = ...` to ignore the expression or result
+    = note: use `let _ = 5 % 3` simply to ignore the expression or result
10    = note: `#[warn(dropping_copy_types)]` on by default
11 
12 warning: 1 warning emitted


The actual stderr differed from the expected stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args binop/can-have-side-effects-consider-operands.rs`

error: 1 errors occurred comparing output.
status: exit status: 0
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/binop/can-have-side-effects-consider-operands.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/binop/can-have-side-effects-consider-operands" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "incomplete_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers"
stdout: none
--- stderr -------------------------------
warning: calls to `std::mem::drop` with a value that implements `Copy` does nothing
##[warning]  --> /checkout/tests/ui/binop/can-have-side-effects-consider-operands.rs:12:15
   |
LL |         () => drop(5 % 3), // No side effects
   |               ^^^^^-----^
   |                    |
   |                    argument has type `i32`
   |
   = note: use `let _ = 5 % 3` simply to ignore the expression or result
   = note: `#[warn(dropping_copy_types)]` on by default

warning: 1 warning emitted
------------------------------------------

---
6    |                    |
7    |                    argument has type `[i32; 1]`
8    |
-    = note: use `let _ = ...` to ignore the expression or result
+    = note: use `let _ = [0; 1]` simply to ignore the expression or result
10    = note: `#[warn(dropping_copy_types)]` on by default
11 
12 warning: 1 warning emitted


The actual stderr differed from the expected stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args repeat-expr/can-have-side-effects-consider-element.rs`

error: 1 errors occurred comparing output.
status: exit status: 0
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/repeat-expr/can-have-side-effects-consider-element.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/repeat-expr/can-have-side-effects-consider-element" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "incomplete_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers"
stdout: none
--- stderr -------------------------------
warning: calls to `std::mem::drop` with a value that implements `Copy` does nothing
##[warning]  --> /checkout/tests/ui/repeat-expr/can-have-side-effects-consider-element.rs:12:15
   |
LL |         () => drop([0; 1]), // No side effects
   |               ^^^^^------^
   |                    |
   |                    argument has type `[i32; 1]`
   |
   = note: use `let _ = [0; 1]` simply to ignore the expression or result
   = note: `#[warn(dropping_copy_types)]` on by default

warning: 1 warning emitted
------------------------------------------

@Kcang-gna
Copy link
Copy Markdown
Contributor Author

Why is let _ = &x regarded as incorrect?

I don’t quite understand from what angle this way of writing is considered incorrect. From my perspective, this syntax either brings about side effects or is used to disable the "unused variable" lint, and its effect seems roughly equivalent to drop(&x).

However, developers may mistakenly assume that drop(&x)has some practical effect. It would be better to directly prompt developers to remove the drop call through code hints, along with prompt messages. This can better eliminate developers’ misunderstandings, make them realize this approach does not work, and that they need to adopt another method to achieve their goal.

Therefore in my opinion, if drop(&x); is highly discouraged, then let _ = &x; can only be classified as not recommended.What’s more, it is simply to disable the "unused variable" lint.

(Translated by LLM)

@Urgau
Copy link
Copy Markdown
Member

Urgau commented May 11, 2026

Why is let _ = &x regarded as incorrect?

Same as drop(&x), it does nothing.

If we are at a point where we are suggesting something to the user, it would be better to suggest code that actually does something.

@Kcang-gna
Copy link
Copy Markdown
Contributor Author

Ok, I got it

Maybe we can give the user links to primitive reference and std::mem::drop as a note, because users only try to drop references due to a lack of relevant knowledge. keep let _ = ... unchanged

@Urgau
Copy link
Copy Markdown
Member

Urgau commented May 11, 2026

Sure, if you think linking to std::mem::drop improves things I'm fine with that.

I'm not sure we add a link to the reference type. If the goal was to make it clear that a reference is Copy, I think it would be better to update std::mem::drop documentation to make that clearer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

incorrect/useless lint warning [warn(dropping_references)]

5 participants