From d20646b2d8033f31423b5bda3e56776df115e144 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 31 Jan 2020 22:22:30 +0000 Subject: [PATCH] Address review comments * Handle arrays with const-generic lengths * Use closure for repeated code. --- src/librustc/ty/util.rs | 33 ++++++++++++++++++++++----------- src/librustc_ty/needs_drop.rs | 35 ++++++++++++++--------------------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index eaa7a43b091..6191d304719 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -18,6 +18,7 @@ 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 @@ pub fn is_freeze( #[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 @@ pub fn needs_drop(&'tcx self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'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 @@ pub fn determine

(self_arg_ty: Ty<'tcx>, is_self_ty: P) -> 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; 4]>, AlwaysRequiresDrop> { +pub fn needs_drop_components( + ty: Ty<'tcx>, + target_layout: &TargetDataLayout, +) -> Result; 2]>, AlwaysRequiresDrop> { match ty.kind { ty::Infer(ty::FreshIntTy(_)) | ty::Infer(ty::FreshFloatTy(_)) @@ -1017,18 +1021,25 @@ pub fn determine

(self_arg_ty: Ty<'tcx>, is_self_ty: P) -> ExplicitSelf<'tcx> // 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) }), diff --git a/src/librustc_ty/needs_drop.rs b/src/librustc_ty/needs_drop.rs index 1a65acb1f98..c01b3e384ae 100644 --- a/src/librustc_ty/needs_drop.rs +++ b/src/librustc_ty/needs_drop.rs @@ -76,30 +76,25 @@ fn next(&mut self) -> Option>> { 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 @@ fn next(&mut self) -> Option>> { 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)),