-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix diagnostics for async block cloning #122589
Conversation
capture_kind_span: _, | ||
path_span, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what is a more fitting span, whether the path_span
or the capture_kind_span
. Both seem to be working correctly with the provided test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fine, but I kind of wish that the message noted that the semantics change due to the clone.
LL | | }.await | ||
| |_________^ value moved here, in previous iteration of loop | ||
| | ||
help: consider cloning the value if the performance cost is acceptable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this error message is not right in spirit. Really we should be saying something like "cloning the value [if this is the behavior that you intended]".
Cloning here could have different semantics than actually moving the value into the async block, for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make sure, are you suggesting we should change the help message in general for cloning or only for this particular case in the async block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't actually know :/ I guess this is a more general problem than what this aims to fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I suggest creating an issue where we can talk about the appropriate wording? I would like to also share my opinion, however, I think this is not the right PR for that.
☔ The latest upstream changes (presumably #121652) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work for cases like:
let x = async {
drop(value);
}.await;
use(value);
Outside of a loop?
FYI: due to the #121652, the suggestion is now (on nightly) even more funnier 😀
|
nightly:
after my patch:
I guess it works even for this case. |
☔ The latest upstream changes (presumably #122947) made this pull request unmergeable. Please resolve the merge conflicts. |
@bors r+ |
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#108675 (Document `adt_const_params` feature in Unstable Book) - rust-lang#122120 (Suggest associated type bounds on problematic associated equality bounds) - rust-lang#122589 (Fix diagnostics for async block cloning) - rust-lang#122835 (Require `DerefMut` and `DerefPure` on `deref!()` patterns when appropriate) - rust-lang#123049 (In `ConstructCoroutineInClosureShim`, pass receiver by mut ref, not mut pointer) - rust-lang#123055 (enable cargo miri test doctests) - rust-lang#123057 (unix fs: Make hurd using explicit new rather than From) - rust-lang#123087 (Change `f16` and `f128` clippy stubs to be nonpanicking) - rust-lang#123103 (Rename `Inherited` -> `TypeckRootCtxt`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#122589 - wutchzone:121547, r=compiler-errors Fix diagnostics for async block cloning Closes rust-lang#121547 r? diagnostics
Closes #121547
r? diagnostics