1
Fork 0

Rollup merge of #104363 - WaffleLapkin:bonk_box_new, r=Nilstrieb

Make `unused_allocation` lint against `Box::new` too

Previously it only linted against `box` syntax, which likely won't ever be stabilized, which is pretty useless. Even now I'm not sure if it's a meaningful lint, but it's at least something 🤷

This means that code like the following will be linted against:
```rust
Box::new([1, 2, 3]).len();
f(&Box::new(1)); // where f : &i32 -> ()
```
The lint works by checking if a `Box::new` (or `box`) expression has an a borrow adjustment, meaning that the code that first stores the box in a variable won't be linted against:
```rust
let boxed = Box::new([1, 2, 3]); // no lint
boxed.len();
```
This commit is contained in:
Matthias Krüger 2023-03-11 15:43:11 +01:00 committed by GitHub
commit fbc121fdfd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 59 additions and 39 deletions

View file

@ -1349,9 +1349,8 @@ declare_lint! {
/// ### Example /// ### Example
/// ///
/// ```rust /// ```rust
/// #![feature(box_syntax)]
/// fn main() { /// fn main() {
/// let a = (box [1, 2, 3]).len(); /// let a = Box::new([1, 2, 3]).len();
/// } /// }
/// ``` /// ```
/// ///
@ -1373,6 +1372,11 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAllocation {
fn check_expr(&mut self, cx: &LateContext<'_>, e: &hir::Expr<'_>) { fn check_expr(&mut self, cx: &LateContext<'_>, e: &hir::Expr<'_>) {
match e.kind { match e.kind {
hir::ExprKind::Box(_) => {} hir::ExprKind::Box(_) => {}
hir::ExprKind::Call(path_expr, [_])
if let hir::ExprKind::Path(qpath) = &path_expr.kind
&& let Some(did) = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id()
&& cx.tcx.is_diagnostic_item(sym::box_new, did)
=> {}
_ => return, _ => return,
} }

View file

@ -429,6 +429,7 @@ symbols! {
borrowck_graphviz_format, borrowck_graphviz_format,
borrowck_graphviz_postflow, borrowck_graphviz_postflow,
box_free, box_free,
box_new,
box_patterns, box_patterns,
box_syntax, box_syntax,
bpf_target_feature, bpf_target_feature,

View file

@ -214,6 +214,7 @@ impl<T> Box<T> {
#[inline(always)] #[inline(always)]
#[stable(feature = "rust1", since = "1.0.0")] #[stable(feature = "rust1", since = "1.0.0")]
#[must_use] #[must_use]
#[rustc_diagnostic_item = "box_new"]
pub fn new(x: T) -> Self { pub fn new(x: T) -> Self {
#[rustc_box] #[rustc_box]
Box::new(x) Box::new(x)

View file

@ -4,7 +4,6 @@ use core::any::Any;
use core::clone::Clone; use core::clone::Clone;
use core::convert::TryInto; use core::convert::TryInto;
use core::ops::Deref; use core::ops::Deref;
use core::result::Result::{Err, Ok};
use std::boxed::Box; use std::boxed::Box;
@ -15,7 +14,7 @@ fn test_owned_clone() {
assert!(a == b); assert!(a == b);
} }
#[derive(PartialEq, Eq)] #[derive(Debug, PartialEq, Eq)]
struct Test; struct Test;
#[test] #[test]
@ -23,24 +22,17 @@ fn any_move() {
let a = Box::new(8) as Box<dyn Any>; let a = Box::new(8) as Box<dyn Any>;
let b = Box::new(Test) as Box<dyn Any>; let b = Box::new(Test) as Box<dyn Any>;
match a.downcast::<i32>() { let a: Box<i32> = a.downcast::<i32>().unwrap();
Ok(a) => { assert_eq!(*a, 8);
assert!(a == Box::new(8));
} let b: Box<Test> = b.downcast::<Test>().unwrap();
Err(..) => panic!(), assert_eq!(*b, Test);
}
match b.downcast::<Test>() {
Ok(a) => {
assert!(a == Box::new(Test));
}
Err(..) => panic!(),
}
let a = Box::new(8) as Box<dyn Any>; let a = Box::new(8) as Box<dyn Any>;
let b = Box::new(Test) as Box<dyn Any>; let b = Box::new(Test) as Box<dyn Any>;
assert!(a.downcast::<Box<Test>>().is_err()); assert!(a.downcast::<Box<i32>>().is_err());
assert!(b.downcast::<Box<i32>>().is_err()); assert!(b.downcast::<Box<Test>>().is_err());
} }
#[test] #[test]

View file

@ -2,9 +2,7 @@
// error-pattern:so long // error-pattern:so long
// ignore-emscripten no processes // ignore-emscripten no processes
#![allow(unused_allocation)]
#![allow(unreachable_code)] #![allow(unreachable_code)]
#![allow(unused_variables)]
fn main() { fn main() {
let mut x = Vec::new(); let mut x = Vec::new();

View file

@ -2,7 +2,7 @@
// run-rustfix // run-rustfix
// rustfix-only-machine-applicable // rustfix-only-machine-applicable
#[allow(unused_must_use)] #[allow(unused_must_use, unused_allocation)]
fn main() { fn main() {
let small = [1, 2]; let small = [1, 2];
let big = [0u8; 33]; let big = [0u8; 33];

View file

@ -2,7 +2,7 @@
// run-rustfix // run-rustfix
// rustfix-only-machine-applicable // rustfix-only-machine-applicable
#[allow(unused_must_use)] #[allow(unused_must_use, unused_allocation)]
fn main() { fn main() {
let small = [1, 2]; let small = [1, 2];
let big = [0u8; 33]; let big = [0u8; 33];

View file

@ -0,0 +1,7 @@
#![feature(rustc_attrs, stmt_expr_attributes)]
#![deny(unused_allocation)]
fn main() {
_ = (#[rustc_box] Box::new([1])).len(); //~ error: unnecessary allocation, use `&` instead
_ = Box::new([1]).len(); //~ error: unnecessary allocation, use `&` instead
}

View file

@ -0,0 +1,20 @@
error: unnecessary allocation, use `&` instead
--> $DIR/unused-allocation.rs:5:9
|
LL | _ = (#[rustc_box] Box::new([1])).len();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/unused-allocation.rs:2:9
|
LL | #![deny(unused_allocation)]
| ^^^^^^^^^^^^^^^^^
error: unnecessary allocation, use `&` instead
--> $DIR/unused-allocation.rs:6:9
|
LL | _ = Box::new([1]).len();
| ^^^^^^^^^^^^^
error: aborting due to 2 previous errors

View file

@ -1,4 +1,5 @@
// run-pass // run-pass
#![allow(unused_allocation)]
use std::rc::Rc; use std::rc::Rc;
@ -13,7 +14,7 @@ impl Trait for Vec<i32> {
} }
fn main() { fn main() {
let v = vec![1,2,3]; let v = vec![1, 2, 3];
assert_eq!(&[1,2,3], Box::new(Rc::new(v)).trait_method()); assert_eq!(&[1, 2, 3], Box::new(Rc::new(v)).trait_method());
} }

View file

@ -1,5 +1,5 @@
// run-pass // run-pass
#![allow(dead_code)] #![allow(dead_code, unused_allocation)]
use std::mem; use std::mem;
@ -20,7 +20,6 @@ struct AlignMany(i32);
// Raising alignment may not alter size. // Raising alignment may not alter size.
#[repr(align(8))] #[repr(align(8))]
#[allow(dead_code)]
struct Align8Many { struct Align8Many {
a: i32, a: i32,
b: i32, b: i32,
@ -29,9 +28,8 @@ struct Align8Many {
} }
enum Enum { enum Enum {
#[allow(dead_code)]
A(i32), A(i32),
B(Align16) B(Align16),
} }
// Nested alignment - use `#[repr(C)]` to suppress field reordering for sizeof test // Nested alignment - use `#[repr(C)]` to suppress field reordering for sizeof test
@ -73,7 +71,7 @@ struct AlignLarge {
union UnionContainsAlign { union UnionContainsAlign {
a: Align16, a: Align16,
b: f32 b: f32,
} }
impl Align16 { impl Align16 {
@ -158,7 +156,7 @@ pub fn main() {
// Note that the size of Nested may change if struct field re-ordering is enabled // Note that the size of Nested may change if struct field re-ordering is enabled
assert_eq!(mem::align_of::<Nested>(), 16); assert_eq!(mem::align_of::<Nested>(), 16);
assert_eq!(mem::size_of::<Nested>(), 48); assert_eq!(mem::size_of::<Nested>(), 48);
let a = Nested{ a: 1, b: 2, c: Align16(3), d: 4}; let a = Nested { a: 1, b: 2, c: Align16(3), d: 4 };
assert_eq!(mem::align_of_val(&a), 16); assert_eq!(mem::align_of_val(&a), 16);
assert_eq!(mem::align_of_val(&a.b), 4); assert_eq!(mem::align_of_val(&a.b), 4);
assert_eq!(mem::align_of_val(&a.c), 16); assert_eq!(mem::align_of_val(&a.c), 16);
@ -179,8 +177,8 @@ pub fn main() {
assert_eq!(a.0, 15); assert_eq!(a.0, 15);
assert_eq!(mem::align_of_val(a), 16); assert_eq!(mem::align_of_val(a), 16);
assert_eq!(mem::size_of_val(a), 16); assert_eq!(mem::size_of_val(a), 16);
}, }
_ => () _ => (),
} }
assert!(is_aligned_to(&e, 16)); assert!(is_aligned_to(&e, 16));
@ -197,8 +195,8 @@ pub fn main() {
} }
// arrays of aligned elements should also be aligned // arrays of aligned elements should also be aligned
assert_eq!(mem::align_of::<[Align16;2]>(), 16); assert_eq!(mem::align_of::<[Align16; 2]>(), 16);
assert_eq!(mem::size_of::<[Align16;2]>(), 32); assert_eq!(mem::size_of::<[Align16; 2]>(), 32);
let a = [Align16(0), Align16(1)]; let a = [Align16(0), Align16(1)];
assert_eq!(mem::align_of_val(&a[0]), 16); assert_eq!(mem::align_of_val(&a[0]), 16);
@ -209,7 +207,7 @@ pub fn main() {
assert_eq!(mem::align_of_val(Box::new(Align16(0)).as_ref()), 16); assert_eq!(mem::align_of_val(Box::new(Align16(0)).as_ref()), 16);
// check heap array is aligned // check heap array is aligned
let a = vec!(Align16(0), Align16(1)); let a = vec![Align16(0), Align16(1)];
assert_eq!(mem::align_of_val(&a[0]), 16); assert_eq!(mem::align_of_val(&a[0]), 16);
assert_eq!(mem::align_of_val(&a[1]), 16); assert_eq!(mem::align_of_val(&a[1]), 16);
@ -224,16 +222,14 @@ pub fn main() {
assert_eq!(mem::align_of::<AlignContainsPacked4C>(), 16); assert_eq!(mem::align_of::<AlignContainsPacked4C>(), 16);
assert_eq!(mem::size_of::<AlignContainsPacked4C>(), 32); assert_eq!(mem::size_of::<AlignContainsPacked4C>(), 32);
let a = AlignContainsPacked4C { a: Packed4C{ a: 1, b: 2 }, b: 3 }; let a = AlignContainsPacked4C { a: Packed4C { a: 1, b: 2 }, b: 3 };
assert_eq!(mem::align_of_val(&a), 16); assert_eq!(mem::align_of_val(&a), 16);
assert_eq!(mem::align_of_val(&a.a), 4); assert_eq!(mem::align_of_val(&a.a), 4);
assert_eq!(mem::align_of_val(&a.b), mem::align_of::<u64>()); assert_eq!(mem::align_of_val(&a.b), mem::align_of::<u64>());
assert_eq!(mem::size_of_val(&a), 32); assert_eq!(mem::size_of_val(&a), 32);
assert!(is_aligned_to(&a, 16)); assert!(is_aligned_to(&a, 16));
let mut large = Box::new(AlignLarge { let mut large = Box::new(AlignLarge { stuff: [0; 0x10000] });
stuff: [0; 0x10000],
});
large.stuff[0] = 132; large.stuff[0] = 132;
*large.stuff.last_mut().unwrap() = 102; *large.stuff.last_mut().unwrap() = 102;
assert_eq!(large.stuff[0], 132); assert_eq!(large.stuff[0], 132);