add lint futures_not_send #5423
Conversation
|
Thanks for your review @eddyb! |
flip1995
added
the
S-waiting-on-author
|
@eddyb brought up a point on discord: in some cases it may depend on the concrete choice of type parameter whether the resulting Future is Send. This could be checked by bringing After thinking about it for a while, I come to the conclusion that while this is an interesting additional feature, it is outside of the scope I intend for this lint: this lint is meant to support library authors in ascertaining that the Futures returned by their library can successfully be used on a multithreaded executor. This is why the lint is opt-in. If an author opts into this check, they want to ensure that their Futures are Send — and the non-generic ones will need to be Send, as will be the library-supplied components of the generic ones. So I don’t see a plausible use-case for wanting futures to be Send but permitting non-Send type parameters to potentially thwart this effort. The lint can be switched off for individual functions or modules where this is too restrictive. Put differently, the use-case of a library that wants to leave the Send-ness of all its Futures open to the user seems too narrow to me, I can only envision very generic Future transformators to fall into this category. That case will probably be better handled by an explicit compile-time assertion (as is done in many projects already). I’ll finish this PR once rust-lang/rust#70821 has been merged. |
This is a good point and one that I wasn't aware of. |
…xt, r=eddyb expose suggestions::InferCtxtExt for clippy This is very useful to do good async/await diagnostic reporting, for example for rust-lang/rust-clippy#5423.
…xt, r=eddyb expose suggestions::InferCtxtExt for clippy This is very useful to do good async/await diagnostic reporting, for example for rust-lang/rust-clippy#5423.
…xt, r=eddyb expose suggestions::InferCtxtExt for clippy This is very useful to do good async/await diagnostic reporting, for example for rust-lang/rust-clippy#5423.
…xt, r=eddyb expose suggestions::InferCtxtExt for clippy This is very useful to do good async/await diagnostic reporting, for example for rust-lang/rust-clippy#5423.
rkuhn
force-pushed
the
add_futures_not_send
branch
4 times, most recently
from
b4155c6 to
bbc4f5b
Compare
|
@flip1995 I have amended the documentation and updated to the latest rustc master, using the nice async/await error reporting. To my mind it is ready. |
|
☔ The latest upstream changes (presumably #5438) made this pull request unmergeable. Please resolve the merge conflicts. |
flip1995
added
S-waiting-on-review
rkuhn
force-pushed
the
add_futures_not_send
branch
from
bbc4f5b to
2cc432d
Compare
flip1995
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #5470) made this pull request unmergeable. Please resolve the merge conflicts. |
rkuhn
force-pushed
the
add_futures_not_send
branch
from
2cc432d to
4a2b79c
Compare
rkuhn
force-pushed
the
add_futures_not_send
branch
from
4a2b79c to
d2cbbff
Compare
|
done |
|
Thanks! @bors r+ |
|
📌 Commit d2cbbff has been approved by |
add lint futures_not_send changelog: add lint futures_not_send fixes #5379 ~Remark: one thing that can (should?) still be improved is to directly include the error message from the `Send` check so that the programmer stays in the flow. Currently, getting the actual error message requires a restructuring of the code to make the `Send` constraint explicit.~ It now shows all unmet constraints for allowing the Future to be Send.
|
💔 Test failed - checks-action_test |
|
@bors retry |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: add lint futures_not_send
fixes #5379
Remark: one thing that can (should?) still be improved is to directly include the error message from theSendcheck so that the programmer stays in the flow. Currently, getting the actual error message requires a restructuring of the code to make theSendconstraint explicit.It now shows all unmet constraints for allowing the Future to be Send.