Skip to content

Commit 9dbcf93

Browse files
Centri3Centri3
committed
move to types group
1 parent 093d4d1 commit 9dbcf93

File tree

9 files changed

+104
-98
lines changed

9 files changed

+104
-98
lines changed

‎book/src/lint_configuration.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat
9595
* [`rc_mutex`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex)
9696
* [`unnecessary_box_returns`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns)
9797
* [`single_call_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#single_call_fn)
98+
* [`null_pointer_optimization`](https://rust-lang.github.io/rust-clippy/master/index.html#null_pointer_optimization)
9899

99100

100101
## `msrv`

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
501501
crate::non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS_INFO,
502502
crate::non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY_INFO,
503503
crate::nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES_INFO,
504-
crate::null_pointer_optimization::NULL_POINTER_OPTIMIZATION_INFO,
505504
crate::octal_escapes::OCTAL_ESCAPES_INFO,
506505
crate::only_used_in_recursion::ONLY_USED_IN_RECURSION_INFO,
507506
crate::operators::ABSURD_EXTREME_COMPARISONS_INFO,
@@ -645,6 +644,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
645644
crate::types::BORROWED_BOX_INFO,
646645
crate::types::BOX_COLLECTION_INFO,
647646
crate::types::LINKEDLIST_INFO,
647+
crate::types::NULL_POINTER_OPTIMIZATION_INFO,
648648
crate::types::OPTION_OPTION_INFO,
649649
crate::types::RC_BUFFER_INFO,
650650
crate::types::RC_MUTEX_INFO,

‎clippy_lints/src/lib.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,6 @@ mod non_expressive_names;
246246
mod non_octal_unix_permissions;
247247
mod non_send_fields_in_send_ty;
248248
mod nonstandard_macro_braces;
249-
mod null_pointer_optimization;
250249
mod octal_escapes;
251250
mod only_used_in_recursion;
252251
mod operators;
@@ -1071,9 +1070,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10711070
Box::new(single_call_fn::SingleCallFn {
10721071
avoid_breaking_exported_api,
10731072
def_id_to_usage: rustc_data_structures::fx::FxHashMap::default(),
1074-
store.register_late_pass(move |_| {
1075-
Box::new(null_pointer_optimization::NullPointerOptimization {
1076-
avoid_breaking_exported_api,
10771073
})
10781074
});
10791075
let needless_raw_string_hashes_allow_one = conf.allow_one_hash_in_raw_strings;

‎clippy_lints/src/null_pointer_optimization.rs‎

Lines changed: 0 additions & 70 deletions
This file was deleted.

‎clippy_lints/src/types/mod.rs‎

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
mod borrowed_box;
22
mod box_collection;
33
mod linked_list;
4+
mod null_pointer_optimization;
45
mod option_option;
56
mod rc_buffer;
67
mod rc_mutex;
@@ -303,13 +304,50 @@ declare_clippy_lint! {
303304
"usage of `Rc<Mutex<T>>`"
304305
}
305306

307+
declare_clippy_lint! {
308+
/// ### What it does
309+
/// Checks for `C<Option<T>>` where `C` is a type that has
310+
/// [null pointer optimization](https://doc.rust-lang.org/core/option/#representation).
311+
///
312+
/// Note: There are some cases where `C<Option<T>>` is necessary, like getting a
313+
/// `&mut Option<T>` from a `Box<Option<T>>`. This requires ownership of the `Box`, so
314+
/// if you have a reference this is not possible to do.
315+
///
316+
/// ### Why is this bad?
317+
/// It's slower, as `Option` can use `null` as `None`, instead of adding another layer of
318+
/// indirection.
319+
///
320+
/// ### Example
321+
/// ```rust
322+
/// struct MyWrapperType<T>(Box<Option<T>>);
323+
/// ```
324+
/// Use instead:
325+
/// ```rust
326+
/// struct MyWrapperType<T>(Option<Box<T>>);
327+
/// ```
328+
#[clippy::version = "1.73.0"]
329+
pub NULL_POINTER_OPTIMIZATION,
330+
perf,
331+
"checks for `C<Option<T>>` where `C` is a type that has null pointer optimization"
332+
}
306333
pub struct Types {
307334
vec_box_size_threshold: u64,
308335
type_complexity_threshold: u64,
309336
avoid_breaking_exported_api: bool,
310337
}
311338

