mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #3545 - Kampfkarren:vec_boxed_sized, r=flip1995

Browse Source 
Adds lint for Vec<Box<T: Sized>>

This adds, and subsequently closes #3530. This is the first time I've ever worked with anything remotely close to internal Rust code, so I'm very much unsure about the if_chain! to figure this out!

I can't get rustfmt working on WSL with nightly 2018-12-07:

`error: component 'rustfmt' for target 'x86_64-unknown-linux-gnu' is unavailable for download`
This commit is contained in:
bors 2018-12-14 12:10:48 +00:00
commit a416c5e0f7
6 changed files with 97 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -899,6 +899,7 @@ All notable changes to this project will be documented in this file.
[`useless_let_if_seq`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_let_if_seq
[`useless_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_transmute
[`useless_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
[`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box
[`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask
[`while_immutable_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_immutable_condition
[`while_let_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop

2
README.md
View File

@ -7,7 +7,7 @@
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
[There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

2
clippy_lints/src/lib.rs
View File

@ -766,6 +766,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
types::UNIT_ARG,
types::UNIT_CMP,
types::UNNECESSARY_CAST,
types::VEC_BOX,
unicode::ZERO_WIDTH_SPACE,
unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
unused_io_amount::UNUSED_IO_AMOUNT,
@ -931,6 +932,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
types::TYPE_COMPLEXITY,
types::UNIT_ARG,
types::UNNECESSARY_CAST,
types::VEC_BOX,
unused_label::UNUSED_LABEL,
zero_div_zero::ZERO_DIVIDED_BY_ZERO,
]);

67
clippy_lints/src/types.rs
View File

@ -68,6 +68,34 @@
"usage of `Box<Vec<T>>`, vector elements are already on the heap"
}
/// **What it does:** Checks for use of `Vec<Box<T>>` where T: Sized anywhere in the code.
///
/// **Why is this bad?** `Vec` already keeps its contents in a separate area on
/// the heap. So if you `Box` its contents, you just add another level of indirection.
///
/// **Known problems:** Vec<Box<T: Sized>> makes sense if T is a large type (see #3530,
/// 1st comment).
///
/// **Example:**
/// ```rust
/// struct X {
/// values: Vec<Box<i32>>,
/// }
/// ```
///
/// Better:
///
/// ```rust
/// struct X {
/// values: Vec<i32>,
/// }
/// ```
declare_clippy_lint! {
pub VEC_BOX,
complexity,
"usage of `Vec<Box<T>>` where T: Sized, vector elements are already on the heap"
}
/// **What it does:** Checks for use of `Option<Option<_>>` in function signatures and type
/// definitions
///
@ -148,7 +176,7 @@
impl LintPass for TypePass {
fn get_lints(&self) -> LintArray {
lint_array!(BOX_VEC, OPTION_OPTION, LINKEDLIST, BORROWED_BOX)
lint_array!(BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX)
}
}
@ -238,6 +266,43 @@ fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) {
);
return; // don't recurse into the type
}
} else if match_def_path(cx.tcx, def_id, &paths::VEC) {
if_chain! {
// Get the _ part of Vec<_>
if let Some(ref last) = last_path_segment(qpath).args;
if let Some(ty) = last.args.iter().find_map(|arg| match arg {
GenericArg::Type(ty) => Some(ty),
GenericArg::Lifetime(_) => None,
});
// ty is now _ at this point
if let TyKind::Path(ref ty_qpath) = ty.node;
let def = cx.tables.qpath_def(ty_qpath, ty.hir_id);
if let Some(def_id) = opt_def_id(def);
if Some(def_id) == cx.tcx.lang_items().owned_box();
// At this point, we know ty is Box<T>, now get T
if let Some(ref last) = last_path_segment(ty_qpath).args;
if let Some(ty) = last.args.iter().find_map(|arg| match arg {
GenericArg::Type(ty) => Some(ty),
GenericArg::Lifetime(_) => None,
});
if let TyKind::Path(ref ty_qpath) = ty.node;
let def = cx.tables.qpath_def(ty_qpath, ty.hir_id);
if let Some(def_id) = opt_def_id(def);
let boxed_type = cx.tcx.type_of(def_id);
if boxed_type.is_sized(cx.tcx.at(ty.span), cx.param_env);
then {
span_lint_and_sugg(
cx,
VEC_BOX,
ast_ty.span,
"`Vec<T>` is already on the heap, the boxing is unnecessary.",
"try",
format!("Vec<{}>", boxed_type),
Applicability::MaybeIncorrect,
);
return; // don't recurse into the type
}
}
} else if match_def_path(cx.tcx, def_id, &paths::OPTION) {
if match_type_parameter(cx, qpath, &paths::OPTION) {
span_lint(

17
tests/ui/vec_box_sized.rs Normal file
View File

@ -0,0 +1,17 @@
struct SizedStruct {
_a: i32,
}
struct UnsizedStruct {
_a: [i32],
}
struct StructWithVecBox {
sized_type: Vec<Box<SizedStruct>>,
}
struct StructWithVecBoxButItsUnsized {
unsized_type: Vec<Box<UnsizedStruct>>,
}
fn main() {}

10
tests/ui/vec_box_sized.stderr Normal file
View File

@ -0,0 +1,10 @@
error: `Vec<T>` is already on the heap, the boxing is unnecessary.
--> $DIR/vec_box_sized.rs:10:14
|
10 | sized_type: Vec<Box<SizedStruct>>,
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>`
|
= note: `-D clippy::vec-box` implied by `-D warnings`
error: aborting due to previous error