Skip to content

Commit

Permalink
Lower core::ops::drop directly to MIR drop
Browse files Browse the repository at this point in the history
This change causes drop() to always drop in-place. This is meant to
allow internal optimizations (see rust-lang#62508).

This does _not_ change the documented contract for drop(). Only internal
compiler code is allowed to rely on this for now.
  • Loading branch information
tmandry committed Jul 22, 2019
1 parent 95b1fe5 commit c20717e
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 30 deletions.
1 change: 1 addition & 0 deletions src/libcore/mem/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,7 @@ pub fn replace<T>(dest: &mut T, mut src: T) -> T {
/// [`Copy`]: ../../std/marker/trait.Copy.html
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[cfg_attr(not(bootstrap), lang = "drop")]
pub fn drop<T>(_x: T) { }

/// Interprets `src` as having type `&U`, and then reads `src` without moving
Expand Down
3 changes: 2 additions & 1 deletion src/libcore/ops/drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@
/// let _second = PrintOnDrop("Declared second!");
/// }
/// ```
#[lang = "drop"]
#[cfg_attr(not(bootstrap), lang = "drop_trait")]
#[cfg_attr(bootstrap, lang = "drop")]
#[stable(feature = "rust1", since = "1.0.0")]
pub trait Drop {
/// Executes the destructor for this type.
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/middle/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ language_item_table! {
SyncTraitLangItem, "sync", sync_trait, Target::Trait;
FreezeTraitLangItem, "freeze", freeze_trait, Target::Trait;

DropTraitLangItem, "drop", drop_trait, Target::Trait;
DropTraitLangItem, "drop_trait", drop_trait, Target::Trait;

CoerceUnsizedTraitLangItem, "coerce_unsized", coerce_unsized_trait, Target::Trait;
DispatchFromDynTraitLangItem,"dispatch_from_dyn", dispatch_from_dyn_trait, Target::Trait;
Expand Down Expand Up @@ -349,6 +349,7 @@ language_item_table! {

ExchangeMallocFnLangItem, "exchange_malloc", exchange_malloc_fn, Target::Fn;
BoxFreeFnLangItem, "box_free", box_free_fn, Target::Fn;
DropFnLangItem, "drop", drop_fn, Target::Fn;
DropInPlaceFnLangItem, "drop_in_place", drop_in_place_fn, Target::Fn;
OomLangItem, "oom", oom, Target::Fn;
AllocLayoutLangItem, "alloc_layout", alloc_layout, Target::Struct;
Expand Down
67 changes: 46 additions & 21 deletions src/librustc_mir/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
exit_block.unit()
}
ExprKind::Call { ty, fun, args, from_hir_call } => {
let intrinsic = match ty.sty {
let (fn_def_id, intrinsic) = match ty.sty {
ty::FnDef(def_id, _) => {
let f = ty.fn_sig(this.hir.tcx());
if f.abi() == Abi::RustIntrinsic || f.abi() == Abi::PlatformIntrinsic {
Some(this.hir.tcx().item_name(def_id).as_str())
(Some(def_id), Some(this.hir.tcx().item_name(def_id).as_str()))
} else {
None
(Some(def_id), None)
}
}
_ => None,
_ => (None, None),
};
let intrinsic = intrinsic.as_ref().map(|s| &s[..]);
let fun = unpack!(block = this.as_local_operand(block, fun));
Expand Down Expand Up @@ -237,26 +237,51 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.map(|arg| unpack!(block = this.as_local_operand(block, arg)))
.collect();

let drop_location = if fn_def_id.is_some()
&& this.hir.tcx().lang_items().drop_fn() == fn_def_id
{
assert_eq!(args.len(), 1, "drop() must have exactly one argument");
match &args[0] {
Operand::Move(place) => Some(place.clone()),
_ => None,
}
} else {
None
};

let success = this.cfg.start_new_block();
let cleanup = this.diverge_cleanup();
this.cfg.terminate(
block,
source_info,
TerminatorKind::Call {
func: fun,
args,
cleanup: Some(cleanup),
// FIXME(varkor): replace this with an uninhabitedness-based check.
// This requires getting access to the current module to call
// `tcx.is_ty_uninhabited_from`, which is currently tricky to do.
destination: if expr.ty.is_never() {
None
} else {
Some((destination.clone(), success))

if let Some(location) = drop_location {
this.cfg.terminate(
block,
source_info,
TerminatorKind::Drop {
location,
target: success,
unwind: Some(cleanup)
},
from_hir_call,
},
);
);
} else {
this.cfg.terminate(
block,
source_info,
TerminatorKind::Call {
func: fun,
args,
cleanup: Some(cleanup),
// FIXME(varkor): replace this with an uninhabitedness-based check.
// This requires getting access to the current module to call
// `tcx.is_ty_uninhabited_from`, which is currently tricky to do.
destination: if expr.ty.is_never() {
None
} else {
Some((destination.clone(), success))
},
from_hir_call,
},
);
}
success.unit()
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/box_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl Drop for S {
// StorageLive(_3);
// StorageLive(_4);
// _4 = move _1;
// _3 = const std::mem::drop::<std::boxed::Box<S>>(move _4) -> [return: bb5, unwind: bb7];
// drop(_4) -> [return: bb5, unwind: bb7];
// }
//
// bb5: {
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/issue-49232.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ fn main() {
// StorageLive(_5);
// StorageLive(_6);
// _6 = &_2;
// _5 = const std::mem::drop::<&i32>(move _6) -> [return: bb13, unwind: bb4];
// drop(_6) -> [return: bb13, unwind: bb4];
// }
// bb13: {
// StorageDead(_6);
Expand Down
4 changes: 3 additions & 1 deletion src/test/ui/type_length_limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ link! { F, G }

pub struct G;

fn take<T>(x: T) {}

fn main() {
drop::<Option<A>>(None);
take::<Option<A>>(None);
}
8 changes: 4 additions & 4 deletions src/test/ui/type_length_limit.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: reached the type-length limit while instantiating `std::mem::drop::<std::option::Op... G), (G, G, G), (G, G, G))))))>>`
--> $SRC_DIR/libcore/mem/mod.rs:LL:COL
error: reached the type-length limit while instantiating `take::<std::option::Option<(((((... G), (G, G, G), (G, G, G))))))>>`
--> $DIR/type_length_limit.rs:25:1
|
LL | pub fn drop<T>(_x: T) { }
| ^^^^^^^^^^^^^^^^^^^^^^^^^
LL | fn take<T>(x: T) {}
| ^^^^^^^^^^^^^^^^^^^
|
= note: consider adding a `#![type_length_limit="1094"]` attribute to your crate

Expand Down

0 comments on commit c20717e

Please sign in to comment.