312-
impl_lint_pass!(Types => [BOX_COLLECTION, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]);
339+
impl_lint_pass!(Types => [
340+
BOX_COLLECTION,
341+
VEC_BOX,
342+
OPTION_OPTION,
343+
LINKEDLIST,
344+
BORROWED_BOX,
345+
REDUNDANT_ALLOCATION,
346+
RC_BUFFER,
347+
RC_MUTEX,
348+
TYPE_COMPLEXITY,
349+
NULL_POINTER_OPTIMIZATION,
350+
]);
313351

314352
impl<'tcx> LateLintPass<'tcx> for Types {
315353
fn check_fn(
@@ -349,10 +387,11 @@ impl<'tcx> LateLintPass<'tcx> for Types {
349387
let is_exported = cx.effective_visibilities.is_exported(item.owner_id.def_id);
350388

351389
match item.kind {
352-
ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => self.check_ty(
390+
ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) | ItemKind::TyAlias(ty, _) => self.check_ty(
353391
cx,
354392
ty,
355393
CheckTyContext {
394+
is_in_ty_alias: matches!(item.kind, ItemKind::TyAlias(..)),
356395
is_exported,
357396
..CheckTyContext::default()
358397
},
@@ -476,7 +515,10 @@ impl Types {
476515
return;
477516
}
478517

479-
if !context.is_nested_call && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) {
518+
if !context.is_nested_call
519+
&& !context.is_in_ty_alias
520+
&& type_complexity::check(cx, hir_ty, self.type_complexity_threshold)
521+
{
480522
return;
481523
}
482524

@@ -492,13 +534,16 @@ impl Types {
492534
// in `clippy_lints::utils::conf.rs`
493535

494536
let mut triggered = false;
495-
triggered |= box_collection::check(cx, hir_ty, qpath, def_id);
496-
triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id);
497-
triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id);
498-
triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold);
499-
triggered |= option_option::check(cx, hir_ty, qpath, def_id);
500-
triggered |= linked_list::check(cx, hir_ty, def_id);
501-
triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id);
537+
if !context.is_in_ty_alias {
538+
triggered |= box_collection::check(cx, hir_ty, qpath, def_id);
539+
triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id);
540+
triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id);
541+
triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold);
542+
triggered |= option_option::check(cx, hir_ty, qpath, def_id);
543+
triggered |= linked_list::check(cx, hir_ty, def_id);
544+
triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id);
545+
}
546+
triggered |= null_pointer_optimization::check(cx, hir_ty, qpath, res);
502547

