rustc: combine type_needs_drop_given_env and may_drop into needs_drop.

10 files changed, 68 insertions, 64 deletions

diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs
index b6e47568ff4..5c0889976c2 100644
--- a/src/librustc/ty/mod.rs
+++ b/src/librustc/ty/mod.rs
@@ -425,8 +425,8 @@ bitflags! {
         const MOVES_BY_DEFAULT  = 1 << 19,
         const FREEZENESS_CACHED = 1 << 20,
         const IS_FREEZE         = 1 << 21,
-        const MAY_DROP_CACHED   = 1 << 22,
-        const MAY_DROP          = 1 << 23,
+        const NEEDS_DROP_CACHED = 1 << 22,
+        const NEEDS_DROP        = 1 << 23,
     }
 }
 
@@ -2381,39 +2381,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         Some(self.item_mir(did))
     }
 
-    /// 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_given_env(self,
-                                     ty: Ty<'gcx>,
-                                     param_env: &ty::ParameterEnvironment<'gcx>) -> 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 tcx = self.global_tcx();
-        let implements_copy = !ty.moves_by_default(tcx, param_env, DUMMY_SP);
-
-        if implements_copy { return false; }
-
-        // ... (issue #22536 continued) but as an optimization, still use
-        // prior logic of asking for the structural `may_drop`.
-
-        // FIXME(#22815): Note that calling `ty::may_drop` is a
-        // conservative heuristic; it may report `true` ("may drop")
-        // 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.
-
-        let may_drop = ty.may_drop(tcx);
-        debug!("type_needs_drop ty={:?} may_drop={:?}", ty, may_drop);
-        may_drop
-    }
-
     /// Get the attributes of a definition.
     pub fn get_attrs(self, did: DefId) -> Cow<'gcx, [ast::Attribute]> {
         if let Some(id) = self.hir.as_local_node_id(did) {
diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs
index a3d2518909d..49d79f6545e 100644
--- a/src/librustc/ty/util.rs
+++ b/src/librustc/ty/util.rs
@@ -699,31 +699,52 @@ impl<'a, 'tcx> ty::TyS<'tcx> {
         result
     }
 
+    /// If `ty.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 `needs_drop` will definitely return `true` for `ty`.)
     #[inline]
