error-msg: expand suggestion for unused_def lint
#109158
Conversation
|
r? @oli-obk (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
Ezrashaw
force-pushed
the
expand-sugg-for-unused-lint
branch
from
629527c
to
35103fe
Compare
|
r? @Nilstrieb |
| @@ -440,7 +432,8 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults { | |||
| cx, | |||
| def_id: *def_id, | |||
| note: *reason, | |||
| suggestion, | |||
| suggestion: (!is_inner) | |||
There was a problem hiding this comment.
Why only for the outermost one and not nested ones as well?
There was a problem hiding this comment.
let-statements can't exist in the expression position. We don't want (get_result(), other_fn(), ...) to become (let _ = get_result(), other_fn(), ...)
| @@ -1390,7 +1390,7 @@ pub struct UnusedOp<'a> { | |||
| pub op: &'a str, | |||
| #[label] | |||
| pub label: Span, | |||
| #[suggestion(style = "verbose", code = "let _ = ", applicability = "machine-applicable")] | |||
There was a problem hiding this comment.
Maybe we could suggest using
_ = ...;Instead? It silences the warning and looks a bit nicer & shorter IMO.
There was a problem hiding this comment.
I disagree, let _ is more widely used and understood and also nicer imo.
There was a problem hiding this comment.
Why is it more idiomatic? I think the only reason let _ is used more, is that it was available since forever, while _ was added after that.
There was a problem hiding this comment.
Also, keep in mind that it is about more than just ignoring a value; you might want to bind it to a variable. Removing let preemptively makes using the value as opposed to ignoring it harder.
Also I'd like to keep the let anyway because it's much more common and IMHO conveys intent better.
|
I don't exactly love the structured suggestion (and would prefer a @bors r+ Also the error reporting code of this lint is a huge mess and I'd love to see it improved (after I split it out from the checking phase some time ago) |
bors
added
S-waiting-on-bors
Might work on this then! :) |
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#108875 (rustdoc: fix type search for `Option` combinators) - rust-lang#108971 (error-msg: impl better suggestion for `E0532`) - rust-lang#109139 (rustdoc: DocFS: Replace rayon with threadpool and enable it for all targets) - rust-lang#109151 (Assert def-kind is correct for alias types) - rust-lang#109158 (error-msg: expand suggestion for `unused_def` lint) - rust-lang#109166 (make `define_opaque_types` fully explicit) - rust-lang#109171 (Some cleanups in our normalization logic) - rust-lang#109180 (Unequal → Not equal) - rust-lang#109185 (rustdoc: remove `std::` from primitive intra-doc link tooltips) - rust-lang#109192 (Mention UEFI target promotion in release notes for 1.67.0) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #108885
Expands
let _ = ..suggestion into more positions.