503548
if triggered {
504549
return;
@@ -580,6 +625,7 @@ impl Types {
580625
#[allow(clippy::struct_excessive_bools)]
581626
#[derive(Clone, Copy, Default)]
582627
struct CheckTyContext {
628+
is_in_ty_alias: bool,
583629
is_in_trait_impl: bool,
584630
/// `true` for types on local variables.
585631
is_local: bool,

‎clippy_lints/src/types/null_pointer_optimization.rs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
use super::NULL_POINTER_OPTIMIZATION;
2+
use clippy_utils::diagnostics::span_lint_and_help;
3+
use clippy_utils::{is_lang_item_or_ctor, last_path_segment, match_def_path, paths};
4+
use rustc_hir::def::{DefKind, Res};
5+
use rustc_hir::{GenericArg, LangItem, QPath, Ty, TyKind};
6+
use rustc_lint::LateContext;
7+
8+
pub(super) fn check(cx: &LateContext<'_>, ty: &Ty<'_>, qpath: &QPath<'_>, res: Res) -> bool {
9+
if let Res::Def(DefKind::Struct, def_id) = res
10+
&& (is_lang_item_or_ctor(cx, def_id, LangItem::OwnedBox) || match_def_path(cx, def_id, &paths::PTR_NON_NULL))
11+
&& let Some(args) = last_path_segment(qpath).args
12+
&& let GenericArg::Type(option_ty) = args.args[0]
13+
&& let TyKind::Path(option_qpath) = option_ty.kind
14+
&& let res = cx.qpath_res(&option_qpath, option_ty.hir_id)
15+
&& let Res::Def(.., def_id) = res
16+
&& is_lang_item_or_ctor(cx, def_id, LangItem::Option)
17+
{
18+
let outer_ty = last_path_segment(qpath).ident.name;
19+
span_lint_and_help(
20+
cx,
21+
NULL_POINTER_OPTIMIZATION,
22+
ty.span,
23+
&format!("usage of `{outer_ty}<Option<T>>`"),
24+
None,
25+
&format!(
26+
"consider using `Option<{outer_ty}<T>>` instead, as it will grant better performance. For more information, see\n\
27+
https://doc.rust-lang.org/core/option/#representation",
28+
),
29+
);
30+
31+
return true;
32+
}
33+
34+
false
35+
}

‎clippy_lints/src/utils/conf.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ define_Conf! {
290290
/// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
291291
/// ```
292292
(arithmetic_side_effects_allowed_unary: rustc_data_structures::fx::FxHashSet<String> = <_>::default()),
293-
/// 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.
293+
/// 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.
294294
///
295295
/// Suppress lints whenever the suggested change would cause breakage for other crates.
296296
(avoid_breaking_exported_api: bool = true),

‎tests/ui/null_pointer_optimization.rs‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,27 @@
22
#![warn(clippy::null_pointer_optimization)]
33
#![no_main]
44

5+
use std::marker::PhantomData;
56
use std::ptr::NonNull;
67

7-
type A<T> = Box<Option<T>>;
8+
type A<T> = Box<Option<T>>; //~ ERROR: usage of `Box<Option<T>>`
89

9-
fn a<T>(a: Box<Option<T>>) {}
10+
fn a<T>(a: Box<Option<T>>) {} //~ ERROR: usage of `Box<Option<T>>`
1011

11-
// lint at `type A<T>`, but not here.
1212
fn a_ty_alias<T>(a: A<T>) {}
1313

1414
fn b(b: String) {}
1515

16-
fn c<T>(c: NonNull<Option<T>>) {}
16+
fn c<T>(c: NonNull<Option<T>>) {} //~ ERROR: usage of `NonNull<Option<T>>`
1717

18-
fn g<T>(e: Option<Box<Option<T>>>) {}
18+
fn g<T>(e: Option<Box<Option<T>>>) {} //~ ERROR: usage of `Box<Option<T>>`
1919

20-
struct H<T>(Box<Option<T>>);
20+
struct H<T>(Box<Option<T>>); //~ ERROR: usage of `Box<Option<T>>`
2121

2222
enum I<T> {
23-
J(Box<Option<T>>),
23+
I(Box<Option<T>>), //~ ERROR: usage of `Box<Option<T>>`
2424
}
2525

26-
// do not lint
27-
2826
struct D<T>(Option<T>);
2927

3028
fn d<T>(d: D<T>) {}

‎tests/ui/null_pointer_optimization.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: usage of `Box<Option<T>>`
2-
--> $DIR/null_pointer_optimization.rs:7:13
2+
--> $DIR/null_pointer_optimization.rs:8:13
33
|
44
LL | type A<T> = Box<Option<T>>;
55
| ^^^^^^^^^^^^^^
@@ -9,7 +9,7 @@ LL | type A<T> = Box<Option<T>>;
99
= note: `-D clippy::null-pointer-optimization` implied by `-D warnings`
1010

1111
error: usage of `Box<Option<T>>`
12-
--> $DIR/null_pointer_optimization.rs:9:12
12+
--> $DIR/null_pointer_optimization.rs:10:12
1313
|
1414
LL | fn a<T>(a: Box<Option<T>>) {}
1515
| ^^^^^^^^^^^^^^
@@ -47,7 +47,7 @@ LL | struct H<T>(Box<Option<T>>);
4747
error: usage of `Box<Option<T>>`
4848
--> $DIR/null_pointer_optimization.rs:23:7
4949
|
50-
LL | J(Box<Option<T>>),
50+
LL | I(Box<Option<T>>),
5151
| ^^^^^^^^^^^^^^
5252
|
5353
= help: consider using `Option<Box<T>>` instead, as it will grant better performance. For more information, see

0 commit comments

Comments
 (0)