Skip to content

Commit

Permalink
move to types group
Browse files Browse the repository at this point in the history
  • Loading branch information
Centri3 committed Jun 19, 2023
1 parent 28f9f53 commit cf628b1
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 86 deletions.
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Expand Up @@ -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`
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/declared_lints.rs
Expand Up @@ -485,7 +485,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS_INFO,
crate::non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY_INFO,
crate::nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES_INFO,
crate::null_pointer_optimization::NULL_POINTER_OPTIMIZATION_INFO,
crate::octal_escapes::OCTAL_ESCAPES_INFO,
crate::only_used_in_recursion::ONLY_USED_IN_RECURSION_INFO,
crate::operators::ABSURD_EXTREME_COMPARISONS_INFO,
Expand Down Expand Up @@ -625,6 +624,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,
Expand Down
4 changes: 0 additions & 4 deletions clippy_lints/src/lib.rs
Expand Up @@ -242,7 +242,6 @@ mod non_expressive_names;
mod non_octal_unix_permissions;
mod non_send_fields_in_send_ty;
mod nonstandard_macro_braces;
mod null_pointer_optimization;
mod octal_escapes;
mod only_used_in_recursion;
mod operators;
Expand Down Expand Up @@ -1061,9 +1060,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
Box::new(single_call_fn::SingleCallFn {
avoid_breaking_exported_api,
def_id_to_usage: rustc_data_structures::fx::FxHashMap::default(),
store.register_late_pass(move |_| {
Box::new(null_pointer_optimization::NullPointerOptimization {
avoid_breaking_exported_api,
})
});
// add lints here, do not remove this comment, it's used in `new_lint`
Expand Down
70 changes: 0 additions & 70 deletions clippy_lints/src/null_pointer_optimization.rs

This file was deleted.

66 changes: 56 additions & 10 deletions 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;
Expand Down Expand Up @@ -303,13 +304,50 @@ declare_clippy_lint! {
"usage of `Rc<Mutex<T>>`"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for `C<Option<T>>` 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<Option<T>>` is necessary, like getting a
/// `&mut Option<T>` from a `Box<Option<T>>`. This is not possible with `Option<Box<T>>` without
/// unstable features, currently.
///
/// ### 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<T>(Box<Option<T>>);
/// ```
/// Use instead:
/// ```rust
/// struct MyWrapperType<T>(Option<Box<T>>);
/// ```
#[clippy::version = "1.72.0"]
pub NULL_POINTER_OPTIMIZATION,
perf,
"checks for `C<Option<T>>` 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(
Expand Down Expand Up @@ -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()
},
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
39 changes: 39 additions & 0 deletions clippy_lints/src/types/null_pointer_optimization.rs
@@ -0,0 +1,39 @@
use clippy_utils::{diagnostics::span_lint_and_help, is_lang_item_or_ctor, last_path_segment, match_def_path, paths};
use rustc_hir::{
def::{DefKind, Res},
GenericArg, LangItem, QPath, Ty, TyKind,
};
use rustc_lint::LateContext;

use super::NULL_POINTER_OPTIMIZATION;

pub(super) fn check(cx: &LateContext<'_>, ty: &Ty<'_>, qpath: &QPath<'_>, res: Res) -> bool {
if let Res::Def(DefKind::Struct, def_id) = res {
if !(is_lang_item_or_ctor(cx, def_id, LangItem::OwnedBox) || match_def_path(cx, def_id, &paths::PTR_NON_NULL)) {
return false;
}

if 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}<Option<T>>`"),
None,
&format!("consider using `Option<{outer_ty}<T>>` instead, as it will grant better performance. For more information, see\n\
https://doc.rust-lang.org/core/option/#representation"),
);

return true;
}
}

false
}
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Expand Up @@ -289,7 +289,7 @@ define_Conf! {
/// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
/// ```
(arithmetic_side_effects_allowed_unary: rustc_data_structures::fx::FxHashSet<String> = <_>::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),
Expand Down

0 comments on commit cf628b1

Please sign in to comment.