diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/future_not_send.rs | 113 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 4 | ||||
| -rw-r--r-- | src/lintlist/mod.rs | 7 | ||||
| -rw-r--r-- | tests/ui/future_not_send.rs | 79 | ||||
| -rw-r--r-- | tests/ui/future_not_send.stderr | 125 |
6 files changed, 329 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 65eab69667f..60ad855d7f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1281,6 +1281,7 @@ Released 2018-09-13 [`for_loop_over_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loop_over_result [`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy [`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref +[`future_not_send`]: https://rust-lang.github.io/rust-clippy/master/index.html#future_not_send [`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap [`identity_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion diff --git a/clippy_lints/src/future_not_send.rs b/clippy_lints/src/future_not_send.rs new file mode 100644 index 00000000000..57f47bc9bc9 --- /dev/null +++ b/clippy_lints/src/future_not_send.rs @@ -0,0 +1,113 @@ +use crate::utils; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, FnDecl, HirId}; +use rustc_infer::infer::TyCtxtInferExt; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{Opaque, Predicate::Trait, ToPolyTraitRef}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::{sym, Span}; +use rustc_trait_selection::traits::error_reporting::suggestions::InferCtxtExt; +use rustc_trait_selection::traits::{self, FulfillmentError, TraitEngine}; + +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. + /// + /// **Why is this bad?** A Future implementation captures some state that it + /// needs to eventually produce its final value. When targeting a multithreaded + /// executor (which is the norm on non-embedded devices) this means that this + /// state may need to be transported to other threads, in other words the + /// whole Future needs to implement the `Send` marker trait. If it does not, + /// then the resulting Future cannot be submitted to a thread pool in the + /// end user’s code. + /// + /// Especially for generic functions it can be confusing to leave the + /// discovery of this problem to the end user: the reported error location + /// will be far from its cause and can in many cases not even be fixed without + /// modifying the library where the offending Future implementation is + /// produced. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// async fn not_send(bytes: std::rc::Rc<[u8]>) {} + /// ``` + /// Use instead: + /// ```rust + /// async fn is_send(bytes: std::sync::Arc<[u8]>) {} + /// ``` + pub FUTURE_NOT_SEND, + nursery, + "public Futures must be Send" +} + +declare_lint_pass!(FutureNotSend => [FUTURE_NOT_SEND]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FutureNotSend { + fn check_fn( + &mut self, + cx: &LateContext<'a, 'tcx>, + kind: FnKind<'tcx>, + decl: &'tcx FnDecl<'tcx>, + _: &'tcx Body<'tcx>, + _: Span, + hir_id: HirId, + ) { + if let FnKind::Closure(_) = kind { + return; + } + let def_id = cx.tcx.hir().local_def_id(hir_id); + let fn_sig = cx.tcx.fn_sig(def_id); + let fn_sig = cx.tcx.erase_late_bound_regions(&fn_sig); + let ret_ty = fn_sig.output(); + if let Opaque(id, subst) = ret_ty.kind { + let preds = cx.tcx.predicates_of(id).instantiate(cx.tcx, subst); + let mut is_future = false; + for p in preds.predicates { + if let Some(trait_ref) = p.to_opt_poly_trait_ref() { + if Some(trait_ref.def_id()) == cx.tcx.lang_items().future_trait() { + is_future = true; + break; + } + } + } + if is_future { + let send_trait = cx.tcx.get_diagnostic_item(sym::send_trait).unwrap(); + let span = decl.output.span(); + let send_result = cx.tcx.infer_ctxt().enter(|infcx| { + let cause = traits::ObligationCause::misc(span, hir_id); + let mut fulfillment_cx = traits::FulfillmentContext::new(); + fulfillment_cx.register_bound(&infcx, cx.param_env, ret_ty, send_trait, cause); + fulfillment_cx.select_all_or_error(&infcx) + }); + if let Err(send_errors) = send_result { + utils::span_lint_and_then( + cx, + FUTURE_NOT_SEND, + span, + "future cannot be sent between threads safely", + |db| { + cx.tcx.infer_ctxt().enter(|infcx| { + for FulfillmentError { obligation, .. } in send_errors { + infcx.maybe_note_obligation_cause_for_async_await(db, &obligation); + if let Trait(trait_pred, _) = obligation.predicate { + let trait_ref = trait_pred.to_poly_trait_ref(); + db.note(&*format!( + "`{}` doesn't implement `{}`", + trait_ref.self_ty(), + trait_ref.print_only_trait_path(), + )); + } + } + }) + }, + ); + } + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1158e70b7a3..a5b55d2ab70 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -218,6 +218,7 @@ mod floating_point_arithmetic; mod format; mod formatting; mod functions; +mod future_not_send; mod get_last_with_len; mod identity_conversion; mod identity_op; @@ -566,6 +567,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &functions::NOT_UNSAFE_PTR_ARG_DEREF, &functions::TOO_MANY_ARGUMENTS, &functions::TOO_MANY_LINES, + &future_not_send::FUTURE_NOT_SEND, &get_last_with_len::GET_LAST_WITH_LEN, &identity_conversion::IDENTITY_CONVERSION, &identity_op::IDENTITY_OP, @@ -1045,6 +1047,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); store.register_late_pass(|| box unnamed_address::UnnamedAddress); store.register_late_pass(|| box dereference::Dereferencing); + store.register_late_pass(|| box future_not_send::FutureNotSend); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1689,6 +1692,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&fallible_impl_from::FALLIBLE_IMPL_FROM), LintId::of(&floating_point_arithmetic::IMPRECISE_FLOPS), LintId::of(&floating_point_arithmetic::SUBOPTIMAL_FLOPS), + LintId::of(&future_not_send::FUTURE_NOT_SEND), LintId::of(&missing_const_for_fn::MISSING_CONST_FOR_FN), LintId::of(&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL), LintId::of(&mutex_atomic::MUTEX_INTEGER), diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index d4602a3ad8e..cf2537e6d66 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -697,6 +697,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![ module: "drop_forget_ref", }, Lint { + name: "future_not_send", + group: "nursery", + desc: "public Futures must be Send", + deprecation: None, + module: "future_not_send", + }, + Lint { name: "get_last_with_len", group: "complexity", desc: "Using `x.get(x.len() - 1)` when `x.last()` is correct and simpler", diff --git a/tests/ui/future_not_send.rs b/tests/ui/future_not_send.rs new file mode 100644 index 00000000000..6d09d71a630 --- /dev/null +++ b/tests/ui/future_not_send.rs @@ -0,0 +1,79 @@ +// edition:2018 +#![warn(clippy::future_not_send)] + +use std::cell::Cell; +use std::rc::Rc; +use std::sync::Arc; + +async fn private_future(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool { + async { true }.await +} + +pub async fn public_future(rc: Rc<[u8]>) { + async { true }.await; +} + +pub async fn public_send(arc: Arc<[u8]>) -> bool { + async { false }.await +} + +async fn private_future2(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool { + true +} + +pub async fn public_future2(rc: Rc<[u8]>) {} + +pub async fn public_send2(arc: Arc<[u8]>) -> bool { + false +} + +struct Dummy { + rc: Rc<[u8]>, +} + +impl Dummy { + async fn private_future(&self) -> usize { + async { true }.await; + self.rc.len() + } + + pub async fn public_future(&self) { + self.private_future().await; + } + + pub fn public_send(&self) -> impl std::future::Future<Output = bool> { + async { false } + } +} + +async fn generic_future<T>(t: T) -> T +where + T: Send, +{ + let rt = &t; + async { true }.await; + t +} + +async fn generic_future_send<T>(t: T) +where + T: Send, +{ + async { true }.await; +} + +async fn unclear_future<T>(t: T) {} + +fn main() { + let rc = Rc::new([1, 2, 3]); + private_future(rc.clone(), &Cell::new(42)); + public_future(rc.clone()); + let arc = Arc::new([4, 5, 6]); + public_send(arc); + generic_future(42); + generic_future_send(42); + + let dummy = Dummy { rc }; + dummy.public_future(); + dummy.public_send(); +} diff --git a/tests/ui/future_not_send.stderr b/tests/ui/future_not_send.stderr new file mode 100644 index 00000000000..3b4968ef0a6 --- /dev/null +++ b/tests/ui/future_not_send.stderr @@ -0,0 +1,125 @@ +error: future cannot be sent between threads safely + --> $DIR/future_not_send.rs:8:62 + | +LL | async fn private_future(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool { + | ^^^^ future returned by `private_future` is not `Send` + | + = note: `-D clippy::future-not-send` implied by `-D warnings` +note: future is not `Send` as this value is used across an await + --> $DIR/future_not_send.rs:9:5 + | +LL | async fn private_future(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool { + | -- has type `std::rc::Rc<[u8]>` which is not `Send` +LL | async { true }.await + | ^^^^^^^^^^^^^^^^^^^^ await occurs here, with `rc` maybe used later +LL | } + | - `rc` is later dropped here + = note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Send` +note: future is not `Send` as this value is used across an await + --> $DIR/future_not_send.rs:9:5 + | +LL | async fn private_future(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool { + | ---- has type `&std::cell::Cell<usize>` which is not `Send` +LL | async { true }.await + | ^^^^^^^^^^^^^^^^^^^^ await occurs here, with `cell` maybe used later +LL | } + | - `cell` is later dropped here + = note: `std::cell::Cell<usize>` doesn't implement `std::marker::Sync` + +error: future cannot be sent between threads safely + --> $DIR/future_not_send.rs:12:42 + | +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 + --> $DIR/future_not_send.rs:13:5 + | +LL | pub async fn public_future(rc: Rc<[u8]>) { + | -- has type `std::rc::Rc<[u8]>` which is not `Send` +LL | async { true }.await; + | ^^^^^^^^^^^^^^^^^^^^ await occurs here, with `rc` maybe used later +LL | } + | - `rc` is later dropped here + = note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Send` + +error: future cannot be sent between threads safely + --> $DIR/future_not_send.rs:20:63 + | +LL | async fn private_future2(rc: Rc<[u8]>, cell: &Cell<usize>) -> bool { + | ^^^^ + | + = note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Send` + = note: `std::cell::Cell<usize>` doesn't implement `std::marker::Sync` + +error: future cannot be sent between threads safely + --> $DIR/future_not_send.rs:24:43 + | +LL | pub async fn public_future2(rc: Rc<[u8]>) {} + | ^ + | + = note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Send` + +error: future cannot be sent between threads safely + --> $DIR/future_not_send.rs:35:39 + | +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 + --> $DIR/future_not_send.rs:36:9 + | +LL | async fn private_future(&self) -> usize { + | ----- has type `&Dummy` which is not `Send` +LL | async { true }.await; + | ^^^^^^^^^^^^^^^^^^^^ await occurs here, with `&self` maybe used later +LL | self.rc.len() +LL | } + | - `&self` is later dropped here + = note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Sync` + +error: future cannot be sent between threads safely + --> $DIR/future_not_send.rs:40:39 + | +LL | pub async fn public_future(&self) { + | ^ future returned by `public_future` is not `Send` + | +note: future is not `Send` as this value is used across an await + --> $DIR/future_not_send.rs:41:9 + | +LL | pub async fn public_future(&self) { + | ----- has type `&Dummy` which is not `Send` +LL | self.private_future().await; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ await occurs here, with `&self` maybe used later +LL | } + | - `&self` is later dropped here + = note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Sync` + +error: future cannot be sent between threads safely + --> $DIR/future_not_send.rs:49:37 + | +LL | async fn generic_future<T>(t: T) -> T + | ^ future returned by `generic_future` is not `Send` + | +note: future is not `Send` as this value is used across an await + --> $DIR/future_not_send.rs:54:5 + | +LL | let rt = &t; + | -- has type `&T` which is not `Send` +LL | async { true }.await; + | ^^^^^^^^^^^^^^^^^^^^ await occurs here, with `rt` maybe used later +LL | t +LL | } + | - `rt` is later dropped here + = note: `T` doesn't implement `std::marker::Sync` + +error: future cannot be sent between threads safely + --> $DIR/future_not_send.rs:65:34 + | +LL | async fn unclear_future<T>(t: T) {} + | ^ + | + = note: `T` doesn't implement `std::marker::Send` + +error: aborting due to 8 previous errors + |