Skip to content

Commit

Permalink
Remove unconditional-send-future config
Browse files Browse the repository at this point in the history
  • Loading branch information
@y21
y21 committed Nov 2, 2024
1 parent 8e6b75d commit 469eced
Show file tree
Hide file tree
Showing 13 changed files with 71 additions and 376 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6220,7 +6220,6 @@ Released 2018-09-13
[`too-many-lines-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-many-lines-threshold
[`trivial-copy-size-limit`]: https://doc.rust-lang.org/clippy/lint_configuration.html#trivial-copy-size-limit
[`type-complexity-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#type-complexity-threshold
[`unconditional-send-futures`]: https://doc.rust-lang.org/clippy/lint_configuration.html#unconditional-send-futures
[`unnecessary-box-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#unnecessary-box-size
[`unreadable-literal-lint-fractions`]: https://doc.rust-lang.org/clippy/lint_configuration.html#unreadable-literal-lint-fractions
[`upper-case-acronyms-aggressive`]: https://doc.rust-lang.org/clippy/lint_configuration.html#upper-case-acronyms-aggressive
Expand Down
27 changes: 0 additions & 27 deletions book/src/lint_configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -883,33 +883,6 @@ The maximum complexity a type can have
* [`type_complexity`](https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity)


## `unconditional-send-futures`
Whether `future_not_send` should require futures to implement `Send` unconditionally.
Enabling this makes the lint more strict and requires generic functions have `Send` bounds
on type parameters.

### Example
```
async fn foo<R>(r: R) {
std::future::ready(()).await;
r;
}
```
The returned future by this async function may or may not be `Send` - it depends on the particular type
that `R` is instantiated with at call site, so it is **not** unconditionally `Send`.
Adding an `R: Send` bound to the function definition satisfies it, but is more restrictive for callers.

For library crate authors that want to make sure that their async functions are compatible
with both single-threaded and multi-threaded executors, the default behavior of allowing `!Send` futures
if type parameters are `!Send` makes sense.

**Default Value:** `false`

---
**Affected lints:**
* [`future_not_send`](https://rust-lang.github.io/rust-clippy/master/index.html#future_not_send)


## `unnecessary-box-size`
The byte size a `T` in `Box<T>` can have, below which it triggers the `clippy::unnecessary_box` lint

Expand Down
20 changes: 0 additions & 20 deletions clippy_config/src/conf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,26 +645,6 @@ define_Conf! {
/// The maximum complexity a type can have
#[lints(type_complexity)]
type_complexity_threshold: u64 = 250,
/// Whether `future_not_send` should require futures to implement `Send` unconditionally.
/// Enabling this makes the lint more strict and requires generic functions have `Send` bounds
/// on type parameters.
///
/// ### Example
/// ```
/// async fn foo<R>(r: R) {
/// std::future::ready(()).await;
/// r;
/// }
/// ```
/// The returned future by this async function may or may not be `Send` - it depends on the particular type
/// that `R` is instantiated with at call site, so it is **not** unconditionally `Send`.
/// Adding an `R: Send` bound to the function definition satisfies it, but is more restrictive for callers.
///
/// For library crate authors that want to make sure that their async functions are compatible
/// with both single-threaded and multi-threaded executors, the default behavior of allowing `!Send` futures
/// if type parameters are `!Send` makes sense.
#[lints(future_not_send)]
unconditional_send_futures: bool = false,
/// The byte size a `T` in `Box<T>` can have, below which it triggers the `clippy::unnecessary_box` lint
#[lints(unnecessary_box_returns)]
unnecessary_box_size: u64 = 128,
Expand Down
103 changes: 40 additions & 63 deletions clippy_lints/src/future_not_send.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::ops::ControlFlow;

use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::return_ty;
use rustc_hir::intravisit::FnKind;
Expand All @@ -11,7 +10,7 @@ use rustc_middle::ty::print::PrintTraitRefExt;
use rustc_middle::ty::{
self, AliasTy, Binder, ClauseKind, PredicateKind, Ty, TyCtxt, TypeVisitable, TypeVisitableExt, TypeVisitor,
};
use rustc_session::impl_lint_pass;
use rustc_session::declare_lint_pass;
use rustc_span::def_id::LocalDefId;
use rustc_span::{Span, sym};
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
Expand All @@ -20,23 +19,17 @@ 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.
/// 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.
///
/// The default configuration of this lint only emits warnings for futures
/// that are unconditionally `!Send`, ignoring generic parameters.
/// This can be used by library authors (public and internal) to ensure
/// their functions are compatible with multi-threaded runtimes that require `Send` futures,
/// 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.
///
/// A more strict version can be enabled through the `unconditional_send_futures` configuration,
/// which requires that futures must always unconditionally implement `Send`,
/// even if whether the future is `Send` or not is determined at call site for generic functions.
/// This can be useful for binary crates that always use a multi-threaded runtime to find `!Send` futures
/// as early as possible and keep errors contained in the most relevant place,
/// instead of propagating `!Send` and be left with a hard-to-debug error in an
/// unrelated place (e.g. the final future passed to `tokio::spawn()`).
///
/// ### Why is this bad?
/// A Future implementation captures some state that it
/// needs to eventually produce its final value. When targeting a multithreaded
Expand Down Expand Up @@ -66,19 +59,7 @@ declare_clippy_lint! {
"public Futures must be Send"
}

pub struct FutureNotSend {
unconditionally_not_send: bool,
}

impl FutureNotSend {
pub fn new(conf: &'static Conf) -> Self {
Self {
unconditionally_not_send: conf.unconditional_send_futures,
}
}
}

impl_lint_pass!(FutureNotSend => [FUTURE_NOT_SEND]);
declare_lint_pass!(FutureNotSend => [FUTURE_NOT_SEND]);

impl<'tcx> LateLintPass<'tcx> for FutureNotSend {
fn check_fn(
Expand Down Expand Up @@ -111,43 +92,27 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend {
ocx.register_bound(cause, cx.param_env, ret_ty, send_trait);
let send_errors = ocx.select_all_or_error();

let is_send = if self.unconditionally_not_send {
send_errors.is_empty()
} else {
// 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.

struct TypeWalker;
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for TypeWalker {
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),
}
}
}
// 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.

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()
&& TypeWalker.visit_ty(pred.self_ty()) == ControlFlow::Break(true)
})
})
};
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(
Expand Down Expand Up @@ -177,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),
}
}
}
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(unnamed_address::UnnamedAddress));
store.register_late_pass(|_| Box::<dereference::Dereferencing<'_>>::default());
store.register_late_pass(|_| Box::new(option_if_let_else::OptionIfLetElse));
store.register_late_pass(move |_| Box::new(future_not_send::FutureNotSend::new(conf)));
store.register_late_pass(|_| Box::new(future_not_send::FutureNotSend));
store.register_late_pass(move |_| Box::new(large_futures::LargeFuture::new(conf)));
store.register_late_pass(|_| Box::new(if_let_mutex::IfLetMutex));
store.register_late_pass(|_| Box::new(if_not_else::IfNotElse));
Expand Down
Empty file removed tests/ui-toml/future_not_send/default/clippy.toml
View file
Empty file.
Loading

0 comments on commit 469eced

Please sign in to comment.