Skip to content

Commit a416c5e

Browse files
borsbors
committed
Auto merge of #3545 - Kampfkarren:vec_boxed_sized, r=flip1995
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`
2 parents 6e4d64a + 985eba0 commit a416c5e

File tree

6 files changed

+97
-2
lines changed

6 files changed

+97
-2
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,7 @@ All notable changes to this project will be documented in this file.
899899
[`useless_let_if_seq`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_let_if_seq
900900
[`useless_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_transmute
901901
[`useless_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
902+
[`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box
902903
[`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask
903904
[`while_immutable_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_immutable_condition
904905
[`while_let_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
99

10-
[There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
10+
[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1111

1212
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1313

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -766,6 +766,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
766766
types::UNIT_ARG,
767767
types::UNIT_CMP,
768768
types::UNNECESSARY_CAST,
769+
types::VEC_BOX,
769770
unicode::ZERO_WIDTH_SPACE,
770771
unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
771772
unused_io_amount::UNUSED_IO_AMOUNT,
@@ -931,6 +932,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
931932
types::TYPE_COMPLEXITY,
932933
types::UNIT_ARG,
933934
types::UNNECESSARY_CAST,
935+
types::VEC_BOX,
934936
unused_label::UNUSED_LABEL,
935937
zero_div_zero::ZERO_DIVIDED_BY_ZERO,
936938
]);

‎clippy_lints/src/types.rs

+66-1
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,34 @@ declare_clippy_lint! {
6868
"usage of `Box<Vec<T>>`, vector elements are already on the heap"
6969
}
7070

71+
/// **What it does:** Checks for use of `Vec<Box<T>>` where T: Sized anywhere in the code.
72+
///
73+
/// **Why is this bad?** `Vec` already keeps its contents in a separate area on
74+
/// the heap. So if you `Box` its contents, you just add another level of indirection.
75+
///
76+
/// **Known problems:** Vec<Box<T: Sized>> makes sense if T is a large type (see #3530,
77+
/// 1st comment).
78+
///
79+
/// **Example:**
80+
/// ```rust
81+
/// struct X {
82+
/// values: Vec<Box<i32>>,
83+
/// }
84+
/// ```
85+
///
86+
/// Better:
87+
///
88+
/// ```rust
89+
/// struct X {
90+
/// values: Vec<i32>,
91+
/// }
92+
/// ```
93+
declare_clippy_lint! {
94+
pub VEC_BOX,
95+
complexity,
96+
"usage of `Vec<Box<T>>` where T: Sized, vector elements are already on the heap"
97+
}
98+
7199
/// **What it does:** Checks for use of `Option<Option<_>>` in function signatures and type
72100
/// definitions
73101
///
@@ -148,7 +176,7 @@ declare_clippy_lint! {
148176

149177
impl LintPass for TypePass {
150178
fn get_lints(&self) -> LintArray {
151-
lint_array!(BOX_VEC, OPTION_OPTION, LINKEDLIST, BORROWED_BOX)
179+
lint_array!(BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX)
152180
}
153181
}
154182

@@ -238,6 +266,43 @@ fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) {
238266
);
239267
return; // don't recurse into the type
240268
}
269+
} else if match_def_path(cx.tcx, def_id, &paths::VEC) {
270+
if_chain! {
271+
// Get the _ part of Vec<_>
272+
if let Some(ref last) = last_path_segment(qpath).args;
273+
if let Some(ty) = last.args.iter().find_map(|arg| match arg {
274+
GenericArg::Type(ty) => Some(ty),
275+
GenericArg::Lifetime(_) => None,
276+
});
277+
// ty is now _ at this point
278+
if let TyKind::Path(ref ty_qpath) = ty.node;
279+
let def = cx.tables.qpath_def(ty_qpath, ty.hir_id);
280+
if let Some(def_id) = opt_def_id(def);
281+
if Some(def_id) == cx.tcx.lang_items().owned_box();
282+
// At this point, we know ty is Box<T>, now get T
283+
if let Some(ref last) = last_path_segment(ty_qpath).args;
284+
if let Some(ty) = last.args.iter().find_map(|arg| match arg {
285+
GenericArg::Type(ty) => Some(ty),
286+
GenericArg::Lifetime(_) => None,
287+
});
288+
if let TyKind::Path(ref ty_qpath) = ty.node;
289+
let def = cx.tables.qpath_def(ty_qpath, ty.hir_id);
290+
if let Some(def_id) = opt_def_id(def);
291+
let boxed_type = cx.tcx.type_of(def_id);
292+
if boxed_type.is_sized(cx.tcx.at(ty.span), cx.param_env);
293+
then {
294+
span_lint_and_sugg(
295+
cx,
296+
VEC_BOX,
297+
ast_ty.span,
298+
"`Vec<T>` is already on the heap, the boxing is unnecessary.",
299+
"try",
300+
format!("Vec<{}>", boxed_type),
301+
Applicability::MaybeIncorrect,
302+
);
303+
return; // don't recurse into the type
304+
}
305+
}
241306
} else if match_def_path(cx.tcx, def_id, &paths::OPTION) {
242307
if match_type_parameter(cx, qpath, &paths::OPTION) {
243308
span_lint(

‎tests/ui/vec_box_sized.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
struct SizedStruct {
2+
_a: i32,
3+
}
4+
5+
struct UnsizedStruct {
6+
_a: [i32],
7+
}
8+
9+
struct StructWithVecBox {
10+
sized_type: Vec<Box<SizedStruct>>,
11+
}
12+
13+
struct StructWithVecBoxButItsUnsized {
14+
unsized_type: Vec<Box<UnsizedStruct>>,
15+
}
16+
17+
fn main() {}

‎tests/ui/vec_box_sized.stderr

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: `Vec<T>` is already on the heap, the boxing is unnecessary.
2+
--> $DIR/vec_box_sized.rs:10:14
3+
|
4+
10 | sized_type: Vec<Box<SizedStruct>>,
5+
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>`
6+
|
7+
= note: `-D clippy::vec-box` implied by `-D warnings`
8+
9+
error: aborting due to previous error
10+

0 commit comments

Comments
 (0)