diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 2c92277b50d21..e0eb4106d9056 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -215,9 +215,6 @@ lint_expectation = this lint expectation is unfulfilled .note = the `unfulfilled_lint_expectations` lint can't be expected and will always produce this message .rationale = {$rationale} -lint_fn_null_check = function pointers are not nullable, so checking them for null will always return false - .help = wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value - lint_for_loops_over_fallibles = for loop over {$article} `{$ty}`. This is more readably written as an `if let` statement .suggestion = consider using `if let` to clear intent @@ -399,6 +396,9 @@ lint_non_fmt_panic_unused = } .add_fmt_suggestion = or add a "{"{"}{"}"}" format string to use the message literally +lint_non_null_check = {$ty_desc}s can never be null, so checking them for null will always return false + .help = wrap the {$ty_desc} inside an `Option` and use `Option::is_none` to check for null pointer values + lint_non_snake_case = {$sort} `{$name}` should have a snake case name .rename_or_convert_suggestion = rename the identifier or convert it to a snake case raw identifier .cannot_convert_note = `{$sc}` cannot be used as a raw identifier diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index beb38dbb94c32..59525f6f3cd5d 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -58,7 +58,6 @@ mod early; mod enum_intrinsics_non_enums; mod errors; mod expect; -mod fn_null_check; mod for_loops_over_fallibles; pub mod hidden_unicode_codepoints; mod internal; @@ -72,6 +71,7 @@ mod methods; mod multiple_supertrait_upcastable; mod non_ascii_idents; mod non_fmt_panic; +mod non_null_check; mod nonstandard_style; mod noop_method_call; mod opaque_hidden_inferred_bound; @@ -103,7 +103,6 @@ use cast_ref_to_mut::*; use deref_into_dyn_supertrait::*; use drop_forget_useless::*; use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums; -use fn_null_check::*; use for_loops_over_fallibles::*; use hidden_unicode_codepoints::*; use internal::*; @@ -114,6 +113,7 @@ use methods::*; use multiple_supertrait_upcastable::*; use non_ascii_idents::*; use non_fmt_panic::NonPanicFmt; +use non_null_check::*; use nonstandard_style::*; use noop_method_call::*; use opaque_hidden_inferred_bound::*; @@ -227,7 +227,7 @@ late_lint_methods!( // Depends on types used in type definitions MissingCopyImplementations: MissingCopyImplementations, // Depends on referenced function signatures in expressions - IncorrectFnNullChecks: IncorrectFnNullChecks, + IncorrectNonNullChecks: IncorrectNonNullChecks, MutableTransmutes: MutableTransmutes, TypeAliasBounds: TypeAliasBounds, TrivialConstraints: TrivialConstraints, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 1dea758bb294b..fc25805ec84c8 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1,5 +1,6 @@ #![allow(rustc::untranslatable_diagnostic)] #![allow(rustc::diagnostic_outside_of_impl)] +use std::borrow::Cow; use std::num::NonZeroU32; use crate::fluent_generated as fluent; @@ -613,11 +614,13 @@ pub struct ExpectationNote { pub rationale: Symbol, } -// fn_null_check.rs +// non_null_check.rs #[derive(LintDiagnostic)] -#[diag(lint_fn_null_check)] +#[diag(lint_non_null_check)] #[help] -pub struct FnNullCheckDiag; +pub struct NonNullCheckDiag { + pub ty_desc: Cow<'static, str>, +} // for_loops_over_fallibles.rs #[derive(LintDiagnostic)] diff --git a/compiler/rustc_lint/src/fn_null_check.rs b/compiler/rustc_lint/src/non_null_check.rs similarity index 51% rename from compiler/rustc_lint/src/fn_null_check.rs rename to compiler/rustc_lint/src/non_null_check.rs index e3b33463ccfbc..3b67270266d80 100644 --- a/compiler/rustc_lint/src/fn_null_check.rs +++ b/compiler/rustc_lint/src/non_null_check.rs @@ -1,12 +1,13 @@ -use crate::{lints::FnNullCheckDiag, LateContext, LateLintPass, LintContext}; +use crate::{lints::NonNullCheckDiag, LateContext, LateLintPass, LintContext}; use rustc_ast::LitKind; use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind}; +use rustc_middle::ty::Ty; use rustc_session::{declare_lint, declare_lint_pass}; use rustc_span::sym; declare_lint! { - /// The `incorrect_fn_null_checks` lint checks for expression that checks if a - /// function pointer is null. + /// The `incorrect_non_null_checks` lint checks for expressions that check if a + /// non-nullable type is null. /// /// ### Example /// @@ -22,16 +23,19 @@ declare_lint! { /// /// ### Explanation /// - /// Function pointers are assumed to be non-null, checking them for null will always - /// return false. - INCORRECT_FN_NULL_CHECKS, + /// A non-nullable type is assumed to never be null, and therefore having an actual + /// non-null pointer is ub. + INCORRECT_NON_NULL_CHECKS, Warn, - "incorrect checking of null function pointer" + "incorrect checking of non null pointers" } -declare_lint_pass!(IncorrectFnNullChecks => [INCORRECT_FN_NULL_CHECKS]); +declare_lint_pass!(IncorrectNonNullChecks => [INCORRECT_NON_NULL_CHECKS]); -fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { +/// Is the cast to a nonnull type? +/// If yes, return (ty, nullable_version) where former is the nonnull type while latter +/// is a nullable version (e.g. (fn, Option) or (&u8, *const u8)). +fn is_nonnull_cast<'a>(cx: &LateContext<'a>, expr: &Expr<'_>) -> Option> { let mut expr = expr.peel_blocks(); let mut had_at_least_one_cast = false; while let ExprKind::Cast(cast_expr, cast_ty) = expr.kind @@ -39,14 +43,27 @@ fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { expr = cast_expr.peel_blocks(); had_at_least_one_cast = true; } - had_at_least_one_cast && cx.typeck_results().expr_ty_adjusted(expr).is_fn() + if !had_at_least_one_cast { + return None; + } + let ty = cx.typeck_results().expr_ty_adjusted(expr); + if ty.is_fn() || ty.is_ref() { + return Some(ty); + } + // Usually, references get coerced to pointers in a casting situation. + // Therefore, we give also give a look to the original type. + let ty_unadjusted = cx.typeck_results().expr_ty_opt(expr); + if let Some(ty_unadjusted) = ty_unadjusted && ty_unadjusted.is_ref() { + return Some(ty_unadjusted); + } + None } -impl<'tcx> LateLintPass<'tcx> for IncorrectFnNullChecks { +impl<'tcx> LateLintPass<'tcx> for IncorrectNonNullChecks { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { match expr.kind { // Catching: - // <* >::is_null(fn_ptr as * ) + // <* >::is_null(test_ptr as * ) ExprKind::Call(path, [arg]) if let ExprKind::Path(ref qpath) = path.kind && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() @@ -54,53 +71,60 @@ impl<'tcx> LateLintPass<'tcx> for IncorrectFnNullChecks { cx.tcx.get_diagnostic_name(def_id), Some(sym::ptr_const_is_null | sym::ptr_is_null) ) - && is_fn_ptr_cast(cx, arg) => + && let Some(ty) = is_nonnull_cast(cx, arg) => { - cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag) + let diag = NonNullCheckDiag { ty_desc: ty.prefix_string(cx.tcx) }; + cx.emit_spanned_lint(INCORRECT_NON_NULL_CHECKS, expr.span, diag) } // Catching: - // (fn_ptr as * ).is_null() + // (test_ptr as * ).is_null() ExprKind::MethodCall(_, receiver, _, _) if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) && matches!( cx.tcx.get_diagnostic_name(def_id), Some(sym::ptr_const_is_null | sym::ptr_is_null) ) - && is_fn_ptr_cast(cx, receiver) => + && let Some(ty) = is_nonnull_cast(cx, receiver) => { - cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag) + let diag = NonNullCheckDiag { ty_desc: ty.prefix_string(cx.tcx) }; + cx.emit_spanned_lint(INCORRECT_NON_NULL_CHECKS, expr.span, diag) } ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq) => { let to_check: &Expr<'_>; - if is_fn_ptr_cast(cx, left) { + let ty: Ty<'_>; + if let Some(ty_) = is_nonnull_cast(cx, left) { to_check = right; - } else if is_fn_ptr_cast(cx, right) { + ty = ty_; + } else if let Some(ty_) = is_nonnull_cast(cx, right) { to_check = left; + ty = ty_; } else { return; } match to_check.kind { // Catching: - // (fn_ptr as * ) == (0 as ) + // (test_ptr as * ) == (0 as ) ExprKind::Cast(cast_expr, _) if let ExprKind::Lit(spanned) = cast_expr.kind && let LitKind::Int(v, _) = spanned.node && v == 0 => { - cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag) + let diag = NonNullCheckDiag { ty_desc: ty.prefix_string(cx.tcx) }; + cx.emit_spanned_lint(INCORRECT_NON_NULL_CHECKS, expr.span, diag) }, // Catching: - // (fn_ptr as * ) == std::ptr::null() + // (test_ptr as * ) == std::ptr::null() ExprKind::Call(path, []) if let ExprKind::Path(ref qpath) = path.kind && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() && let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id) && (diag_item == sym::ptr_null || diag_item == sym::ptr_null_mut) => { - cx.emit_spanned_lint(INCORRECT_FN_NULL_CHECKS, expr.span, FnNullCheckDiag) + let diag = NonNullCheckDiag { ty_desc: ty.prefix_string(cx.tcx) }; + cx.emit_spanned_lint(INCORRECT_NON_NULL_CHECKS, expr.span, diag) }, _ => {}, diff --git a/tests/ui/lint/fn_null_check.rs b/tests/ui/lint/fn_null_check.rs deleted file mode 100644 index 7f01f2c428395..0000000000000 --- a/tests/ui/lint/fn_null_check.rs +++ /dev/null @@ -1,30 +0,0 @@ -// check-pass - -fn main() { - let fn_ptr = main; - - if (fn_ptr as *mut ()).is_null() {} - //~^ WARN function pointers are not nullable - if (fn_ptr as *const u8).is_null() {} - //~^ WARN function pointers are not nullable - if (fn_ptr as *const ()) == std::ptr::null() {} - //~^ WARN function pointers are not nullable - if (fn_ptr as *mut ()) == std::ptr::null_mut() {} - //~^ WARN function pointers are not nullable - if (fn_ptr as *const ()) == (0 as *const ()) {} - //~^ WARN function pointers are not nullable - if <*const _>::is_null(fn_ptr as *const ()) {} - //~^ WARN function pointers are not nullable - if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {} - //~^ WARN function pointers are not nullable - if (fn_ptr as fn() as *const ()).is_null() {} - //~^ WARN function pointers are not nullable - - const ZPTR: *const () = 0 as *const _; - const NOT_ZPTR: *const () = 1 as *const _; - - // unlike the uplifted clippy::fn_null_check lint we do - // not lint on them - if (fn_ptr as *const ()) == ZPTR {} - if (fn_ptr as *const ()) == NOT_ZPTR {} -} diff --git a/tests/ui/lint/fn_null_check.stderr b/tests/ui/lint/fn_null_check.stderr deleted file mode 100644 index 0398c0da50feb..0000000000000 --- a/tests/ui/lint/fn_null_check.stderr +++ /dev/null @@ -1,67 +0,0 @@ -warning: function pointers are not nullable, so checking them for null will always return false - --> $DIR/fn_null_check.rs:6:8 - | -LL | if (fn_ptr as *mut ()).is_null() {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value - = note: `#[warn(incorrect_fn_null_checks)]` on by default - -warning: function pointers are not nullable, so checking them for null will always return false - --> $DIR/fn_null_check.rs:8:8 - | -LL | if (fn_ptr as *const u8).is_null() {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value - -warning: function pointers are not nullable, so checking them for null will always return false - --> $DIR/fn_null_check.rs:10:8 - | -LL | if (fn_ptr as *const ()) == std::ptr::null() {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value - -warning: function pointers are not nullable, so checking them for null will always return false - --> $DIR/fn_null_check.rs:12:8 - | -LL | if (fn_ptr as *mut ()) == std::ptr::null_mut() {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value - -warning: function pointers are not nullable, so checking them for null will always return false - --> $DIR/fn_null_check.rs:14:8 - | -LL | if (fn_ptr as *const ()) == (0 as *const ()) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value - -warning: function pointers are not nullable, so checking them for null will always return false - --> $DIR/fn_null_check.rs:16:8 - | -LL | if <*const _>::is_null(fn_ptr as *const ()) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value - -warning: function pointers are not nullable, so checking them for null will always return false - --> $DIR/fn_null_check.rs:18:8 - | -LL | if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value - -warning: function pointers are not nullable, so checking them for null will always return false - --> $DIR/fn_null_check.rs:20:8 - | -LL | if (fn_ptr as fn() as *const ()).is_null() {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: wrap the function pointer inside an `Option` and use `Option::is_none` to check for null pointer value - -warning: 8 warnings emitted - diff --git a/tests/ui/lint/non_null_check.rs b/tests/ui/lint/non_null_check.rs new file mode 100644 index 0000000000000..829f4ff9cc03d --- /dev/null +++ b/tests/ui/lint/non_null_check.rs @@ -0,0 +1,45 @@ +// check-pass + +fn main() { + let fn_ptr = main; + + if (fn_ptr as *mut ()).is_null() {} + //~^ WARN can never be null, so checking + if (fn_ptr as *const u8).is_null() {} + //~^ WARN can never be null, so checking + if (fn_ptr as *const ()) == std::ptr::null() {} + //~^ WARN can never be null, so checking + if (fn_ptr as *mut ()) == std::ptr::null_mut() {} + //~^ WARN can never be null, so checking + if (fn_ptr as *const ()) == (0 as *const ()) {} + //~^ WARN can never be null, so checking + if <*const _>::is_null(fn_ptr as *const ()) {} + //~^ WARN can never be null, so checking + if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {} + //~^ WARN can never be null, so checking + if (fn_ptr as fn() as *const ()).is_null() {} + //~^ WARN can never be null, so checking + + const ZPTR: *const () = 0 as *const _; + const NOT_ZPTR: *const () = 1 as *const _; + + // unlike the uplifted clippy::fn_null_check lint we do + // not lint on them + if (fn_ptr as *const ()) == ZPTR {} + if (fn_ptr as *const ()) == NOT_ZPTR {} + + // Non fn pointers + + let tup_ref: &_ = &(10u8, 10u8); + if (tup_ref as *const (u8, u8)).is_null() {} + //~^ WARN can never be null, so checking + if (&mut (10u8, 10u8) as *mut (u8, u8)).is_null() {} + //~^ WARN can never be null, so checking + + // We could warn on these too, but don't: + if Box::into_raw(Box::new("hi")).is_null() {} + + let ptr = &mut () as *mut (); + if core::ptr::NonNull::new(ptr).unwrap().as_ptr().is_null() {} + +} diff --git a/tests/ui/lint/non_null_check.stderr b/tests/ui/lint/non_null_check.stderr new file mode 100644 index 0000000000000..bab8ecd16eb2b --- /dev/null +++ b/tests/ui/lint/non_null_check.stderr @@ -0,0 +1,83 @@ +warning: fn pointers can never be null, so checking them for null will always return false + --> $DIR/non_null_check.rs:6:8 + | +LL | if (fn_ptr as *mut ()).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the fn pointer inside an `Option` and use `Option::is_none` to check for null pointer values + = note: `#[warn(incorrect_non_null_checks)]` on by default + +warning: fn pointers can never be null, so checking them for null will always return false + --> $DIR/non_null_check.rs:8:8 + | +LL | if (fn_ptr as *const u8).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the fn pointer inside an `Option` and use `Option::is_none` to check for null pointer values + +warning: fn pointers can never be null, so checking them for null will always return false + --> $DIR/non_null_check.rs:10:8 + | +LL | if (fn_ptr as *const ()) == std::ptr::null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the fn pointer inside an `Option` and use `Option::is_none` to check for null pointer values + +warning: fn pointers can never be null, so checking them for null will always return false + --> $DIR/non_null_check.rs:12:8 + | +LL | if (fn_ptr as *mut ()) == std::ptr::null_mut() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the fn pointer inside an `Option` and use `Option::is_none` to check for null pointer values + +warning: fn pointers can never be null, so checking them for null will always return false + --> $DIR/non_null_check.rs:14:8 + | +LL | if (fn_ptr as *const ()) == (0 as *const ()) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the fn pointer inside an `Option` and use `Option::is_none` to check for null pointer values + +warning: fn pointers can never be null, so checking them for null will always return false + --> $DIR/non_null_check.rs:16:8 + | +LL | if <*const _>::is_null(fn_ptr as *const ()) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the fn pointer inside an `Option` and use `Option::is_none` to check for null pointer values + +warning: fn pointers can never be null, so checking them for null will always return false + --> $DIR/non_null_check.rs:18:8 + | +LL | if (fn_ptr as *mut fn() as *const fn() as *const ()).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the fn pointer inside an `Option` and use `Option::is_none` to check for null pointer values + +warning: fn pointers can never be null, so checking them for null will always return false + --> $DIR/non_null_check.rs:20:8 + | +LL | if (fn_ptr as fn() as *const ()).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the fn pointer inside an `Option` and use `Option::is_none` to check for null pointer values + +warning: references can never be null, so checking them for null will always return false + --> $DIR/non_null_check.rs:34:8 + | +LL | if (tup_ref as *const (u8, u8)).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the reference inside an `Option` and use `Option::is_none` to check for null pointer values + +warning: mutable references can never be null, so checking them for null will always return false + --> $DIR/non_null_check.rs:36:8 + | +LL | if (&mut (10u8, 10u8) as *mut (u8, u8)).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: wrap the mutable reference inside an `Option` and use `Option::is_none` to check for null pointer values + +warning: 10 warnings emitted +