Address review comments

* Handle arrays with const-generic lengths
* Use closure for repeated code.

Author: matthewjasper

src/librustc/ty/util.rs

@@ -18,6 +18,7 @@ use rustc_hir::def::DefKind;
 use rustc_hir::def_id::DefId;
 use rustc_macros::HashStable;
 use rustc_span::Span;
+use rustc_target::abi::TargetDataLayout;
 use smallvec::SmallVec;
 use std::{cmp, fmt};
 use syntax::ast;
@@ -726,7 +727,7 @@ impl<'tcx> ty::TyS<'tcx> {
     #[inline]
     pub fn needs_drop(&'tcx self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool {
         // Avoid querying in simple cases.
-        match needs_drop_components(self) {
+        match needs_drop_components(self, &tcx.data_layout) {
             Err(AlwaysRequiresDrop) => true,
             Ok(components) => {
                 let query_ty = match *components {
@@ -736,7 +737,7 @@ impl<'tcx> ty::TyS<'tcx> {
                     [component_ty] => component_ty,
                     _ => self,
                 };
-                // This doesn't depend on regions, so try to minimize distinct.
+                // This doesn't depend on regions, so try to minimize distinct
                 // query keys used.
                 let erased = tcx.normalize_erasing_regions(param_env, query_ty);
                 tcx.needs_drop_raw(param_env.and(erased))
@@ -992,7 +993,10 @@ impl<'tcx> ExplicitSelf<'tcx> {
 /// Returns a list of types such that the given type needs drop if and only if
 /// *any* of the returned types need drop. Returns `Err(AlwaysRequiresDrop)` if
 /// this type always needs drop.
-pub fn needs_drop_components(ty: Ty<'tcx>) -> Result<SmallVec<[Ty<'tcx>; 4]>, AlwaysRequiresDrop> {
+pub fn needs_drop_components(
+    ty: Ty<'tcx>,
+    target_layout: &TargetDataLayout,
+) -> Result<SmallVec<[Ty<'tcx>; 2]>, AlwaysRequiresDrop> {
     match ty.kind {
         ty::Infer(ty::FreshIntTy(_))
         | ty::Infer(ty::FreshFloatTy(_))
@@ -1017,18 +1021,25 @@ pub fn needs_drop_components(ty: Ty<'tcx>) -> Result<SmallVec<[Ty<'tcx>; 4]>, Al
         // state transformation pass
         ty::Generator(..) | ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop),
 
-        ty::Slice(ty) => needs_drop_components(ty),
-        ty::Array(elem_ty, ..) => {
-            match needs_drop_components(elem_ty) {
+        ty::Slice(ty) => needs_drop_components(ty, target_layout),
+        ty::Array(elem_ty, size) => {
+            match needs_drop_components(elem_ty, target_layout) {
                 Ok(v) if v.is_empty() => Ok(v),
-                // Arrays of size zero don't need drop, even if their element
-                // type does.
-                _ => Ok(smallvec![ty]),
+                res => match size.val.try_to_bits(target_layout.pointer_size) {
+                    // Arrays of size zero don't need drop, even if their element
+                    // type does.
+                    Some(0) => Ok(SmallVec::new()),
+                    Some(_) => res,
+                    // We don't know which of the cases above we are in, so
+                    // return the whole type and let the caller decide what to
+                    // do.
+                    None => Ok(smallvec![ty]),
+                },
             }
         }
         // If any field needs drop, then the whole tuple does.
-        ty::Tuple(..) => ty.tuple_fields().try_fold(SmallVec::new(), |mut acc, elem| {
-            acc.extend(needs_drop_components(elem)?);
+        ty::Tuple(..) => ty.tuple_fields().try_fold(SmallVec::new(), move |mut acc, elem| {
+            acc.extend(needs_drop_components(elem, target_layout)?);
             Ok(acc)
         }),
 

src/librustc_ty/needs_drop.rs

@@ -76,30 +76,25 @@ where
                 return Some(Err(AlwaysRequiresDrop));
             }
 
-            let components = match needs_drop_components(ty) {
+            let components = match needs_drop_components(ty, &tcx.data_layout) {
                 Err(e) => return Some(Err(e)),
                 Ok(components) => components,
             };
             debug!("needs_drop_components({:?}) = {:?}", ty, components);
 
+            let queue_type = move |this: &mut Self, component: Ty<'tcx>| {
+                if this.seen_tys.insert(component) {
+                    this.unchecked_tys.push((component, level + 1));
+                }
+            };
+
             for component in components {
                 match component.kind {
                     _ if component.is_copy_modulo_regions(tcx, self.param_env, DUMMY_SP) => (),
 
-                    ty::Array(elem_ty, len) => {
-                        // Zero-length arrays never contain anything to drop.
-                        if len.try_eval_usize(tcx, self.param_env) != Some(0) {
-                            if self.seen_tys.insert(elem_ty) {
-                                self.unchecked_tys.push((elem_ty, level + 1));
-                            }
-                        }
-                    }
-
                     ty::Closure(def_id, substs) => {
                         for upvar_ty in substs.as_closure().upvar_tys(def_id, tcx) {
-                            if self.seen_tys.insert(upvar_ty) {
-                                self.unchecked_tys.push((upvar_ty, level + 1));
-                            }
+                            queue_type(self, upvar_ty);
                         }
                     }
 
@@ -116,21 +111,19 @@ where
                                 self.param_env,
                                 required_ty.subst(tcx, substs),
                             );
-                            if self.seen_tys.insert(subst_ty) {
-                                self.unchecked_tys.push((subst_ty, level + 1));
-                            }
+                            queue_type(self, subst_ty);
                         }
                     }
-                    ty::Opaque(..) | ty::Projection(..) | ty::Param(_) => {
+                    ty::Array(..) | ty::Opaque(..) | ty::Projection(..) | ty::Param(_) => {
                         if ty == component {
-                            // Return the type to the caller so they can decide
-                            // what to do with it.
+                            // Return the type to the caller: they may be able
+                            // to normalize further than we can.
                             return Some(Ok(component));
-                        } else if self.seen_tys.insert(component) {
+                        } else {
                             // Store the type for later. We can't return here
                             // because we would then lose any other components
                             // of the type.
-                            self.unchecked_tys.push((component, level + 1));
+                            queue_type(self, component);
                         }
                     }
                     _ => return Some(Err(AlwaysRequiresDrop)),