Auto merge of #8074 - Qwaz:send_nonnull, r=xFrednet

Consider NonNull as a pointer type

PR 1/2 for issue #8045. Add `NonNull` as a pointer class to suppress false positives like `UnsafeCell<NonNull<()>>`. However, this change is not sufficient to handle the cases shared in gtk-rs and Rug in the issue.

changelog: none

r? `@xFrednet`
This commit is contained in:
bors 2021-12-04 17:19:07 +00:00
commit 907f6d9294
4 changed files with 34 additions and 20 deletions

View file

@ -1,11 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_lint_allowed;
use clippy_utils::source::snippet; use clippy_utils::source::snippet;
use clippy_utils::ty::{implements_trait, is_copy}; use clippy_utils::ty::{implements_trait, is_copy};
use clippy_utils::{is_lint_allowed, match_def_path, paths};
use rustc_ast::ImplPolarity; use rustc_ast::ImplPolarity;
use rustc_hir::def_id::DefId; use rustc_hir::def_id::DefId;
use rustc_hir::{FieldDef, Item, ItemKind, Node}; use rustc_hir::{FieldDef, Item, ItemKind, Node};
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, subst::GenericArgKind, Ty}; use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::sym; use rustc_span::sym;
@ -77,6 +78,7 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
// single `AdtDef` may have multiple `Send` impls due to generic // single `AdtDef` may have multiple `Send` impls due to generic
// parameters, and the lint is much easier to implement in this way. // parameters, and the lint is much easier to implement in this way.
if_chain! { 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 Some(send_trait) = cx.tcx.get_diagnostic_item(sym::Send);
if let ItemKind::Impl(hir_impl) = &item.kind; if let ItemKind::Impl(hir_impl) = &item.kind;
if let Some(trait_ref) = &hir_impl.of_trait; if let Some(trait_ref) = &hir_impl.of_trait;
@ -181,7 +183,7 @@ fn ty_allowed_without_raw_pointer_heuristic<'tcx>(cx: &LateContext<'tcx>, ty: Ty
return true; return true;
} }
if is_copy(cx, ty) && !contains_raw_pointer(cx, ty) { if is_copy(cx, ty) && !contains_pointer_like(cx, ty) {
return true; return true;
} }
@ -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)), .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::Array(ty, _) | ty::Slice(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait),
ty::Adt(_, substs) => { ty::Adt(_, substs) => {
if contains_raw_pointer(cx, ty) { if contains_pointer_like(cx, ty) {
// descends only if ADT contains any raw pointers // descends only if ADT contains any raw pointers
substs.iter().all(|generic_arg| match generic_arg.unpack() { substs.iter().all(|generic_arg| match generic_arg.unpack() {
GenericArgKind::Type(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait), GenericArgKind::Type(ty) => ty_allowed_with_raw_pointer_heuristic(cx, ty, send_trait),
@ -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). /// Checks if the type contains any pointer-like types in substs (including nested ones)
fn contains_raw_pointer<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool { fn contains_pointer_like<'tcx>(cx: &LateContext<'tcx>, target_ty: Ty<'tcx>) -> bool {
for ty_node in target_ty.walk(cx.tcx) { for ty_node in target_ty.walk(cx.tcx) {
if_chain! { if let GenericArgKind::Type(inner_ty) = ty_node.unpack() {
if let GenericArgKind::Type(inner_ty) = ty_node.unpack(); match inner_ty.kind() {
if let ty::RawPtr(_) = inner_ty.kind(); ty::RawPtr(_) => {
then { return true;
return true; },
ty::Adt(adt_def, _) => {
if match_def_path(cx, adt_def.did, &paths::PTR_NON_NULL) {
return true;
}
},
_ => (),
} }
} }
} }

View file

@ -206,3 +206,4 @@ pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"];
pub const WRITE_MACRO: [&str; 3] = ["core", "macros", "write"]; pub const WRITE_MACRO: [&str; 3] = ["core", "macros", "write"];
#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros #[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros
pub const WRITELN_MACRO: [&str; 3] = ["core", "macros", "writeln"]; pub const WRITELN_MACRO: [&str; 3] = ["core", "macros", "writeln"];
pub const PTR_NON_NULL: [&str; 4] = ["core", "ptr", "non_null", "NonNull"];

View file

@ -69,6 +69,11 @@ pub enum MyOption<T> {
unsafe impl<T> Send for 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 // Multiple type parameters
pub struct MultiParam<A, B> { pub struct MultiParam<A, B> {
vec: Vec<(A, B)>, vec: Vec<(A, B)>,

View file

@ -103,65 +103,65 @@ LL | MySome(T),
= help: add `T: Send` bound in `Send` impl = help: add `T: Send` bound in `Send` impl
error: this implementation is unsound, as some fields in `MultiParam<A, B>` are `!Send` 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> {} LL | unsafe impl<A, B> Send for MultiParam<A, B> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
note: the type of field `vec` is `!Send` 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)>, LL | vec: Vec<(A, B)>,
| ^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^
= help: add bounds on type parameters `A, B` that satisfy `Vec<(A, B)>: Send` = 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` 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 {} LL | unsafe impl Send for HeuristicTest {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
note: the type of field `field4` is `!Send` 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>), LL | field4: (*const NonSend, Rc<u8>),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: use a thread-safe type that implements `Send` = help: use a thread-safe type that implements `Send`
error: this implementation is unsound, as some fields in `AttrTest3<T>` are `!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> {} LL | unsafe impl<T> Send for AttrTest3<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
note: the type of field `0` is `!Send` 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), LL | Enum2(T),
| ^ | ^
= help: add `T: Send` bound in `Send` impl = help: add `T: Send` bound in `Send` impl
error: this implementation is unsound, as some fields in `Complex<P, u32>` are `!Send` 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> {} LL | unsafe impl<P> Send for Complex<P, u32> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
note: the type of field `field1` is `!Send` 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, LL | field1: A,
| ^^^^^^^^^ | ^^^^^^^^^
= help: add `P: Send` bound in `Send` impl = help: add `P: Send` bound in `Send` impl
error: this implementation is unsound, as some fields in `Complex<Q, MutexGuard<'static, bool>>` are `!Send` 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>> {} LL | unsafe impl<Q: Send> Send for Complex<Q, MutexGuard<'static, bool>> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
note: the type of field `field2` is `!Send` 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, LL | field2: B,
| ^^^^^^^^^ | ^^^^^^^^^