rust/clippy_lints/src/future_not_send.rs
Nicholas Nethercote 431cce1540 Restrict From<S> for {D,Subd}iagnosticMessage.
Currently a `{D,Subd}iagnosticMessage` can be created from any type that
impls `Into<String>`. That includes `&str`, `String`, and `Cow<'static,
str>`, which are reasonable. It also includes `&String`, which is pretty
weird, and results in many places making unnecessary allocations for
patterns like this:
```
self.fatal(&format!(...))
```
This creates a string with `format!`, takes a reference, passes the
reference to `fatal`, which does an `into()`, which clones the
reference, doing a second allocation. Two allocations for a single
string, bleh.

This commit changes the `From` impls so that you can only create a
`{D,Subd}iagnosticMessage` from `&str`, `String`, or `Cow<'static,
str>`. This requires changing all the places that currently create one
from a `&String`. Most of these are of the `&format!(...)` form
described above; each one removes an unnecessary static `&`, plus an
allocation when executed. There are also a few places where the existing
use of `&String` was more reasonable; these now just use `clone()` at
the call site.

As well as making the code nicer and more efficient, this is a step
towards possibly using `Cow<'static, str>` in
`{D,Subd}iagnosticMessage::{Str,Eager}`. That would require changing
the `From<&'a str>` impls to `From<&'static str>`, which is doable, but
I'm not yet sure if it's worthwhile.
2023-05-03 08:44:39 +10:00

113 lines
4.8 KiB
Rust
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::return_ty;
use rustc_hir::intravisit::FnKind;
use rustc_hir::{Body, FnDecl};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, AliasTy, Clause, PredicateKind};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::def_id::LocalDefId;
use rustc_span::{sym, Span};
use rustc_trait_selection::traits::error_reporting::suggestions::TypeErrCtxtExt;
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.
///
/// ### 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 users 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.
///
/// ### Example
/// ```rust
/// async fn not_send(bytes: std::rc::Rc<[u8]>) {}
/// ```
/// Use instead:
/// ```rust
/// async fn is_send(bytes: std::sync::Arc<[u8]>) {}
/// ```
#[clippy::version = "1.44.0"]
pub FUTURE_NOT_SEND,
nursery,
"public Futures must be Send"
}
declare_lint_pass!(FutureNotSend => [FUTURE_NOT_SEND]);
impl<'tcx> LateLintPass<'tcx> for FutureNotSend {
fn check_fn(
&mut self,
cx: &LateContext<'tcx>,
kind: FnKind<'tcx>,
decl: &'tcx FnDecl<'tcx>,
_: &'tcx Body<'tcx>,
_: Span,
fn_def_id: LocalDefId,
) {
if let FnKind::Closure = kind {
return;
}
let ret_ty = return_ty(cx, cx.tcx.hir().local_def_id_to_hir_id(fn_def_id).expect_owner());
if let ty::Alias(ty::Opaque, AliasTy { def_id, substs, .. }) = *ret_ty.kind() {
let preds = cx.tcx.explicit_item_bounds(def_id);
let mut is_future = false;
for (p, _span) in preds.subst_iter_copied(cx.tcx, substs) {
if let Some(trait_pred) = p.to_opt_poly_trait_pred() {
if Some(trait_pred.skip_binder().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).unwrap();
let span = decl.output.span();
let infcx = cx.tcx.infer_ctxt().build();
let ocx = ObligationCtxt::new(&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() {
span_lint_and_then(
cx,
FUTURE_NOT_SEND,
span,
"future cannot be sent between threads safely",
|db| {
for FulfillmentError { obligation, .. } in send_errors {
infcx
.err_ctxt()
.maybe_note_obligation_cause_for_async_await(db, &obligation);
if let PredicateKind::Clause(Clause::Trait(trait_pred)) =
obligation.predicate.kind().skip_binder()
{
db.note(format!(
"`{}` doesn't implement `{}`",
trait_pred.self_ty(),
trait_pred.trait_ref.print_only_trait_path(),
));
}
}
},
);
}
}
}
}
}