From e030eb109489e71c059075d31b75b6752a99c0d0 Mon Sep 17 00:00:00 2001 From: est31 Date: Mon, 6 Jun 2022 00:49:41 +0200 Subject: [PATCH 1/4] Make Box::new use owned_box_new lang item --- compiler/rustc_hir/src/lang_items.rs | 1 + compiler/rustc_span/src/symbol.rs | 1 + library/alloc/src/boxed.rs | 1 + 3 files changed, 3 insertions(+) diff --git a/compiler/rustc_hir/src/lang_items.rs b/compiler/rustc_hir/src/lang_items.rs index c337be12ae492..1f95da8bff20e 100644 --- a/compiler/rustc_hir/src/lang_items.rs +++ b/compiler/rustc_hir/src/lang_items.rs @@ -276,6 +276,7 @@ language_item_table! { EhCatchTypeinfo, sym::eh_catch_typeinfo, eh_catch_typeinfo, Target::Static, GenericRequirement::None; OwnedBox, sym::owned_box, owned_box, Target::Struct, GenericRequirement::Minimum(1); + OwnedBoxNew, sym::owned_box_new, owned_box_new, Target::Method(MethodKind::Inherent), GenericRequirement::None; PhantomData, sym::phantom_data, phantom_data, Target::Struct, GenericRequirement::Exact(1); diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 2f3519e3edd77..a726882314d6e 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1037,6 +1037,7 @@ symbols! { out, overlapping_marker_traits, owned_box, + owned_box_new, packed, panic, panic_2015, diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index c1ceeb0deb837..0be0adf3033d9 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -207,6 +207,7 @@ impl Box { /// let five = Box::new(5); /// ``` #[cfg(all(not(no_global_oom_handling)))] + #[lang = "owned_box_new"] #[inline(always)] #[stable(feature = "rust1", since = "1.0.0")] #[must_use] From 7ca8ff0719e96202964d0eaae731ef53a2a125d7 Mon Sep 17 00:00:00 2001 From: est31 Date: Mon, 6 Jun 2022 01:11:03 +0200 Subject: [PATCH 2/4] Also lint Box::new in the unused_allocation lint Previously, only (box foo).something() would be linted, wher something takes &self or &mut self. Now, we also lint for Box::new(foo).something(). --- compiler/rustc_lint/src/unused.rs | 36 ++++++++++++++----- .../iterators/into-iter-on-arrays-lint.fixed | 2 +- .../ui/iterators/into-iter-on-arrays-lint.rs | 2 +- .../ui/self/arbitrary_self_types_trait.rs | 1 + src/test/ui/structs-enums/align-struct.rs | 1 + 5 files changed, 32 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 4e7ba1c6ce4fa..3730486f5ce62 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -1149,9 +1149,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(); /// } /// ``` /// @@ -1159,8 +1158,8 @@ declare_lint! { /// /// ### Explanation /// - /// When a `box` expression is immediately coerced to a reference, then - /// the allocation is unnecessary, and a reference (using `&` or `&mut`) + /// When a `Box::new()` expression is immediately coerced to a reference, + /// then the allocation is unnecessary, and a reference (using `&` or `&mut`) /// should be used instead to avoid the allocation. pub(super) UNUSED_ALLOCATION, Warn, @@ -1171,14 +1170,35 @@ declare_lint_pass!(UnusedAllocation => [UNUSED_ALLOCATION]); impl<'tcx> LateLintPass<'tcx> for UnusedAllocation { fn check_expr(&mut self, cx: &LateContext<'_>, e: &hir::Expr<'_>) { - match e.kind { - hir::ExprKind::Box(_) => {} + let span = match e.kind { + hir::ExprKind::Box(_) => { + // Ideally, we'd underline the `box` part of the expression here, + // but at this point we have lost the span of the `box` keyword. + // Constructing a span from the inner and the entire expression's + // span won't work in many cases, so we just return the entire + // span of the `box foo`. + e.span + } + hir::ExprKind::Call(ref callee, _) => { + // Look for Box::new(foo) + if let hir::ExprKind::Path(ref qpath) = callee.kind && + // `Res::Local` indicates a closure + let Res::Def(_, def_id) = cx.qpath_res(qpath, callee.hir_id) && + let Some(owned_box_new_def_id) = cx.tcx.lang_items().owned_box_new() && + def_id == owned_box_new_def_id + { + // We have a Box::new() call here + callee.span + } else { + return + } + } _ => return, - } + }; for adj in cx.typeck_results().expr_adjustments(e) { if let adjustment::Adjust::Borrow(adjustment::AutoBorrow::Ref(_, m)) = adj.kind { - cx.struct_span_lint(UNUSED_ALLOCATION, e.span, |lint| { + cx.struct_span_lint(UNUSED_ALLOCATION, span, |lint| { lint.build(match m { adjustment::AutoBorrowMutability::Not => fluent::lint::unused_allocation, adjustment::AutoBorrowMutability::Mut { .. } => { diff --git a/src/test/ui/iterators/into-iter-on-arrays-lint.fixed b/src/test/ui/iterators/into-iter-on-arrays-lint.fixed index 6e02a7024b949..5b91aaf9ea554 100644 --- a/src/test/ui/iterators/into-iter-on-arrays-lint.fixed +++ b/src/test/ui/iterators/into-iter-on-arrays-lint.fixed @@ -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]; diff --git a/src/test/ui/iterators/into-iter-on-arrays-lint.rs b/src/test/ui/iterators/into-iter-on-arrays-lint.rs index 582d5cadd0658..25b0cef73d777 100644 --- a/src/test/ui/iterators/into-iter-on-arrays-lint.rs +++ b/src/test/ui/iterators/into-iter-on-arrays-lint.rs @@ -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]; diff --git a/src/test/ui/self/arbitrary_self_types_trait.rs b/src/test/ui/self/arbitrary_self_types_trait.rs index 973c7cae85a94..d4624ef84ef74 100644 --- a/src/test/ui/self/arbitrary_self_types_trait.rs +++ b/src/test/ui/self/arbitrary_self_types_trait.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_allocation)] use std::rc::Rc; diff --git a/src/test/ui/structs-enums/align-struct.rs b/src/test/ui/structs-enums/align-struct.rs index f5418e754b239..cfbb1107230e3 100644 --- a/src/test/ui/structs-enums/align-struct.rs +++ b/src/test/ui/structs-enums/align-struct.rs @@ -1,5 +1,6 @@ // run-pass #![allow(dead_code)] +#![allow(unused_allocation)] use std::mem; From 37c193af278cc37f5bd3584b5061678d2393694c Mon Sep 17 00:00:00 2001 From: est31 Date: Mon, 6 Jun 2022 01:27:01 +0200 Subject: [PATCH 3/4] Add test for unused_allocation Before the unused_allocation lint has never been tested outside of its lint documentation. --- src/test/ui/lint/unused/unused-allocation.rs | 10 ++++++ .../ui/lint/unused/unused-allocation.stderr | 32 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 src/test/ui/lint/unused/unused-allocation.rs create mode 100644 src/test/ui/lint/unused/unused-allocation.stderr diff --git a/src/test/ui/lint/unused/unused-allocation.rs b/src/test/ui/lint/unused/unused-allocation.rs new file mode 100644 index 0000000000000..9e23f802b8968 --- /dev/null +++ b/src/test/ui/lint/unused/unused-allocation.rs @@ -0,0 +1,10 @@ +#![feature(box_syntax)] +#![deny(unused_allocation)] + +fn main() { + let foo = (box [1, 2, 3]).len(); //~ ERROR: unnecessary allocation + let one = (box vec![1]).pop(); //~ ERROR: unnecessary allocation + + let foo = Box::new([1, 2, 3]).len(); //~ ERROR: unnecessary allocation + let one = Box::new(vec![1]).pop(); //~ ERROR: unnecessary allocation +} diff --git a/src/test/ui/lint/unused/unused-allocation.stderr b/src/test/ui/lint/unused/unused-allocation.stderr new file mode 100644 index 0000000000000..9103712970c06 --- /dev/null +++ b/src/test/ui/lint/unused/unused-allocation.stderr @@ -0,0 +1,32 @@ +error: unnecessary allocation, use `&` instead + --> $DIR/unused-allocation.rs:5:15 + | +LL | let foo = (box [1, 2, 3]).len(); + | ^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/unused-allocation.rs:2:9 + | +LL | #![deny(unused_allocation)] + | ^^^^^^^^^^^^^^^^^ + +error: unnecessary allocation, use `&mut` instead + --> $DIR/unused-allocation.rs:6:15 + | +LL | let one = (box vec![1]).pop(); + | ^^^^^^^^^^^^^ + +error: unnecessary allocation, use `&` instead + --> $DIR/unused-allocation.rs:8:15 + | +LL | let foo = Box::new([1, 2, 3]).len(); + | ^^^^^^^^ + +error: unnecessary allocation, use `&mut` instead + --> $DIR/unused-allocation.rs:9:15 + | +LL | let one = Box::new(vec![1]).pop(); + | ^^^^^^^^ + +error: aborting due to 4 previous errors + From e94a9dbf2a852dc80f4def27587d9f85e4ae6480 Mon Sep 17 00:00:00 2001 From: est31 Date: Mon, 6 Jun 2022 04:10:09 +0200 Subject: [PATCH 4/4] Fix unused_allocation warning in alloc test --- library/alloc/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/tests.rs b/library/alloc/src/tests.rs index 299ed156a5d27..ff06a6a6bf504 100644 --- a/library/alloc/src/tests.rs +++ b/library/alloc/src/tests.rs @@ -25,13 +25,13 @@ fn any_move() { match a.downcast::() { Ok(a) => { - assert!(a == Box::new(8)); + assert!(*a == 8); } Err(..) => panic!(), } match b.downcast::() { Ok(a) => { - assert!(a == Box::new(Test)); + assert!(*a == Test); } Err(..) => panic!(), }