diff --git a/CHANGELOG.md b/CHANGELOG.md index 2655d93599e7..b14a661d4f2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5120,6 +5120,7 @@ Released 2018-09-13 [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options [`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref +[`null_pointer_optimization`]: https://rust-lang.github.io/rust-clippy/master/index.html#null_pointer_optimization [`obfuscated_if_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#obfuscated_if_else [`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index caaad6d11736..5643f85d41fb 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -95,6 +95,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat * [`rc_mutex`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex) * [`unnecessary_box_returns`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns) * [`single_call_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#single_call_fn) +* [`null_pointer_optimization`](https://rust-lang.github.io/rust-clippy/master/index.html#null_pointer_optimization) ## `msrv` diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index d4e0d2863348..4a48118a5f86 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -644,6 +644,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::types::BORROWED_BOX_INFO, crate::types::BOX_COLLECTION_INFO, crate::types::LINKEDLIST_INFO, + crate::types::NULL_POINTER_OPTIMIZATION_INFO, crate::types::OPTION_OPTION_INFO, crate::types::RC_BUFFER_INFO, crate::types::RC_MUTEX_INFO, diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index 3c873a5901d4..82aa48ea642a 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -1,6 +1,7 @@ mod borrowed_box; mod box_collection; mod linked_list; +mod null_pointer_optimization; mod option_option; mod rc_buffer; mod rc_mutex; @@ -303,13 +304,50 @@ declare_clippy_lint! { "usage of `Rc>`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `C>` where `C` is a type that has + /// [null pointer optimization](https://doc.rust-lang.org/core/option/#representation). + /// + /// Note: There are some cases where `C>` is necessary, like getting a + /// `&mut Option` from a `Box>`. This requires ownership of the `Box`, so + /// if you have a reference this is not possible to do. + /// + /// ### Why is this bad? + /// It's slower, as `Option` can use `null` as `None`, instead of adding another layer of + /// indirection. + /// + /// ### Example + /// ```rust + /// struct MyWrapperType(Box>); + /// ``` + /// Use instead: + /// ```rust + /// struct MyWrapperType(Option>); + /// ``` + #[clippy::version = "1.73.0"] + pub NULL_POINTER_OPTIMIZATION, + perf, + "checks for `C>` where `C` is a type that has null pointer optimization" +} pub struct Types { vec_box_size_threshold: u64, type_complexity_threshold: u64, avoid_breaking_exported_api: bool, } -impl_lint_pass!(Types => [BOX_COLLECTION, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]); +impl_lint_pass!(Types => [ + BOX_COLLECTION, + VEC_BOX, + OPTION_OPTION, + LINKEDLIST, + BORROWED_BOX, + REDUNDANT_ALLOCATION, + RC_BUFFER, + RC_MUTEX, + TYPE_COMPLEXITY, + NULL_POINTER_OPTIMIZATION, +]); impl<'tcx> LateLintPass<'tcx> for Types { fn check_fn( @@ -349,10 +387,11 @@ impl<'tcx> LateLintPass<'tcx> for Types { let is_exported = cx.effective_visibilities.is_exported(item.owner_id.def_id); match item.kind { - ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => self.check_ty( + ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) | ItemKind::TyAlias(ty, _) => self.check_ty( cx, ty, CheckTyContext { + is_in_ty_alias: matches!(item.kind, ItemKind::TyAlias(..)), is_exported, ..CheckTyContext::default() }, @@ -476,7 +515,10 @@ impl Types { return; } - if !context.is_nested_call && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) { + if !context.is_nested_call + && !context.is_in_ty_alias + && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) + { return; } @@ -492,13 +534,16 @@ impl Types { // in `clippy_lints::utils::conf.rs` let mut triggered = false; - triggered |= box_collection::check(cx, hir_ty, qpath, def_id); - triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id); - triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id); - triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold); - triggered |= option_option::check(cx, hir_ty, qpath, def_id); - triggered |= linked_list::check(cx, hir_ty, def_id); - triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id); + if !context.is_in_ty_alias { + triggered |= box_collection::check(cx, hir_ty, qpath, def_id); + triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id); + triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id); + triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold); + triggered |= option_option::check(cx, hir_ty, qpath, def_id); + triggered |= linked_list::check(cx, hir_ty, def_id); + triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id); + } + triggered |= null_pointer_optimization::check(cx, hir_ty, qpath, res); if triggered { return; @@ -580,6 +625,7 @@ impl Types { #[allow(clippy::struct_excessive_bools)] #[derive(Clone, Copy, Default)] struct CheckTyContext { + is_in_ty_alias: bool, is_in_trait_impl: bool, /// `true` for types on local variables. is_local: bool, diff --git a/clippy_lints/src/types/null_pointer_optimization.rs b/clippy_lints/src/types/null_pointer_optimization.rs new file mode 100644 index 000000000000..ef6920d894ad --- /dev/null +++ b/clippy_lints/src/types/null_pointer_optimization.rs @@ -0,0 +1,35 @@ +use super::NULL_POINTER_OPTIMIZATION; +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::{is_lang_item_or_ctor, last_path_segment, match_def_path, paths}; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::{GenericArg, LangItem, QPath, Ty, TyKind}; +use rustc_lint::LateContext; + +pub(super) fn check(cx: &LateContext<'_>, ty: &Ty<'_>, qpath: &QPath<'_>, res: Res) -> bool { + if let Res::Def(DefKind::Struct, def_id) = res + && (is_lang_item_or_ctor(cx, def_id, LangItem::OwnedBox) || match_def_path(cx, def_id, &paths::PTR_NON_NULL)) + && let Some(args) = last_path_segment(qpath).args + && let GenericArg::Type(option_ty) = args.args[0] + && let TyKind::Path(option_qpath) = option_ty.kind + && let res = cx.qpath_res(&option_qpath, option_ty.hir_id) + && let Res::Def(.., def_id) = res + && is_lang_item_or_ctor(cx, def_id, LangItem::Option) + { + let outer_ty = last_path_segment(qpath).ident.name; + span_lint_and_help( + cx, + NULL_POINTER_OPTIMIZATION, + ty.span, + &format!("usage of `{outer_ty}>`"), + None, + &format!( + "consider using `Option<{outer_ty}>` instead, as it will grant better performance. For more information, see\n\ + https://doc.rust-lang.org/core/option/#representation", + ), + ); + + return true; + } + + false +} diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 58ae0656db73..1d3ffce20e62 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -290,7 +290,7 @@ define_Conf! { /// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"] /// ``` (arithmetic_side_effects_allowed_unary: rustc_data_structures::fx::FxHashSet = <_>::default()), - /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX, UNNECESSARY_BOX_RETURNS, SINGLE_CALL_FN. + /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX, UNNECESSARY_BOX_RETURNS, SINGLE_CALL_FN, NULL_POINTER_OPTIMIZATION. /// /// Suppress lints whenever the suggested change would cause breakage for other crates. (avoid_breaking_exported_api: bool = true), diff --git a/tests/ui/null_pointer_optimization.rs b/tests/ui/null_pointer_optimization.rs new file mode 100644 index 000000000000..3637a8137659 --- /dev/null +++ b/tests/ui/null_pointer_optimization.rs @@ -0,0 +1,32 @@ +#![allow(clippy::boxed_local, unused)] +#![warn(clippy::null_pointer_optimization)] +#![no_main] + +use std::marker::PhantomData; +use std::ptr::NonNull; + +type A = Box>; //~ ERROR: usage of `Box>` + +fn a(a: Box>) {} //~ ERROR: usage of `Box>` + +fn a_ty_alias(a: A) {} + +fn b(b: String) {} + +fn c(c: NonNull>) {} //~ ERROR: usage of `NonNull>` + +fn g(e: Option>>) {} //~ ERROR: usage of `Box>` + +struct H(Box>); //~ ERROR: usage of `Box>` + +enum I { + I(Box>), //~ ERROR: usage of `Box>` +} + +struct D(Option); + +fn d(d: D) {} + +fn e(e: Option>) {} + +fn f(e: Option>) {} diff --git a/tests/ui/null_pointer_optimization.stderr b/tests/ui/null_pointer_optimization.stderr new file mode 100644 index 000000000000..8908dc077ea7 --- /dev/null +++ b/tests/ui/null_pointer_optimization.stderr @@ -0,0 +1,57 @@ +error: usage of `Box>` + --> $DIR/null_pointer_optimization.rs:8:13 + | +LL | type A = Box>; + | ^^^^^^^^^^^^^^ + | + = help: consider using `Option>` instead, as it will grant better performance. For more information, see + https://doc.rust-lang.org/core/option/#representation + = note: `-D clippy::null-pointer-optimization` implied by `-D warnings` + +error: usage of `Box>` + --> $DIR/null_pointer_optimization.rs:10:12 + | +LL | fn a(a: Box>) {} + | ^^^^^^^^^^^^^^ + | + = help: consider using `Option>` instead, as it will grant better performance. For more information, see + https://doc.rust-lang.org/core/option/#representation + +error: usage of `NonNull>` + --> $DIR/null_pointer_optimization.rs:16:12 + | +LL | fn c(c: NonNull>) {} + | ^^^^^^^^^^^^^^^^^^ + | + = help: consider using `Option>` instead, as it will grant better performance. For more information, see + https://doc.rust-lang.org/core/option/#representation + +error: usage of `Box>` + --> $DIR/null_pointer_optimization.rs:18:19 + | +LL | fn g(e: Option>>) {} + | ^^^^^^^^^^^^^^ + | + = help: consider using `Option>` instead, as it will grant better performance. For more information, see + https://doc.rust-lang.org/core/option/#representation + +error: usage of `Box>` + --> $DIR/null_pointer_optimization.rs:20:13 + | +LL | struct H(Box>); + | ^^^^^^^^^^^^^^ + | + = help: consider using `Option>` instead, as it will grant better performance. For more information, see + https://doc.rust-lang.org/core/option/#representation + +error: usage of `Box>` + --> $DIR/null_pointer_optimization.rs:23:7 + | +LL | I(Box>), + | ^^^^^^^^^^^^^^ + | + = help: consider using `Option>` instead, as it will grant better performance. For more information, see + https://doc.rust-lang.org/core/option/#representation + +error: aborting due to 6 previous errors +