#53840: Consolidate pattern check errors #54269
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @estebank |
There was a problem hiding this comment.
Nitpick, no need to change: I prefer to include a description in git commits that can be understood without having to check the issue tracker, something along the lines of the PR description you used is good. Then in the description of the commit or at least the PR add a mention of the issue being fixed.
Other than me not knowing why a new block is being added, pretty cool change.
Please add to the test a case using pattern destructuring with a struct variant (Bar { a, b, ref c}).
r=me after addressing comments.
| @@ -527,53 +527,61 @@ fn check_legality_of_move_bindings(cx: &MatchVisitor, | |||
| } | |||
| }) | |||
| } | |||
| let span_vec = &mut Vec::new(); | |||
| { | |||
There was a problem hiding this comment.
I'm not sure I get why you need a new scope here.
There was a problem hiding this comment.
above point regarding commit, I will ensure I follow that in future :) Agreed, its best practice to mention everything in the commit, so, if someone backtracks and checks those commit should get the idea of what is being done without going to issue tracker :)
There was a problem hiding this comment.
@estebank now instead of creating a new scope, I am passing span_vec to check_move closure, this is a much clearer approach. Thanks for your feedback :)
estebank
added
S-waiting-on-author
| span, | ||
| E0009, | ||
| "cannot bind by-move and by-ref in the same pattern", | ||
| ); |
There was a problem hiding this comment.
Nitpick: the appropriate indentation for this macro call is:
let mut err = struct_span_err!(
cx.tcx.sess,
span,
E0009,
"cannot bind by-move and by-ref in the same pattern",
);
This comment has been minimized.
This comment has been minimized.
|
Ping from triage @PramodBisht: It looks like some changes have been requested to your PR and it also needs to be rebased. |
we are consolidating `cannot bind by-move and by-ref in the same pattern` message present on the different lines into single diagnostic message. To do this, we are first gathering those spans into the vector after that we are throwing them with the help of MultiSpan in a separate block. Addresses: rust-lang#53840
PramodBisht
force-pushed
the
issue/53840
branch
from
68aedda to
e536e64
Compare
|
r? @estebank again |
|
@bors r+ rollup |
|
📌 Commit e536e64 has been approved by |
bors
added
S-waiting-on-bors
rust-lang#53840: Consolidate pattern check errors rust-lang#53840 on this PR we are aggregating `cannot bind by-move and by-ref in the same pattern` message present on the different lines into one diagnostic message. Here we are first gathering those `spans` on `vector` then we are throwing them with the help of `MultiSpan` r? @estebank Addresses: rust-lang#53480
rust-lang#53840: Consolidate pattern check errors rust-lang#53840 on this PR we are aggregating `cannot bind by-move and by-ref in the same pattern` message present on the different lines into one diagnostic message. Here we are first gathering those `spans` on `vector` then we are throwing them with the help of `MultiSpan` r? @estebank Addresses: rust-lang#53480
Rollup of 10 pull requests Successful merges: - #54269 (#53840: Consolidate pattern check errors) - #54458 (Allow both explicit and elided lifetimes in the same impl header) - #54603 (Add `crate::` to trait suggestions in Rust 2018.) - #54648 (Update Cargo's submodule) - #54680 (make run-pass tests with empty main just compile-pass tests) - #54687 (Use impl_header_lifetime_elision in libcore) - #54699 (Re-export `getopts` so custom drivers can reference it.) - #54702 (do not promote comparing function pointers) - #54728 (Renumber `proc_macro` tracking issues) - #54745 (make `CStr::from_bytes_with_nul_unchecked()` a const fn) Failed merges: r? @ghost
#53840 on this PR we are aggregating
cannot bind by-move and by-ref in the same patternmessage present on the different lines into one diagnostic message. Here we are first gathering thosespansonvectorthen we are throwing them with the help ofMultiSpanr? @estebank
Addresses: #53480