-    pub fn may_drop(&'tcx self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool {
-        if self.flags.get().intersects(TypeFlags::MAY_DROP_CACHED) {
-            return self.flags.get().intersects(TypeFlags::MAY_DROP);
+    pub fn needs_drop(&'tcx self, tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                    param_env: &ty::ParameterEnvironment<'tcx>) -> bool {
+        if self.flags.get().intersects(TypeFlags::NEEDS_DROP_CACHED) {
+            return self.flags.get().intersects(TypeFlags::NEEDS_DROP);
         }
 
-        self.may_drop_inner(tcx, &mut FxHashSet())
+        self.needs_drop_uncached(tcx, param_env, &mut FxHashSet())
     }
 
-    fn may_drop_inner(&'tcx self,
-                      tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                      visited: &mut FxHashSet<Ty<'tcx>>)
-                      -> bool {
-        if self.flags.get().intersects(TypeFlags::MAY_DROP_CACHED) {
-            return self.flags.get().intersects(TypeFlags::MAY_DROP);
+    fn needs_drop_inner(&'tcx self,
+                        tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                        param_env: &ty::ParameterEnvironment<'tcx>,
+                        stack: &mut FxHashSet<Ty<'tcx>>)
+                        -> bool {
+        if self.flags.get().intersects(TypeFlags::NEEDS_DROP_CACHED) {
+            return self.flags.get().intersects(TypeFlags::NEEDS_DROP);
         }
 
         // This should be reported as an error by `check_representable`.
         //
         // Consider the type as not needing drop in the meanwhile to avoid
         // further errors.
-        if visited.replace(self).is_some() {
+        if let Some(_) = stack.replace(self) {
             return false;
         }
 
+        let needs_drop = self.needs_drop_uncached(tcx, param_env, stack);
+
+        // "Pop" the cycle detection "stack".
+        stack.remove(self);
+
+        needs_drop
+    }
+
+    fn needs_drop_uncached(&'tcx self,
+                           tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                           param_env: &ty::ParameterEnvironment<'tcx>,
+                           stack: &mut FxHashSet<Ty<'tcx>>)
+                           -> bool {
         assert!(!self.needs_infer());
 
         let result = match self.sty {
@@ -733,6 +754,21 @@ impl<'a, 'tcx> ty::TyS<'tcx> {
             ty::TyFnDef(..) | ty::TyFnPtr(_) | ty::TyChar |
             ty::TyRawPtr(_) | ty::TyRef(..) | ty::TyStr => false,
 
+            // 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) ...
+            _ if !self.moves_by_default(tcx, param_env, DUMMY_SP) => false,
+
+            // ... (issue #22536 continued) but as an optimization, still use
+            // prior logic of asking for the structural "may drop".
+
+            // FIXME(#22815): Note that this is a conservative heuristic;
+            // it may report that the type "may drop" when actual type does
+            // not actually have a destructor associated with it. But since
+            // the type absolutely did not have the `Copy` bound attached
+            // (see above), it is sound to treat it as having a destructor.
+
             // User destructors are the only way to have concrete drop types.
             ty::TyAdt(def, _) if def.has_dtor(tcx) => true,
 
@@ -743,16 +779,16 @@ impl<'a, 'tcx> ty::TyS<'tcx> {
 
             // Structural recursion.
             ty::TyArray(ty, _) | ty::TySlice(ty) => {
-                ty.may_drop_inner(tcx, visited)
+                ty.needs_drop_inner(tcx, param_env, stack)
             }
 
             ty::TyClosure(def_id, ref substs) => {
                 substs.upvar_tys(def_id, tcx)
-                    .any(|ty| ty.may_drop_inner(tcx, visited))
+                    .any(|ty| ty.needs_drop_inner(tcx, param_env, stack))
             }
 
             ty::TyTuple(ref tys, _) => {
-                tys.iter().any(|ty| ty.may_drop_inner(tcx, visited))
+                tys.iter().any(|ty| ty.needs_drop_inner(tcx, param_env, stack))
             }
 
             // unions don't have destructors regardless of the child types
@@ -761,17 +797,19 @@ impl<'a, 'tcx> ty::TyS<'tcx> {
             ty::TyAdt(def, substs) => {
                 def.variants.iter().any(|v| {
                     v.fields.iter().any(|f| {
-                        f.ty(tcx, substs).may_drop_inner(tcx, visited)
+                        f.ty(tcx, substs).needs_drop_inner(tcx, param_env, stack)
                     })
                 })
             }
         };
 
-        self.flags.set(self.flags.get() | if result {
-            TypeFlags::MAY_DROP_CACHED | TypeFlags::MAY_DROP
-        } else {
-            TypeFlags::MAY_DROP_CACHED
-        });
+        if !self.has_param_types() && !self.has_self_ty() {
+            self.flags.set(self.flags.get() | if result {
+                TypeFlags::NEEDS_DROP_CACHED | TypeFlags::NEEDS_DROP
+            } else {
+                TypeFlags::NEEDS_DROP_CACHED
+            });
+        }
 
         result
     }
diff --git a/src/librustc_borrowck/borrowck/mir/mod.rs b/src/librustc_borrowck/borrowck/mir/mod.rs
index dc01cbe5e76..de5613dbfaa 100644
--- a/src/librustc_borrowck/borrowck/mir/mod.rs
+++ b/src/librustc_borrowck/borrowck/mir/mod.rs
@@ -322,7 +322,7 @@ fn on_all_drop_children_bits<'a, 'tcx, F>(
         let ty = lvalue.ty(mir, tcx).to_ty(tcx);
         debug!("on_all_drop_children_bits({:?}, {:?} : {:?})", path, lvalue, ty);
 
-        if tcx.type_needs_drop_given_env(ty, &ctxt.param_env) {
+        if ty.needs_drop(tcx, &ctxt.param_env) {
             each_child(child);
         } else {
             debug!("on_all_drop_children_bits - skipping")
diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
index 0ee9d4a42c7..1c69f3cff17 100644
--- a/src/librustc_lint/builtin.rs
+++ b/src/librustc_lint/builtin.rs
@@ -1152,7 +1152,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnionsWithDropFields {
             let param_env = &ty::ParameterEnvironment::for_item(ctx.tcx, item.id);
             for field in vdata.fields() {
                 let field_ty = ctx.tcx.item_type(ctx.tcx.hir.local_def_id(field.id));
-                if ctx.tcx.type_needs_drop_given_env(field_ty, param_env) {
+                if field_ty.needs_drop(ctx.tcx, param_env) {
                     ctx.span_lint(UNIONS_WITH_DROP_FIELDS,
                                   field.span,
                                   "union contains a field with possibly non-trivial drop code, \
diff --git a/src/librustc_mir/hair/cx/mod.rs b/src/librustc_mir/hair/cx/mod.rs
index 5f9fb8e1b12..db9da2a280b 100644
--- a/src/librustc_mir/hair/cx/mod.rs
+++ b/src/librustc_mir/hair/cx/mod.rs
@@ -168,7 +168,7 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> {
                   type with inference types/regions",
                  ty);
         });
-        self.tcx.type_needs_drop_given_env(ty, &self.infcx.parameter_environment)
+        ty.needs_drop(self.tcx.global_tcx(), &self.infcx.parameter_environment)
     }
 
     pub fn tcx(&self) -> TyCtxt<'a, 'gcx, 'tcx> {
diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs
index ac2bdaad24f..892d67ac237 100644
--- a/src/librustc_mir/transform/inline.rs
+++ b/src/librustc_mir/transform/inline.rs
@@ -357,7 +357,7 @@ impl<'a, 'tcx> Inliner<'a, 'tcx> {
                     // a regular goto.
                     let ty = location.ty(&callee_mir, tcx).subst(tcx, callsite.substs);
                     let ty = ty.to_ty(tcx);
-                    if tcx.type_needs_drop_given_env(ty, &param_env) {
+                    if ty.needs_drop(tcx, &param_env) {
                         cost += CALL_PENALTY;
                         if let Some(unwind) = unwind {
                             work_list.push(unwind);
diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs
index cc7d2562843..526c1488ab4 100644
--- a/src/librustc_mir/transform/qualify_consts.rs
+++ b/src/librustc_mir/transform/qualify_consts.rs
@@ -83,7 +83,7 @@ impl<'a, 'tcx> Qualif {
         if ty.is_freeze(tcx, param_env, DUMMY_SP) {
             *self = *self - Qualif::MUTABLE_INTERIOR;
         }
-        if !tcx.type_needs_drop_given_env(ty, param_env) {
+        if !ty.needs_drop(tcx, param_env) {
             *self = *self - Qualif::NEEDS_DROP;
         }
     }
diff --git a/src/librustc_mir/util/elaborate_drops.rs b/src/librustc_mir/util/elaborate_drops.rs
index 04a1fc891cf..07025fcfdb9 100644
--- a/src/librustc_mir/util/elaborate_drops.rs
+++ b/src/librustc_mir/util/elaborate_drops.rs
@@ -277,8 +277,7 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
 
         let mut fields = fields;
         fields.retain(|&(ref lvalue, _)| {
-            self.tcx().type_needs_drop_given_env(
-                self.lvalue_ty(lvalue), self.elaborator.param_env())
+            self.lvalue_ty(lvalue).needs_drop(self.tcx(), self.elaborator.param_env())
         });
 
         debug!("drop_ladder - fields needing drop: {:?}", fields);
diff --git a/src/librustc_passes/consts.rs b/src/librustc_passes/consts.rs
index 535c6a7ab9e..fdb67522133 100644
--- a/src/librustc_passes/consts.rs
+++ b/src/librustc_passes/consts.rs
@@ -89,7 +89,7 @@ impl<'a, 'gcx> CheckCrateVisitor<'a, 'gcx> {
             self.promotable = false;
         }
 
-        if self.tcx.type_needs_drop_given_env(ty, &self.param_env) {
+        if ty.needs_drop(self.tcx, &self.param_env) {
             self.promotable = false;
         }
     }
diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs
index fd9ff17cc64..1d1921bf7b9 100644
--- a/src/librustc_trans/context.rs
+++ b/src/librustc_trans/context.rs
@@ -392,7 +392,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> {
     }
 
     pub fn type_needs_drop(&self, ty: Ty<'tcx>) -> bool {
-        self.tcx.type_needs_drop_given_env(ty, &self.empty_param_env)
+        ty.needs_drop(self.tcx, &self.empty_param_env)
     }
 
     pub fn type_is_sized(&self, ty: Ty<'tcx>) -> bool {