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

Experiment: Remove #[rustc_box] usage in the vec![] macro #135068

Closed
wants to merge 2 commits into from
Closed
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
5 changes: 0 additions & 5 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -933,11 +933,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
"#[rustc_has_incoherent_inherent_impls] allows the addition of incoherent inherent impls for \
the given type by annotating all impl items with #[rustc_allow_incoherent_impl]."
),
rustc_attr!(
rustc_box, AttributeType::Normal, template!(Word), ErrorFollowing, EncodeCrossCrate::No,
"#[rustc_box] allows creating boxes \
and it is only intended to be used in `alloc`."
),

BuiltinAttribute {
name: sym::rustc_diagnostic_item,
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -
| sym::assert_inhabited
| sym::assert_zero_valid
| sym::assert_mem_uninitialized_valid
| sym::box_new
| sym::breakpoint
| sym::size_of
| sym::min_align_of
Expand Down Expand Up @@ -606,6 +607,8 @@ pub fn check_intrinsic_type(

sym::ub_checks => (0, 0, Vec::new(), tcx.types.bool),

sym::box_new => (1, 0, vec![param(0)], Ty::new_box(tcx, param(0))),

sym::simd_eq
| sym::simd_ne
| sym::simd_lt
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,6 @@ mir_build_privately_uninhabited = pattern `{$witness_1}` is currently uninhabite
mir_build_rust_2024_incompatible_pat = this pattern relies on behavior which may change in edition 2024
mir_build_rustc_box_attribute_error = `#[rustc_box]` attribute used incorrectly
.attributes = no other attributes may be applied
.not_box = `#[rustc_box]` may only be applied to a `Box::new()` call
.missing_box = `#[rustc_box]` requires the `owned_box` lang item
mir_build_static_in_pattern = statics cannot be referenced in patterns
.label = can't be used in patterns
mir_build_static_in_pattern_def = `static` defined here
Expand Down
19 changes: 0 additions & 19 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1067,25 +1067,6 @@ pub(crate) enum MiscPatternSuggestion {
},
}

#[derive(Diagnostic)]
#[diag(mir_build_rustc_box_attribute_error)]
pub(crate) struct RustcBoxAttributeError {
#[primary_span]
pub(crate) span: Span,
#[subdiagnostic]
pub(crate) reason: RustcBoxAttrReason,
}

#[derive(Subdiagnostic)]
pub(crate) enum RustcBoxAttrReason {
#[note(mir_build_attributes)]
Attributes,
#[note(mir_build_not_box)]
NotBoxNew,
#[note(mir_build_missing_box)]
MissingBox,
}

#[derive(LintDiagnostic)]
#[diag(mir_build_rust_2024_incompatible_pat)]
pub(crate) struct Rust2024IncompatiblePat<'a> {
Expand Down
57 changes: 18 additions & 39 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use rustc_middle::{bug, span_bug};
use rustc_span::{Span, sym};
use tracing::{debug, info, instrument, trace};

use crate::errors;
use crate::thir::cx::Cx;
use crate::thir::util::UserAnnotatedTyHelpers;

Expand Down Expand Up @@ -380,45 +379,25 @@ impl<'tcx> Cx<'tcx> {
from_hir_call: true,
fn_span: expr.span,
}
} else {
let attrs = tcx.hir().attrs(expr.hir_id);
if attrs.iter().any(|a| a.name_or_empty() == sym::rustc_box) {
if attrs.len() != 1 {
tcx.dcx().emit_err(errors::RustcBoxAttributeError {
span: attrs[0].span,
reason: errors::RustcBoxAttrReason::Attributes,
});
} else if let Some(box_item) = tcx.lang_items().owned_box() {
if let hir::ExprKind::Path(hir::QPath::TypeRelative(ty, fn_path)) =
fun.kind
&& let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = ty.kind
&& path.res.opt_def_id().is_some_and(|did| did == box_item)
&& fn_path.ident.name == sym::new
&& let [value] = args
{
return Expr {
temp_lifetime: TempLifetime {
temp_lifetime,
backwards_incompatible,
},
ty: expr_ty,
span: expr.span,
kind: ExprKind::Box { value: self.mirror_expr(value) },
};
} else {
tcx.dcx().emit_err(errors::RustcBoxAttributeError {
span: expr.span,
reason: errors::RustcBoxAttrReason::NotBoxNew,
});
}
} else {
tcx.dcx().emit_err(errors::RustcBoxAttributeError {
span: attrs[0].span,
reason: errors::RustcBoxAttrReason::MissingBox,
});
}
} else if let ty::FnDef(def_id, _) = self.typeck_results().expr_ty(fun).kind()
&& let Some(intrinsic) = self.tcx.intrinsic(def_id)
&& intrinsic.name == sym::box_new
{
// We don't actually evaluate `fun` here, so make sure that doesn't miss any side-effects.
if !matches!(fun.kind, hir::ExprKind::Path(_)) {
span_bug!(
expr.span,
"`box_new` intrinsic can only be called via path expression"
);
}

let value = &args[0];
return Expr {
temp_lifetime: TempLifetime { temp_lifetime, backwards_incompatible },
ty: expr_ty,
span: expr.span,
kind: ExprKind::Box { value: self.mirror_expr(value) },
};
} else {
// Tuple-like ADTs are represented as ExprKind::Call. We convert them here.
let adt_data = if let hir::ExprKind::Path(ref qpath) = fun.kind
&& let Some(adt_def) = expr_ty.ty_adt_def()
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1696,7 +1696,6 @@ symbols! {
rustc_as_ptr,
rustc_attrs,
rustc_autodiff,
rustc_box,
rustc_builtin_macro,
rustc_capture_analysis,
rustc_clean,
Expand Down
2 changes: 1 addition & 1 deletion library/alloc/src/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ unsafe impl Allocator for Global {
}
}

/// The allocator for unique pointers.
/// The allocator for `Box`.
#[cfg(all(not(no_global_oom_handling), not(test)))]
#[lang = "exchange_malloc"]
#[inline]
Expand Down
24 changes: 22 additions & 2 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,27 @@ pub struct Box<
#[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global,
>(Unique<T>, A);

/// Constructs a `Box<T>` by calling the `exchange_malloc` lang item and moving the argument into
/// the newly allocated memory. This is an intrinsic to avoid unnecessary copies.
///
/// This is the surface syntax for `box <expr>` expressions.
Copy link
Member

Choose a reason for hiding this comment

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

This comment is now wildly outdated. We haven't had exchange_malloc nor box <expr> for years.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's from #135046 :D

Copy link
Member Author

Choose a reason for hiding this comment

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

It does call exchange_malloc though:

let exchange_malloc = Operand::function_handle(

Copy link
Member

@RalfJung RalfJung Jan 3, 2025

Choose a reason for hiding this comment

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

This intrinsic is how we write box <expr>, so in that sense it still exists.

And yeah, nobody ever bothered to rename exchange_malloc 😂

Copy link
Member Author

Choose a reason for hiding this comment

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

box <expr> doesn't exist as syntax in the language anymore: #108471

Copy link
Member

Choose a reason for hiding this comment

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

Box expressions still exist in the compiler, and this intrinsic are their syntax.

#[cfg(not(bootstrap))]
#[rustc_intrinsic]
#[rustc_intrinsic_must_be_overridden]
#[unstable(feature = "liballoc_internals", issue = "none")]
pub fn box_new<T>(_x: T) -> Box<T> {
unreachable!()
}

/// Transition function for the next bootstrap bump.
#[cfg(bootstrap)]
#[unstable(feature = "liballoc_internals", issue = "none")]
#[inline(always)]
pub fn box_new<T>(x: T) -> Box<T> {
#[rustc_box]
Box::new(x)
}

impl<T> Box<T> {
/// Allocates memory on the heap and then places `x` into it.
///
Expand All @@ -250,8 +271,7 @@ impl<T> Box<T> {
#[rustc_diagnostic_item = "box_new"]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub fn new(x: T) -> Self {
#[rustc_box]
Box::new(x)
return box_new(x);
}

/// Constructs a new box with uninitialized contents.
Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@
#![feature(dropck_eyepatch)]
#![feature(fundamental)]
#![feature(hashmap_internals)]
#![feature(intrinsics)]
#![feature(lang_items)]
#![feature(min_specialization)]
#![feature(multiple_supertrait_upcastable)]
Expand Down
3 changes: 0 additions & 3 deletions library/alloc/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ macro_rules! vec {
);
($($x:expr),+ $(,)?) => (
<[_]>::into_vec(
// This rustc_box is not required, but it produces a dramatic improvement in compile
// time when constructing arrays with many elements.
#[rustc_box]
$crate::boxed::Box::new([$($x),+])
)
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
error: use of a disallowed macro `std::vec`
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:16:5
|
LL | vec![1, 2, 3];
| ^^^^^^^^^^^^^
|
= note: `-D clippy::disallowed-macros` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::disallowed_macros)]`

error: use of a disallowed macro `serde::Serialize`
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:18:14
|
LL | #[derive(Serialize)]
| ^^^^^^^^^
|
= note: no serializing
= note: `-D clippy::disallowed-macros` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::disallowed_macros)]`

error: use of a disallowed macro `macros::attr`
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:31:1
Expand Down Expand Up @@ -47,6 +40,12 @@ error: use of a disallowed macro `std::cfg`
LL | cfg!(unix);
| ^^^^^^^^^^

error: use of a disallowed macro `std::vec`
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:16:5
|
LL | vec![1, 2, 3];
| ^^^^^^^^^^^^^

error: use of a disallowed macro `macros::expr`
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:21:13
|
Expand Down
5 changes: 2 additions & 3 deletions tests/mir-opt/box_expr.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//@ test-mir-pass: ElaborateDrops
//@ needs-unwind

#![feature(rustc_attrs, stmt_expr_attributes)]
#![feature(rustc_attrs, liballoc_internals)]

// EMIT_MIR box_expr.main.ElaborateDrops.diff
fn main() {
Expand All @@ -17,8 +17,7 @@ fn main() {
// CHECK: [[boxref:_.*]] = &mut [[box]];
// CHECK: <Box<S> as Drop>::drop(move [[boxref]])

let x = #[rustc_box]
Box::new(S::new());
let x = std::boxed::box_new(S::new());
drop(x);
}

Expand Down
16 changes: 3 additions & 13 deletions tests/mir-opt/building/uniform_array_move_out.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,15 @@
// skip-filecheck
#![feature(stmt_expr_attributes, rustc_attrs)]
#![feature(liballoc_internals, rustc_attrs)]

// EMIT_MIR uniform_array_move_out.move_out_from_end.built.after.mir
fn move_out_from_end() {
let a = [
#[rustc_box]
Box::new(1),
#[rustc_box]
Box::new(2),
];
let a = [std::boxed::box_new(1), std::boxed::box_new(2)];
let [.., _y] = a;
}

// EMIT_MIR uniform_array_move_out.move_out_by_subslice.built.after.mir
fn move_out_by_subslice() {
let a = [
#[rustc_box]
Box::new(1),
#[rustc_box]
Box::new(2),
];
let a = [std::boxed::box_new(1), std::boxed::box_new(2)];
let [_y @ ..] = a;
}

Expand Down
6 changes: 2 additions & 4 deletions tests/mir-opt/const_prop/boxes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//@ compile-flags: -O
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY

#![feature(rustc_attrs, stmt_expr_attributes)]
#![feature(rustc_attrs, liballoc_internals)]

// Note: this test verifies that we, in fact, do not const prop `#[rustc_box]`

Expand All @@ -13,7 +13,5 @@ fn main() {
// CHECK: (*{{_.*}}) = const 42_i32;
// CHECK: [[tmp:_.*]] = copy (*{{_.*}});
// CHECK: [[x]] = copy [[tmp]];
let x = *(#[rustc_box]
Box::new(42))
+ 0;
let x = *(std::boxed::box_new(42)) + 0;
}
7 changes: 2 additions & 5 deletions tests/mir-opt/issue_62289.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,11 @@
// initializing it
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY

#![feature(rustc_attrs)]
#![feature(rustc_attrs, liballoc_internals)]

// EMIT_MIR issue_62289.test.ElaborateDrops.before.mir
fn test() -> Option<Box<u32>> {
Some(
#[rustc_box]
Box::new(None?),
)
Some(std::boxed::box_new(None?))
}

fn main() {
Expand Down
18 changes: 0 additions & 18 deletions tests/ui/attributes/rustc-box.rs

This file was deleted.

34 changes: 0 additions & 34 deletions tests/ui/attributes/rustc-box.stderr

This file was deleted.

4 changes: 2 additions & 2 deletions tests/ui/consts/min_const_fn/bad_const_fn_body_ice.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const fn foo(a: i32) -> Vec<i32> {
vec![1, 2, 3]
//~^ ERROR allocations are not allowed
//~| ERROR cannot call non-const method
//~^ ERROR cannot call non-const
//~| ERROR cannot call non-const
}

fn main() {}
Loading
Loading