add lint futures_not_send

CHANGELOG.md

@@ -1280,6 +1280,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

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: Rc<[u8]>) {}
+ /// ```
+ /// Use instead:
+ /// ```rust
+ /// async fn is_send(bytes: 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(),
+ ));
+ }
+ }
+ })
+ },
+ );
+ }
+ }
+ }
+ }
+}

clippy_lints/src/lib.rs

@@ -217,6 +217,7 @@ pub mod floating_point_arithmetic;
 pub mod format;
 pub mod formatting;
 pub mod functions;
+pub mod future_not_send;
 pub mod get_last_with_len;
 pub mod identity_conversion;
 pub mod identity_op;
@@ -563,6 +564,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,
@@ -1032,6 +1034,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);
  store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default());
  store.register_late_pass(|| box unnamed_address::UnnamedAddress);
+ store.register_late_pass(|| box future_not_send::FutureNotSend);
 
  store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
  LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1677,6 +1680,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),

src/lintlist/mod.rs

@@ -689,6 +689,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
  deprecation: None,
  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",

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();
+}

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]>`
+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>`
+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]>`
+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`
+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`
+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`
+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
+