Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider NonNull as a pointer type #8074

Merged
merged 1 commit into from
Dec 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions clippy_lints/src/non_send_fields_in_send_ty.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_lint_allowed;
use clippy_utils::source::snippet;
use clippy_utils::ty::{implements_trait, is_copy};
use clippy_utils::{is_lint_allowed, match_def_path, paths};
use rustc_ast::ImplPolarity;
use rustc_hir::def_id::DefId;
use rustc_hir::{FieldDef, Item, ItemKind, Node};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::sym;
Expand Down Expand Up @@ -77,6 +78,7 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
// single `AdtDef` may have multiple `Send` impls due to generic
// parameters, and the lint is much easier to implement in this way.
if_chain! {
if !in_external_macro(cx.tcx.sess, item.span);
if let Some(send_trait) = cx.tcx.get_diagnostic_item(sym::Send);
if let ItemKind::Impl(hir_impl) = &item.kind;
if let Some(trait_ref) = &hir_impl.of_trait;
Expand Down Expand Up @@ -181,7 +183,7 @@ fn ty_allowed_without_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty
return true;
}

if is_copy(cx, ty) && !contains_raw_pointer(cx, ty) {
if is_copy(cx, ty) && !contains_pointer_like(cx, ty) {
return true;
}

Expand All @@ -201,7 +203,7 @@ fn ty_allowed_with_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'t
.all(|ty| ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait)),
ty::Array(ty, _) | ty::Slice(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait),
ty::Adt(_, substs) => {
if contains_raw_pointer(cx, ty) {
if contains_pointer_like(cx, ty) {
// descends only if ADT contains any raw pointers
substs.iter().all(|generic_arg| match generic_arg.unpack() {
GenericArgKind::Type(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait),
Expand All @@ -218,14 +220,20 @@ fn ty_allowed_with_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'t
}
}

/// Checks if the type contains any raw pointers in substs (including nested ones).
fn contains_raw_pointer<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool {
/// Checks if the type contains any pointer-like types in substs (including nested ones)
fn contains_pointer_like<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool {
for ty_node in target_ty.walk(cx.tcx) {
if_chain! {
if let GenericArgKind::Type(inner_ty) = ty_node.unpack();
if let ty::RawPtr(_) = inner_ty.kind();
then {
return true;
if let GenericArgKind::Type(inner_ty) = ty_node.unpack() {
match inner_ty.kind() {
ty::RawPtr(_) => {
return true;
},
ty::Adt(adt_def, _) => {
if match_def_path(cx, adt_def.did, &paths::PTR_NON_NULL) {
return true;
}
},
_ => (),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,3 +206,4 @@ pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"];
pub const WRITE_MACRO: [&str; 3] = ["core", "macros", "write"];
#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros
pub const WRITELN_MACRO: [&str; 3] = ["core", "macros", "writeln"];
pub const PTR_NON_NULL: [&str; 4] = ["core", "ptr", "non_null", "NonNull"];
5 changes: 5 additions & 0 deletions tests/ui/non_send_fields_in_send_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ pub enum MyOption<T> {

unsafe impl<T> Send for MyOption<T> {}

// Test types that contain `NonNull` instead of raw pointers (#8045)
pub struct WrappedNonNull(UnsafeCell<NonNull<()>>);

unsafe impl Send for WrappedNonNull {}

// Multiple type parameters
pub struct MultiParam<A, B> {
vec: Vec<(A, B)>,
Expand Down
20 changes: 10 additions & 10 deletions tests/ui/non_send_fields_in_send_ty.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -103,65 +103,65 @@ LL | MySome(T),
= help: add `T: Send` bound in `Send` impl

error: this implementation is unsound, as some fields in `MultiParam<A, B>` are `!Send`
--> $DIR/non_send_fields_in_send_ty.rs:77:1
--> $DIR/non_send_fields_in_send_ty.rs:82:1
|
LL | unsafe impl<A, B> Send for MultiParam<A, B> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `vec` is `!Send`
--> $DIR/non_send_fields_in_send_ty.rs:74:5
--> $DIR/non_send_fields_in_send_ty.rs:79:5
|
LL | vec: Vec<(A, B)>,
| ^^^^^^^^^^^^^^^^
= help: add bounds on type parameters `A, B` that satisfy `Vec<(A, B)>: Send`

error: this implementation is unsound, as some fields in `HeuristicTest` are `!Send`
--> $DIR/non_send_fields_in_send_ty.rs:95:1
--> $DIR/non_send_fields_in_send_ty.rs:100:1
|
LL | unsafe impl Send for HeuristicTest {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `field4` is `!Send`
--> $DIR/non_send_fields_in_send_ty.rs:90:5
--> $DIR/non_send_fields_in_send_ty.rs:95:5
|
LL | field4: (*const NonSend, Rc<u8>),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: use a thread-safe type that implements `Send`

error: this implementation is unsound, as some fields in `AttrTest3<T>` are `!Send`
--> $DIR/non_send_fields_in_send_ty.rs:114:1
--> $DIR/non_send_fields_in_send_ty.rs:119:1
|
LL | unsafe impl<T> Send for AttrTest3<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `0` is `!Send`
--> $DIR/non_send_fields_in_send_ty.rs:109:11
--> $DIR/non_send_fields_in_send_ty.rs:114:11
|
LL | Enum2(T),
| ^
= help: add `T: Send` bound in `Send` impl

error: this implementation is unsound, as some fields in `Complex<P, u32>` are `!Send`
--> $DIR/non_send_fields_in_send_ty.rs:122:1
--> $DIR/non_send_fields_in_send_ty.rs:127:1
|
LL | unsafe impl<P> Send for Complex<P, u32> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `field1` is `!Send`
--> $DIR/non_send_fields_in_send_ty.rs:118:5
--> $DIR/non_send_fields_in_send_ty.rs:123:5
|
LL | field1: A,
| ^^^^^^^^^
= help: add `P: Send` bound in `Send` impl

error: this implementation is unsound, as some fields in `Complex<Q, MutexGuard<'static, bool>>` are `!Send`
--> $DIR/non_send_fields_in_send_ty.rs:125:1
--> $DIR/non_send_fields_in_send_ty.rs:130:1
|
LL | unsafe impl<Q: Send> Send for Complex<Q, MutexGuard<'static, bool>> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the type of field `field2` is `!Send`
--> $DIR/non_send_fields_in_send_ty.rs:119:5
--> $DIR/non_send_fields_in_send_ty.rs:124:5
|
LL | field2: B,
| ^^^^^^^^^
Expand Down