Use more precise `type_needs_drop` for deciding about emitting cleanup code.

Fix #22536

10 files changed, 69 insertions, 17 deletions

diff --git a/src/librustc_trans/trans/callee.rs b/src/librustc_trans/trans/callee.rs
index 59fcd5492eb..4f234fac9a4 100644
--- a/src/librustc_trans/trans/callee.rs
+++ b/src/librustc_trans/trans/callee.rs
@@ -734,7 +734,7 @@ pub fn trans_call_inner<'a, 'blk, 'tcx, F>(bcx: Block<'blk, 'tcx>,
             };
             if !is_rust_fn ||
               type_of::return_uses_outptr(ccx, ret_ty) ||
-              common::type_needs_drop(bcx.tcx(), ret_ty) {
+              bcx.fcx.type_needs_drop(ret_ty) {
                 // Push the out-pointer if we use an out-pointer for this
                 // return type, otherwise push "undef".
                 if common::type_is_zero_size(ccx, ret_ty) {
diff --git a/src/librustc_trans/trans/cleanup.rs b/src/librustc_trans/trans/cleanup.rs
index cb9156be479..ad07f3953cc 100644
--- a/src/librustc_trans/trans/cleanup.rs
+++ b/src/librustc_trans/trans/cleanup.rs
@@ -386,7 +386,7 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
                          cleanup_scope: ScopeId,
                          val: ValueRef,
                          ty: Ty<'tcx>) {
-        if !common::type_needs_drop(self.ccx.tcx(), ty) { return; }
+        if !self.type_needs_drop(ty) { return; }
         let drop = box DropValue {
             is_immediate: false,
             must_unwind: common::type_needs_unwind_cleanup(self.ccx, ty),
@@ -408,7 +408,8 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
                                   cleanup_scope: ScopeId,
                                   val: ValueRef,
                                   ty: Ty<'tcx>) {
-        if !common::type_needs_drop(self.ccx.tcx(), ty) { return; }
+        if !self.type_needs_drop(ty) { return; }
+
         let drop = box DropValue {
             is_immediate: false,
             must_unwind: common::type_needs_unwind_cleanup(self.ccx, ty),
@@ -432,7 +433,7 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
                                val: ValueRef,
                                ty: Ty<'tcx>) {
 
-        if !common::type_needs_drop(self.ccx.tcx(), ty) { return; }
+        if !self.type_needs_drop(ty) { return; }
         let drop = box DropValue {
             is_immediate: true,
             must_unwind: common::type_needs_unwind_cleanup(self.ccx, ty),
diff --git a/src/librustc_trans/trans/common.rs b/src/librustc_trans/trans/common.rs
index d8fc6df2685..ec7ed2fe890 100644
--- a/src/librustc_trans/trans/common.rs
+++ b/src/librustc_trans/trans/common.rs
@@ -213,8 +213,43 @@ pub fn type_needs_unwind_cleanup<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty<
     }
 }
 
+/// If `type_needs_drop` returns true, then `ty` is definitely
+/// non-copy and *might* have a destructor attached; if it returns
+/// false, then `ty` definitely has no destructor (i.e. no drop glue).
+///
+/// (Note that this implies that if `ty` has a destructor attached,
+/// then `type_needs_drop` will definitely return `true` for `ty`.)
 pub fn type_needs_drop<'tcx>(cx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool {
-    ty::type_contents(cx, ty).needs_drop(cx)
+    type_needs_drop_given_env(cx, ty, &ty::empty_parameter_environment(cx))
+}
+
+/// Core implementation of type_needs_drop, potentially making use of
+/// and/or updating caches held in the `param_env`.
+fn type_needs_drop_given_env<'a,'tcx>(cx: &ty::ctxt<'tcx>,
+                                      ty: Ty<'tcx>,
+                                      param_env: &ty::ParameterEnvironment<'a,'tcx>) -> bool {
+    // Issue #22536: We first query type_moves_by_default.  It sees a
+    // normalized version of the type, and therefore will definitely
+    // know whether the type implements Copy (and thus needs no
+    // cleanup/drop/zeroing) ...
+    let implements_copy = !ty::type_moves_by_default(&param_env, DUMMY_SP, ty);
+
+    if implements_copy { return false; }
+
+    // ... (issue #22536 continued) but as an optimization, still use
+    // prior logic of asking if the `needs_drop` bit is set; we need
+    // not zero non-Copy types if they have no destructor.
+
+    // FIXME(#22815): Note that calling `ty::type_contents` is a
+    // conservative heuristic; it may report that `needs_drop` is set
+    // when actual type does not actually have a destructor associated
+    // with it. But since `ty` absolutely did not have the `Copy`
+    // bound attached (see above), it is sound to treat it as having a
+    // destructor (e.g. zero its memory on move).
+
+    let contents = ty::type_contents(cx, ty);
+    debug!("type_needs_drop ty={} contents={:?}", ty.repr(cx), contents);
+    contents.needs_drop(cx)
 }
 
 fn type_is_newtype_immediate<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool {
@@ -534,6 +569,12 @@ impl<'a, 'tcx> FunctionContext<'a, 'tcx> {
                                          self.param_substs,
                                          value)
     }
+
+    /// This is the same as `common::type_needs_drop`, except that it
+    /// may use or update caches within this `FunctionContext`.
+    pub fn type_needs_drop(&self, ty: Ty<'tcx>) -> bool {
+        type_needs_drop_given_env(self.ccx.tcx(), ty, &self.param_env)
+    }
 }
 
 // Basic block context.  We create a block context for each basic block
diff --git a/src/librustc_trans/trans/controlflow.rs b/src/librustc_trans/trans/controlflow.rs
index ad96c506c9d..85d0bc0319f 100644
--- a/src/librustc_trans/trans/controlflow.rs
+++ b/src/librustc_trans/trans/controlflow.rs
@@ -77,7 +77,7 @@ pub fn trans_stmt_semi<'blk, 'tcx>(cx: Block<'blk, 'tcx>, e: &ast::Expr)
                                    -> Block<'blk, 'tcx> {
     let _icx = push_ctxt("trans_stmt_semi");
     let ty = expr_ty(cx, e);
-    if type_needs_drop(cx.tcx(), ty) {
+    if cx.fcx.type_needs_drop(ty) {
         expr::trans_to_lvalue(cx, e, "stmt").bcx
     } else {
         expr::trans_into(cx, e, expr::Ignore)
diff --git a/src/librustc_trans/trans/datum.rs b/src/librustc_trans/trans/datum.rs
index 636c902d363..6ca71254868 100644
--- a/src/librustc_trans/trans/datum.rs
+++ b/src/librustc_trans/trans/datum.rs
@@ -312,7 +312,7 @@ impl KindOps for Lvalue {
                               ty: Ty<'tcx>)
                               -> Block<'blk, 'tcx> {
         let _icx = push_ctxt("<Lvalue as KindOps>::post_store");
-        if type_needs_drop(bcx.tcx(), ty) {
+        if bcx.fcx.type_needs_drop(ty) {
             // cancel cleanup of affine values by zeroing out
             let () = zero_mem(bcx, val, ty);
             bcx
@@ -657,7 +657,7 @@ impl<'tcx, K: KindOps + fmt::Debug> Datum<'tcx, K> {
     /// scalar-ish (like an int or a pointer) which (1) does not require drop glue and (2) is
     /// naturally passed around by value, and not by reference.
     pub fn to_llscalarish<'blk>(self, bcx: Block<'blk, 'tcx>) -> ValueRef {
-        assert!(!type_needs_drop(bcx.tcx(), self.ty));
+        assert!(!bcx.fcx.type_needs_drop(self.ty));
         assert!(self.appropriate_rvalue_mode(bcx.ccx()) == ByValue);
         if self.kind.is_by_ref() {
             load_ty(bcx, self.val, self.ty)
diff --git a/src/librustc_trans/trans/expr.rs b/src/librustc_trans/trans/expr.rs
index 5cc1baf66c6..27f9b9506a5 100644
--- a/src/librustc_trans/trans/expr.rs
+++ b/src/librustc_trans/trans/expr.rs
@@ -974,7 +974,7 @@ fn trans_rvalue_stmt_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
             let src_datum = unpack_datum!(bcx, trans(bcx, &**src));
             let dst_datum = unpack_datum!(bcx, trans_to_lvalue(bcx, &**dst, "assign"));
 
-            if type_needs_drop(bcx.tcx(), dst_datum.ty) {
+            if bcx.fcx.type_needs_drop(dst_datum.ty) {
                 // If there are destructors involved, make sure we
                 // are copying from an rvalue, since that cannot possible
                 // alias an lvalue. We are concerned about code like:
@@ -1498,7 +1498,7 @@ pub fn trans_adt<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
         assert_eq!(discr, 0);
 
         match ty::expr_kind(bcx.tcx(), &*base.expr) {
-            ty::RvalueDpsExpr | ty::RvalueDatumExpr if !type_needs_drop(bcx.tcx(), ty) => {
+            ty::RvalueDpsExpr | ty::RvalueDatumExpr if !bcx.fcx.type_needs_drop(ty) => {
                 bcx = trans_into(bcx, &*base.expr, SaveIn(addr));
             },
             ty::RvalueStmtExpr => bcx.tcx().sess.bug("unexpected expr kind for struct base expr"),
@@ -2116,7 +2116,7 @@ fn trans_assign_op<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
 
     // Evaluate LHS (destination), which should be an lvalue
     let dst_datum = unpack_datum!(bcx, trans_to_lvalue(bcx, dst, "assign_op"));
-    assert!(!type_needs_drop(bcx.tcx(), dst_datum.ty));
+    assert!(!bcx.fcx.type_needs_drop(dst_datum.ty));
     let dst_ty = dst_datum.ty;
     let dst = load_ty(bcx, dst_datum.val, dst_datum.ty);
 
diff --git a/src/librustc_trans/trans/glue.rs b/src/librustc_trans/trans/glue.rs
index c14683aeade..9491c8377a6 100644
--- a/src/librustc_trans/trans/glue.rs
+++ b/src/librustc_trans/trans/glue.rs
@@ -99,6 +99,16 @@ pub fn get_drop_glue_type<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
     if !type_is_sized(tcx, t) {
         return t
     }
+
+    // FIXME (#22815): note that type_needs_drop conservatively
+    // approximates in some cases and may say a type expression
+    // requires drop glue when it actually does not.
+    //
+    // (In this case it is not clear whether any harm is done, i.e.
+    // erroneously returning `t` in some cases where we could have
+    // returned `tcx.types.i8` does not appear unsound. The impact on
+    // code quality is unknown at this time.)
+
     if !type_needs_drop(tcx, t) {
         return tcx.types.i8;
     }
@@ -125,7 +135,7 @@ pub fn drop_ty<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
     // NB: v is an *alias* of type t here, not a direct value.
     debug!("drop_ty(t={})", t.repr(bcx.tcx()));
     let _icx = push_ctxt("drop_ty");
-    if type_needs_drop(bcx.tcx(), t) {
+    if bcx.fcx.type_needs_drop(t) {
         let ccx = bcx.ccx();
         let glue = get_drop_glue(ccx, t);
         let glue_type = get_drop_glue_type(ccx, t);
@@ -480,7 +490,7 @@ fn make_drop_glue<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, v0: ValueRef, t: Ty<'tcx>)
         },
         _ => {
             assert!(type_is_sized(bcx.tcx(), t));
-            if type_needs_drop(bcx.tcx(), t) && ty::type_is_structural(t) {
+            if bcx.fcx.type_needs_drop(t) && ty::type_is_structural(t) {
                 iter_structural_ty(bcx,
                                    v0,
                                    t,
diff --git a/src/librustc_trans/trans/intrinsic.rs b/src/librustc_trans/trans/intrinsic.rs
index 0706189a567..54644c92869 100644
--- a/src/librustc_trans/trans/intrinsic.rs
+++ b/src/librustc_trans/trans/intrinsic.rs
@@ -378,7 +378,8 @@ pub fn trans_intrinsic_call<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
         }
         (_, "needs_drop") => {
             let tp_ty = *substs.types.get(FnSpace, 0);
-            C_bool(ccx, type_needs_drop(ccx.tcx(), tp_ty))
+
+            C_bool(ccx, bcx.fcx.type_needs_drop(tp_ty))
         }
         (_, "owns_managed") => {
             let tp_ty = *substs.types.get(FnSpace, 0);
diff --git a/src/librustc_trans/trans/meth.rs b/src/librustc_trans/trans/meth.rs
index 3411f12886d..4423cd27744 100644
--- a/src/librustc_trans/trans/meth.rs
+++ b/src/librustc_trans/trans/meth.rs
@@ -454,7 +454,7 @@ fn trans_trait_callee<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
     let self_datum = unpack_datum!(
         bcx, expr::trans(bcx, self_expr));
 
-    let llval = if type_needs_drop(bcx.tcx(), self_datum.ty) {
+    let llval = if bcx.fcx.type_needs_drop(self_datum.ty) {
         let self_datum = unpack_datum!(
             bcx, self_datum.to_rvalue_datum(bcx, "trait_callee"));
 
diff --git a/src/librustc_trans/trans/tvec.rs b/src/librustc_trans/trans/tvec.rs
index d3acd23e641..a5c3923336a 100644
--- a/src/librustc_trans/trans/tvec.rs
+++ b/src/librustc_trans/trans/tvec.rs
@@ -53,11 +53,10 @@ pub fn make_drop_glue_unboxed<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
     let not_null = IsNotNull(bcx, vptr);
     with_cond(bcx, not_null, |bcx| {
         let ccx = bcx.ccx();
-        let tcx = bcx.tcx();
         let _icx = push_ctxt("tvec::make_drop_glue_unboxed");
 
         let dataptr = get_dataptr(bcx, vptr);
-        let bcx = if type_needs_drop(tcx, unit_ty) {
+        let bcx = if bcx.fcx.type_needs_drop(unit_ty) {
             let len = get_len(bcx, vptr);
             iter_vec_raw(bcx,
                          dataptr,