Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-24 21:53:23 +00:00
Code Issues Projects Releases Packages Wiki Activity

Allow conditional Send futures in future_not_send (#13590)

Browse source 
Closes #6947

This changes the lint to allow futures which are not `Send` as a result
of a generic type parameter not having a `Send` bound and only lint
futures that are always `!Send` for any type, which I believe is the
more useful behavior (like the comments in the linked issue explain).

This is still only a heuristic (I'm not sure if there's a more general
way to do this), but it should cover the common cases I could think of
(including the code examples in the linked issue)

changelog: [`future_not_send`]: allow conditional `Send` futures
This commit is contained in:
Alex Macleod 2024-11-15 19:23:58 +00:00 committed by GitHub
commit 83f7526cf0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 101 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

67
clippy_lints/src/future_not_send.rs
View file

@ -1,3 +1,5 @@
use std::ops::ControlFlow;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::return_ty;
use rustc_hir::intravisit::FnKind;
@ -5,7 +7,9 @@ use rustc_hir::{Body, FnDecl};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::print::PrintTraitRefExt;
use rustc_middle::ty::{self, AliasTy, ClauseKind, PredicateKind};
use rustc_middle::ty::{
self, AliasTy, Binder, ClauseKind, PredicateKind, Ty, TyCtxt, TypeVisitable, TypeVisitableExt, TypeVisitor,
};
use rustc_session::declare_lint_pass;
use rustc_span::def_id::LocalDefId;
use rustc_span::{Span, sym};
@ -15,9 +19,16 @@ use rustc_trait_selection::traits::{self, FulfillmentError, ObligationCtxt};
declare_clippy_lint! {
/// ### What it does
/// This lint requires Future implementations returned from
/// functions and methods to implement the `Send` marker trait. It is mostly
/// used by library authors (public and internal) that target an audience where
/// multithreaded executors are likely to be used for running these Futures.
/// functions and methods to implement the `Send` marker trait,
/// ignoring type parameters.
///
/// If a function is generic and its Future conditionally implements `Send`
/// based on a generic parameter then it is considered `Send` and no warning is emitted.
///
/// This can be used by library authors (public and internal) to ensure
/// their functions are compatible with both multi-threaded runtimes that require `Send` futures,
/// as well as single-threaded runtimes where callers may choose `!Send` types
/// for generic parameters.
///
/// ### Why is this bad?
/// A Future implementation captures some state that it
@ -64,22 +75,46 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend {
return;
}
let ret_ty = return_ty(cx, cx.tcx.local_def_id_to_hir_id(fn_def_id).expect_owner());
if let ty::Alias(ty::Opaque, AliasTy { def_id, args, .. }) = *ret_ty.kind() {
if let ty::Alias(ty::Opaque, AliasTy { def_id, args, .. }) = *ret_ty.kind()
&& let Some(future_trait) = cx.tcx.lang_items().future_trait()
&& let Some(send_trait) = cx.tcx.get_diagnostic_item(sym::Send)
{
let preds = cx.tcx.explicit_item_super_predicates(def_id);
let is_future = preds.iter_instantiated_copied(cx.tcx, args).any(|(p, _)| {
p.as_trait_clause().is_some_and(|trait_pred| {
Some(trait_pred.skip_binder().trait_ref.def_id) == cx.tcx.lang_items().future_trait()
})
p.as_trait_clause()
.is_some_and(|trait_pred| trait_pred.skip_binder().trait_ref.def_id == future_trait)
});
if is_future {
let send_trait = cx.tcx.get_diagnostic_item(sym::Send).unwrap();
let span = decl.output.span();
let infcx = cx.tcx.infer_ctxt().build(cx.typing_mode());
let ocx = ObligationCtxt::new_with_diagnostics(&infcx);
let cause = traits::ObligationCause::misc(span, fn_def_id);
ocx.register_bound(cause, cx.param_env, ret_ty, send_trait);
let send_errors = ocx.select_all_or_error();
if !send_errors.is_empty() {
// Allow errors that try to prove `Send` for types that "mention" a generic parameter at the "top
// level".
// For example, allow errors that `T: Send` can't be proven, but reject `Rc<T>: Send` errors,
// which is always unconditionally `!Send` for any possible type `T`.
//
// We also allow associated type projections if the self type is either itself a projection or a
// type parameter.
// This is to prevent emitting warnings for e.g. holding a `<Fut as Future>::Output` across await
// points, where `Fut` is a type parameter.
let is_send = send_errors.iter().all(|err| {
err.obligation
.predicate
.as_trait_clause()
.map(Binder::skip_binder)
.is_some_and(|pred| {
pred.def_id() == send_trait
&& pred.self_ty().has_param()
&& TyParamAtTopLevelVisitor.visit_ty(pred.self_ty()) == ControlFlow::Break(true)
})
});
if !is_send {
span_lint_and_then(
cx,
FUTURE_NOT_SEND,
@ -107,3 +142,15 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend {
}
}
}
struct TyParamAtTopLevelVisitor;
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for TyParamAtTopLevelVisitor {
type Result = ControlFlow<bool>;
fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result {
match ty.kind() {
ty::Param(_) => ControlFlow::Break(true),
ty::Alias(ty::AliasTyKind::Projection, ty) => ty.visit_with(self),
_ => ControlFlow::Break(false),
}
}
}

7
tests/ui/crashes/ice-10645.rs
View file

@ -1,7 +0,0 @@
//@compile-flags: --cap-lints=warn
// https://github.com/rust-lang/rust-clippy/issues/10645
#![warn(clippy::future_not_send)]
pub async fn bar<'a, T: 'a>(_: T) {}
fn main() {}

17
tests/ui/crashes/ice-10645.stderr
View file

@ -1,17 +0,0 @@
warning: future cannot be sent between threads safely
--> tests/ui/crashes/ice-10645.rs:5:1
|
LL | pub async fn bar<'a, T: 'a>(_: T) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `bar` is not `Send`
|
note: captured value is not `Send`
--> tests/ui/crashes/ice-10645.rs:5:29
|
LL | pub async fn bar<'a, T: 'a>(_: T) {}
| ^ has type `T` which is not `Send`
= note: `T` doesn't implement `std::marker::Send`
= note: `-D clippy::future-not-send` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::future_not_send)]`
warning: 1 warning emitted

18
tests/ui/future_not_send.rs
View file

@ -1,6 +1,7 @@
#![warn(clippy::future_not_send)]
use std::cell::Cell;
use std::future::Future;
use std::rc::Rc;
use std::sync::Arc;
@ -63,6 +64,22 @@ where
t
}
async fn maybe_send_generic_future<T>(t: T) -> T {
async { true }.await;
t
}
async fn maybe_send_generic_future2<F: Fn() -> Fut, Fut: Future>(f: F) {
async { true }.await;
let res = f();
async { true }.await;
}
async fn generic_future_always_unsend<T>(_: Rc<T>) {
//~^ ERROR: future cannot be sent between threads safely
async { true }.await;
}
async fn generic_future_send<T>(t: T)
where
T: Send,
@ -71,7 +88,6 @@ where
}
async fn unclear_future<T>(t: T) {}
//~^ ERROR: future cannot be sent between threads safely
fn main() {
let rc = Rc::new([1, 2, 3]);

51
tests/ui/future_not_send.stderr
View file

@ -1,11 +1,11 @@
error: future cannot be sent between threads safely
--> tests/ui/future_not_send.rs:7:1
--> tests/ui/future_not_send.rs:8:1
|
LL | async fn private_future(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `private_future` is not `Send`
|
note: future is not `Send` as this value is used across an await
--> tests/ui/future_not_send.rs:9:20
--> tests/ui/future_not_send.rs:10:20
|
LL | async fn private_future(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool {
| -- has type `std::rc::Rc<[u8]>` which is not `Send`
@ -14,7 +14,7 @@ LL | async { true }.await
| ^^^^^ await occurs here, with `rc` maybe used later
= note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Send`
note: captured value is not `Send` because `&` references cannot be sent unless their referent is `Sync`
--> tests/ui/future_not_send.rs:7:39
--> tests/ui/future_not_send.rs:8:39
|
LL | async fn private_future(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool {
| ^^^^ has type `&std::cell::Cell<usize>` which is not `Send`, because `std::cell::Cell<usize>` is not `Sync`
@ -23,13 +23,13 @@ LL | async fn private_future(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool {
= help: to override `-D warnings` add `#[allow(clippy::future_not_send)]`
error: future cannot be sent between threads safely
--> tests/ui/future_not_send.rs:12:1
--> tests/ui/future_not_send.rs:13:1
|
LL | pub async fn public_future(rc: Rc<[u8]>) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `public_future` is not `Send`
|
note: future is not `Send` as this value is used across an await
--> tests/ui/future_not_send.rs:14:20
--> tests/ui/future_not_send.rs:15:20
|
LL | pub async fn public_future(rc: Rc<[u8]>) {
| -- has type `std::rc::Rc<[u8]>` which is not `Send`
@ -39,45 +39,45 @@ LL | async { true }.await;
= note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Send`
error: future cannot be sent between threads safely
--> tests/ui/future_not_send.rs:21:1
--> tests/ui/future_not_send.rs:22:1
|
LL | async fn private_future2(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `private_future2` is not `Send`
|
note: captured value is not `Send`
--> tests/ui/future_not_send.rs:21:26
--> tests/ui/future_not_send.rs:22:26
|
LL | async fn private_future2(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool {
| ^^ has type `std::rc::Rc<[u8]>` which is not `Send`
= note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Send`
note: captured value is not `Send` because `&` references cannot be sent unless their referent is `Sync`
--> tests/ui/future_not_send.rs:21:40
--> tests/ui/future_not_send.rs:22:40
|
LL | async fn private_future2(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool {
| ^^^^ has type `&std::cell::Cell<usize>` which is not `Send`, because `std::cell::Cell<usize>` is not `Sync`
= note: `std::cell::Cell<usize>` doesn't implement `std::marker::Sync`
error: future cannot be sent between threads safely
--> tests/ui/future_not_send.rs:26:1
--> tests/ui/future_not_send.rs:27:1
|
LL | pub async fn public_future2(rc: Rc<[u8]>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `public_future2` is not `Send`
|
note: captured value is not `Send`
--> tests/ui/future_not_send.rs:26:29
--> tests/ui/future_not_send.rs:27:29
|
LL | pub async fn public_future2(rc: Rc<[u8]>) {}
| ^^ has type `std::rc::Rc<[u8]>` which is not `Send`
= note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Send`
error: future cannot be sent between threads safely
--> tests/ui/future_not_send.rs:38:5
--> tests/ui/future_not_send.rs:39:5
|
LL | async fn private_future(&self) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `private_future` is not `Send`
|
note: future is not `Send` as this value is used across an await
--> tests/ui/future_not_send.rs:40:24
--> tests/ui/future_not_send.rs:41:24
|
LL | async fn private_future(&self) -> usize {
| ----- has type `&Dummy` which is not `Send`
@ -87,20 +87,20 @@ LL | async { true }.await;
= note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Sync`
error: future cannot be sent between threads safely
--> tests/ui/future_not_send.rs:44:5
--> tests/ui/future_not_send.rs:45:5
|
LL | pub async fn public_future(&self) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `public_future` is not `Send`
|
note: captured value is not `Send` because `&` references cannot be sent unless their referent is `Sync`
--> tests/ui/future_not_send.rs:44:32
--> tests/ui/future_not_send.rs:45:32
|
LL | pub async fn public_future(&self) {
| ^^^^^ has type `&Dummy` which is not `Send`, because `Dummy` is not `Sync`
= note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Sync`
error: future cannot be sent between threads safely
--> tests/ui/future_not_send.rs:55:1
--> tests/ui/future_not_send.rs:56:1
|
LL | / async fn generic_future<T>(t: T) -> T
LL | |
@ -109,7 +109,7 @@ LL | | T: Send,
| |____________^ future returned by `generic_future` is not `Send`
|
note: future is not `Send` as this value is used across an await
--> tests/ui/future_not_send.rs:61:20
--> tests/ui/future_not_send.rs:62:20
|
LL | let rt = &t;
| -- has type `&T` which is not `Send`
@ -118,17 +118,20 @@ LL | async { true }.await;
= note: `T` doesn't implement `std::marker::Sync`
error: future cannot be sent between threads safely
--> tests/ui/future_not_send.rs:73:1
--> tests/ui/future_not_send.rs:78:1
|
LL | async fn unclear_future<T>(t: T) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `unclear_future` is not `Send`
LL | async fn generic_future_always_unsend<T>(_: Rc<T>) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future returned by `generic_future_always_unsend` is not `Send`
|
note: captured value is not `Send`
--> tests/ui/future_not_send.rs:73:28
note: future is not `Send` as this value is used across an await
--> tests/ui/future_not_send.rs:80:20
|
LL | async fn unclear_future<T>(t: T) {}
| ^ has type `T` which is not `Send`
= note: `T` doesn't implement `std::marker::Send`
LL | async fn generic_future_always_unsend<T>(_: Rc<T>) {
| - has type `std::rc::Rc<T>` which is not `Send`
LL |
LL | async { true }.await;
| ^^^^^ await occurs here, with `_` maybe used later
= note: `std::rc::Rc<T>` doesn't implement `std::marker::Send`
error: aborting due to 8 previous errors