Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Draft RFC to add a
must_not_await
lint #16Draft RFC to add a
must_not_await
lint #16Changes from all commits
6d8aaf1
dac6980
031d93a
c027fb2
50ba6a3
7e0787e
7ce3b3e
0d51148
6b423e0
9b25a4a
82cb98b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 think we should answer this question before stabilizing. If we add this afterwards it'll create unnecessary breakage.
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 think we should include this in this change. In my opinion, this would be the best experience for the user of this attribute.
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.
If we do that it'll be inconsistent with
#[must_use]
. If that's the end goal we want we should explain why they should be different, or discuss the possibility of changing#[must_use]
in the future.Strawman: This is about what types are safe to include (transitively or not) in a Future object.
#[must_use]
is more about how individual values are handled in the code. The state of things today is that new types already have to think about#[must_use]
, but rarely do they think about yielding and async/await, and we shouldn't expect them to.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 was brought up before by @nikomatsakis in an earlier comment. There was a PR which introduced the behavior we are speaking of for #[must_use] but was closed because of crater breakages.
That particular conversation was biased towards having the same behavior across both attributes.
It would certainly increase the scope of this project especially so if it's a breaking change we might have to ship it as a part of the 2021 edition (if it's happening).
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.
Maybe for now we can add an "Unresolved questions" section and add this to it?