Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make unused_allocation lint against Box::new too #104363

Merged
merged 6 commits into from
Mar 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1349,9 +1349,8 @@ declare_lint! {
/// ### Example
///
/// ```rust
/// #![feature(box_syntax)]
/// fn main() {
/// let a = (box [1, 2, 3]).len();
/// let a = Box::new([1, 2, 3]).len();
/// }
/// ```
///
Expand All @@ -1373,6 +1372,11 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAllocation {
fn check_expr(&mut self, cx: &LateContext<'_>, e: &hir::Expr<'_>) {
match e.kind {
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,
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ symbols! {
borrowck_graphviz_format,
borrowck_graphviz_postflow,
box_free,
box_new,
box_patterns,
box_syntax,
bpf_target_feature,
Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ impl<T> Box<T> {
#[inline(always)]
#[stable(feature = "rust1", since = "1.0.0")]
#[must_use]
#[rustc_diagnostic_item = "box_new"]
pub fn new(x: T) -> Self {
#[rustc_box]
Box::new(x)
Expand Down
24 changes: 8 additions & 16 deletions library/alloc/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use core::any::Any;
use core::clone::Clone;
use core::convert::TryInto;
use core::ops::Deref;
use core::result::Result::{Err, Ok};

use std::boxed::Box;

Expand All @@ -15,32 +14,25 @@ fn test_owned_clone() {
assert!(a == b);
}

#[derive(PartialEq, Eq)]
#[derive(Debug, PartialEq, Eq)]
struct Test;

#[test]
fn any_move() {
let a = Box::new(8) as Box<dyn Any>;
let b = Box::new(Test) as Box<dyn Any>;

match a.downcast::<i32>() {
Ok(a) => {
assert!(a == Box::new(8));
}
Err(..) => panic!(),
}
match b.downcast::<Test>() {
Ok(a) => {
assert!(a == Box::new(Test));
}
Err(..) => panic!(),
}
let a: Box<i32> = a.downcast::<i32>().unwrap();
assert_eq!(*a, 8);

let b: Box<Test> = b.downcast::<Test>().unwrap();
assert_eq!(*b, Test);

let a = Box::new(8) as Box<dyn Any>;
let b = Box::new(Test) as Box<dyn Any>;

assert!(a.downcast::<Box<Test>>().is_err());
assert!(b.downcast::<Box<i32>>().is_err());
assert!(a.downcast::<Box<i32>>().is_err());
assert!(b.downcast::<Box<Test>>().is_err());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a fair amount of squinting before I realized why this change was an improvement to the test. :)

}

#[test]
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/issues/issue-3029.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
// error-pattern:so long
// ignore-emscripten no processes

#![allow(unused_allocation)]
#![allow(unreachable_code)]
#![allow(unused_variables)]

fn main() {
let mut x = Vec::new();
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/iterators/into-iter-on-arrays-lint.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// run-rustfix
// rustfix-only-machine-applicable

#[allow(unused_must_use)]
#[allow(unused_must_use, unused_allocation)]
fn main() {
let small = [1, 2];
let big = [0u8; 33];
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/iterators/into-iter-on-arrays-lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// run-rustfix
// rustfix-only-machine-applicable

#[allow(unused_must_use)]
#[allow(unused_must_use, unused_allocation)]
fn main() {
let small = [1, 2];
let big = [0u8; 33];
Expand Down
7 changes: 7 additions & 0 deletions tests/ui/lint/unused/unused-allocation.rs
Original file line number Diff line number Diff line change
@@ -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
}
20 changes: 20 additions & 0 deletions tests/ui/lint/unused/unused-allocation.stderr
Original file line number Diff line number Diff line change
@@ -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

5 changes: 3 additions & 2 deletions tests/ui/self/arbitrary_self_types_trait.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// run-pass
#![allow(unused_allocation)]

use std::rc::Rc;

Expand All @@ -13,7 +14,7 @@ impl Trait for Vec<i32> {
}

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());
}
26 changes: 11 additions & 15 deletions tests/ui/structs-enums/align-struct.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// run-pass
#![allow(dead_code)]
#![allow(dead_code, unused_allocation)]

use std::mem;

Expand All @@ -20,7 +20,6 @@ struct AlignMany(i32);

// Raising alignment may not alter size.
#[repr(align(8))]
#[allow(dead_code)]
struct Align8Many {
a: i32,
b: i32,
Expand All @@ -29,9 +28,8 @@ struct Align8Many {
}

enum Enum {
#[allow(dead_code)]
A(i32),
B(Align16)
B(Align16),
}

// Nested alignment - use `#[repr(C)]` to suppress field reordering for sizeof test
Expand Down Expand Up @@ -73,7 +71,7 @@ struct AlignLarge {

union UnionContainsAlign {
a: Align16,
b: f32
b: f32,
}

impl Align16 {
Expand Down Expand Up @@ -158,7 +156,7 @@ pub fn main() {
// 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::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.b), 4);
assert_eq!(mem::align_of_val(&a.c), 16);
Expand All @@ -179,8 +177,8 @@ pub fn main() {
assert_eq!(a.0, 15);
assert_eq!(mem::align_of_val(a), 16);
assert_eq!(mem::size_of_val(a), 16);
},
_ => ()
}
_ => (),
}
assert!(is_aligned_to(&e, 16));

Expand All @@ -197,8 +195,8 @@ pub fn main() {
}

// arrays of aligned elements should also be aligned
assert_eq!(mem::align_of::<[Align16;2]>(), 16);
assert_eq!(mem::size_of::<[Align16;2]>(), 32);
assert_eq!(mem::align_of::<[Align16; 2]>(), 16);
assert_eq!(mem::size_of::<[Align16; 2]>(), 32);

let a = [Align16(0), Align16(1)];
assert_eq!(mem::align_of_val(&a[0]), 16);
Expand All @@ -209,7 +207,7 @@ pub fn main() {
assert_eq!(mem::align_of_val(Box::new(Align16(0)).as_ref()), 16);

// 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[1]), 16);

Expand All @@ -224,16 +222,14 @@ pub fn main() {

assert_eq!(mem::align_of::<AlignContainsPacked4C>(), 16);
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.a), 4);
assert_eq!(mem::align_of_val(&a.b), mem::align_of::<u64>());
assert_eq!(mem::size_of_val(&a), 32);
assert!(is_aligned_to(&a, 16));

let mut large = Box::new(AlignLarge {
stuff: [0; 0x10000],
});
let mut large = Box::new(AlignLarge { stuff: [0; 0x10000] });
large.stuff[0] = 132;
*large.stuff.last_mut().unwrap() = 102;
assert_eq!(large.stuff[0], 132);
Expand Down