Fix suggestion from incorrect move async to async move.
#63699
Conversation
|
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @estebank or @petrochenkov |
This comment has been minimized.
This comment has been minimized.
gilescope
force-pushed
the
async-move-diagnostic
branch
from
9b5b6bf to
ef3e66d
Compare
|
@bors r+ rollup |
|
📌 Commit ef3e66d has been approved by |
bors
added
S-waiting-on-bors
…estebank Fix suggestion from incorrect `move async` to `async move`. PR for rust-lang#61920. Happy with the test. There must be a better implementation though - possibly a MIR visitor to estabilsh a span that doesn't include the `async` keyword?
Rollup of 5 pull requests Successful merges: - #63252 (Remove recommendation about idiomatic syntax for Arc::clone) - #63376 (use different lifetime name for object-lifetime-default elision) - #63620 (Use constraint span when lowering associated types) - #63699 (Fix suggestion from incorrect `move async` to `async move`.) - #63704 ( Fixed: error: unnecessary trailing semicolon) Failed merges: r? @ghost
|
☔ The latest upstream changes (presumably #63715) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
| @@ -1190,7 +1190,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { | |||
| ); | |||
|
|
|||
| let suggestion = match tcx.sess.source_map().span_to_snippet(args_span) { | |||
| Ok(string) => format!("move {}", string), | |||
| Ok(mut string) => { | |||
| if string.starts_with("async ") { | |||
There was a problem hiding this comment.
This is a rather error-prone way of checking this that will fail with:
async
|x| {}or
async\t|x|I don't think this should be looking at the code snippet but rather trace with self.tcx whether this closure was an async one before lowering. I sadly cannot tell you off the top of my head how to do it and cannot look at the code at this very moment but maybe someone else can chim in. @estebank?
There was a problem hiding this comment.
That is true. Ideally you'd want to get the original node, but I don't think you can. An alternative that keeps operating on the string (less than ideal, as already established) would be to use string.split_whitespace().next() which would yield Some("async") for the cases we care about in a pretty foolproof way.
There was a problem hiding this comment.
String operations for these checks is not ideal, but sometimes we have no way around them unless we track more things from the AST onwards.
PR for #61920. Happy with the test. There must be a better implementation though - possibly a MIR visitor to estabilsh a span that doesn't include the
asynckeyword?