Skip to content

Commit 741b684

Browse files
authored
New lint - same_length_and_capacity (#15656)
Fixes #5955 I understand that there's a feature freeze right now. I had started working on this before I was aware of the feature freeze. I don't mind waiting as long as I need to for feedback. changelog: [`same_length_and_capacity`]: adds a new lint that checks for usages of `from_raw_parts` where the same expression is passed for the length and the capacity <!-- TRIAGEBOT_START --> <!-- TRIAGEBOT_SUMMARY_START --> ### Summary Notes - [Feature-freeze](#15656 (comment)) by [github-actions[bot]](https://github.com/github-actions[bot]) *Managed by `@rustbot`—see [help](https://forge.rust-lang.org/triagebot/note.html) for details* <!-- TRIAGEBOT_SUMMARY_END --> <!-- TRIAGEBOT_END -->
2 parents 6110c80 + b1993dc commit 741b684

File tree

7 files changed

+157
-0
lines changed

7 files changed

+157
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6916,6 +6916,7 @@ Released 2018-09-13
69166916
[`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
69176917
[`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition
69186918
[`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push
6919+
[`same_length_and_capacity`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_length_and_capacity
69196920
[`same_name_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_name_method
69206921
[`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
69216922
[`seek_from_current`]: https://rust-lang.github.io/rust-clippy/master/index.html#seek_from_current

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
667667
crate::returns::LET_AND_RETURN_INFO,
668668
crate::returns::NEEDLESS_RETURN_INFO,
669669
crate::returns::NEEDLESS_RETURN_WITH_QUESTION_MARK_INFO,
670+
crate::same_length_and_capacity::SAME_LENGTH_AND_CAPACITY_INFO,
670671
crate::same_name_method::SAME_NAME_METHOD_INFO,
671672
crate::self_named_constructors::SELF_NAMED_CONSTRUCTORS_INFO,
672673
crate::semicolon_block::SEMICOLON_INSIDE_BLOCK_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ mod replace_box;
318318
mod reserve_after_initialization;
319319
mod return_self_not_must_use;
320320
mod returns;
321+
mod same_length_and_capacity;
321322
mod same_name_method;
322323
mod self_named_constructors;
323324
mod semicolon_block;
@@ -855,6 +856,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
855856
Box::new(|_| Box::new(volatile_composites::VolatileComposites)),
856857
Box::new(|_| Box::<replace_box::ReplaceBox>::default()),
857858
Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))),
859+
Box::new(|_| Box::new(same_length_and_capacity::SameLengthAndCapacity)),
858860
// add late passes here, used by `cargo dev new_lint`
859861
];
860862
store.late_passes.extend(late_lints);
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
use clippy_utils::diagnostics::span_lint_and_help;
2+
use clippy_utils::res::MaybeDef;
3+
use clippy_utils::{eq_expr_value, sym};
4+
use rustc_hir::{Expr, ExprKind, LangItem, QPath};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_session::declare_lint_pass;
7+
use rustc_span::symbol::sym as rustc_sym;
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
///
12+
/// Checks for usages of `Vec::from_raw_parts` and `String::from_raw_parts`
13+
/// where the same expression is used for the length and the capacity.
14+
///
15+
/// ### Why is this bad?
16+
///
17+
/// If the same expression is being passed for the length and
18+
/// capacity, it is most likely a semantic error. In the case of a
19+
/// Vec, for example, the only way to end up with one that has
20+
/// the same length and capacity is by going through a boxed slice,
21+
/// e.g. `Box::from(some_vec)`, which shrinks the capacity to match
22+
/// the length.
23+
///
24+
/// ### Example
25+
///
26+
/// ```no_run
27+
/// #![feature(vec_into_raw_parts)]
28+
/// let mut original: Vec::<i32> = Vec::with_capacity(20);
29+
/// original.extend([1, 2, 3, 4, 5]);
30+
///
31+
/// let (ptr, mut len, cap) = original.into_raw_parts();
32+
///
33+
/// // I will add three more integers:
34+
/// unsafe {
35+
/// let ptr = ptr as *mut i32;
36+
///
37+
/// for i in 6..9 {
38+
/// *ptr.add(i - 1) = i as i32;
39+
/// len += 1;
40+
/// }
41+
/// }
42+
///
43+
/// // But I forgot the capacity was separate from the length:
44+
/// let reconstructed = unsafe { Vec::from_raw_parts(ptr, len, len) };
45+
/// ```
46+
///
47+
/// Use instead:
48+
/// ```no_run
49+
/// #![feature(vec_into_raw_parts)]
50+
/// let mut original: Vec::<i32> = Vec::with_capacity(20);
51+
/// original.extend([1, 2, 3, 4, 5]);
52+
///
53+
/// let (ptr, mut len, cap) = original.into_raw_parts();
54+
///
55+
/// // I will add three more integers:
56+
/// unsafe {
57+
/// let ptr = ptr as *mut i32;
58+
///
59+
/// for i in 6..9 {
60+
/// *ptr.add(i - 1) = i as i32;
61+
/// len += 1;
62+
/// }
63+
/// }
64+
///
65+
/// // This time, leverage the previously saved capacity:
66+
/// let reconstructed = unsafe { Vec::from_raw_parts(ptr, len, cap) };
67+
/// ```
68+
#[clippy::version = "1.93.0"]
69+
pub SAME_LENGTH_AND_CAPACITY,
70+
pedantic,
71+
"`from_raw_parts` with same length and capacity"
72+
}
73+
declare_lint_pass!(SameLengthAndCapacity => [SAME_LENGTH_AND_CAPACITY]);
74+
75+
impl<'tcx> LateLintPass<'tcx> for SameLengthAndCapacity {
76+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
77+
if let ExprKind::Call(path_expr, args) = expr.kind
78+
&& let ExprKind::Path(QPath::TypeRelative(ty, fn_path)) = path_expr.kind
79+
&& fn_path.ident.name == sym::from_raw_parts
80+
&& args.len() >= 3
81+
&& eq_expr_value(cx, &args[1], &args[2])
82+
{
83+
let middle_ty = cx.typeck_results().node_type(ty.hir_id);
84+
if middle_ty.is_diag_item(cx, rustc_sym::Vec) {
85+
span_lint_and_help(
86+
cx,
87+
SAME_LENGTH_AND_CAPACITY,
88+
expr.span,
89+
"usage of `Vec::from_raw_parts` with the same expression for length and capacity",
90+
None,
91+
"try `Box::from(slice::from_raw_parts(...)).into::<Vec<_>>()`",
92+
);
93+
} else if middle_ty.is_lang_item(cx, LangItem::String) {
94+
span_lint_and_help(
95+
cx,
96+
SAME_LENGTH_AND_CAPACITY,
97+
expr.span,
98+
"usage of `String::from_raw_parts` with the same expression for length and capacity",
99+
None,
100+
"try `String::from(str::from_utf8_unchecked(slice::from_raw_parts(...)))`",
101+
);
102+
}
103+
}
104+
}
105+
}

clippy_utils/src/sym.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ generate! {
168168
from_ne_bytes,
169169
from_ptr,
170170
from_raw,
171+
from_raw_parts,
171172
from_str_radix,
172173
fs,
173174
fuse,
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#![warn(clippy::same_length_and_capacity)]
2+
3+
fn main() {
4+
let mut my_vec: Vec<i32> = Vec::with_capacity(20);
5+
my_vec.extend([1, 2, 3, 4, 5]);
6+
let (ptr, mut len, cap) = my_vec.into_raw_parts();
7+
len = 8;
8+
9+
let _reconstructed_vec = unsafe { Vec::from_raw_parts(ptr, len, len) };
10+
//~^ same_length_and_capacity
11+
12+
// Don't want to lint different expressions for len and cap
13+
let _properly_reconstructed_vec = unsafe { Vec::from_raw_parts(ptr, len, cap) };
14+
15+
// Don't want to lint if len and cap are distinct variables but happen to be equal
16+
let len_from_cap = cap;
17+
let _another_properly_reconstructed_vec = unsafe { Vec::from_raw_parts(ptr, len_from_cap, cap) };
18+
19+
let my_string = String::from("hello");
20+
let (string_ptr, string_len, string_cap) = my_string.into_raw_parts();
21+
22+
let _reconstructed_string = unsafe { String::from_raw_parts(string_ptr, string_len, string_len) };
23+
//~^ same_length_and_capacity
24+
25+
// Don't want to lint different expressions for len and cap
26+
let _properly_reconstructed_string = unsafe { String::from_raw_parts(string_ptr, string_len, string_cap) };
27+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: usage of `Vec::from_raw_parts` with the same expression for length and capacity
2+
--> tests/ui/same_length_and_capacity.rs:9:39
3+
|
4+
LL | let _reconstructed_vec = unsafe { Vec::from_raw_parts(ptr, len, len) };
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= help: try `Box::from(slice::from_raw_parts(...)).into::<Vec<_>>()`
8+
= note: `-D clippy::same-length-and-capacity` implied by `-D warnings`
9+
= help: to override `-D warnings` add `#[allow(clippy::same_length_and_capacity)]`
10+
11+
error: usage of `String::from_raw_parts` with the same expression for length and capacity
12+
--> tests/ui/same_length_and_capacity.rs:22:42
13+
|
14+
LL | let _reconstructed_string = unsafe { String::from_raw_parts(string_ptr, string_len, string_len) };
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
|
17+
= help: try `String::from(str::from_utf8_unchecked(slice::from_raw_parts(...)))`
18+
19+
error: aborting due to 2 previous errors
20+

0 commit comments

Comments
 (0)