Fixes incorrect handling of ADT's drop requirements

compiler/rustc_ty_utils/src/needs_drop.rs

@@ -12,14 +12,12 @@ use rustc_span::{sym, DUMMY_SP};
 type NeedsDropResult<T> = Result<T, AlwaysRequiresDrop>;
 
 fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
-    let adt_components =
-        move |adt_def: &ty::AdtDef, _| tcx.adt_drop_tys(adt_def.did).map(|tys| tys.iter());
-
     // If we don't know a type doesn't need drop, for example if it's a type
     // parameter without a `Copy` bound, then we conservatively return that it
     // needs drop.
-    let res =
-        NeedsDropTypes::new(tcx, query.param_env, query.value, adt_components).next().is_some();
+    let adt_has_dtor =
+        |adt_def: &ty::AdtDef| adt_def.destructor(tcx).map(|_| DtorType::Significant);
+    let res = drop_tys_helper(tcx, query.value, query.param_env, adt_has_dtor).next().is_some();
 
     debug!("needs_drop_raw({:?}) = {:?}", query, res);
     res
@@ -29,12 +27,10 @@ fn has_significant_drop_raw<'tcx>(
     tcx: TyCtxt<'tcx>,
     query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
 ) -> bool {
-    let significant_drop_fields = move |adt_def: &ty::AdtDef, _| {
-        tcx.adt_significant_drop_tys(adt_def.did).map(|tys| tys.iter())
-    };
-    let res = NeedsDropTypes::new(tcx, query.param_env, query.value, significant_drop_fields)
-        .next()
-        .is_some();
+    let res =
+        drop_tys_helper(tcx, query.value, query.param_env, adt_consider_insignificant_dtor(tcx))
+            .next()
+            .is_some();
     debug!("has_significant_drop_raw({:?}) = {:?}", query, res);
     res
 }
@@ -140,15 +136,14 @@ where
                     // `ManuallyDrop`. If it's a struct or enum without a `Drop`
                     // impl then check whether the field types need `Drop`.
                     ty::Adt(adt_def, substs) => {
+                        debug!("Got value {:?} with substs {:?}", adt_def, substs);
                         let tys = match (self.adt_components)(adt_def, substs) {
                             Err(e) => return Some(Err(e)),
                             Ok(tys) => tys,
                         };
                         for required_ty in tys {
-                            let subst_ty = tcx.normalize_erasing_regions(
-                                self.param_env,
-                                required_ty.subst(tcx, substs),
-                            );
+                            let subst_ty =
+                                tcx.normalize_erasing_regions(self.param_env, required_ty);
                             queue_type(self, subst_ty);
                         }
                     }
@@ -187,11 +182,12 @@ enum DtorType {
 // Depending on the implentation of `adt_has_dtor`, it is used to check if the
 // ADT has a destructor or if the ADT only has a significant destructor. For
 // understanding significant destructor look at `adt_significant_drop_tys`.
-fn adt_drop_tys_helper<'tcx>(
+fn drop_tys_helper<'tcx>(
     tcx: TyCtxt<'tcx>,
-    def_id: DefId,
+    ty: Ty<'tcx>,
+    param_env: rustc_middle::ty::ParamEnv<'tcx>,
     adt_has_dtor: impl Fn(&ty::AdtDef) -> Option<DtorType>,
-) -> Result<&ty::List<Ty<'tcx>>, AlwaysRequiresDrop> {
+) -> impl Iterator<Item = NeedsDropResult<Ty<'tcx>>> {
     let adt_components = move |adt_def: &ty::AdtDef, substs: SubstsRef<'tcx>| {
         if adt_def.is_manually_drop() {
             debug!("adt_drop_tys: `{:?}` is manually drop", adt_def);
@@ -215,31 +211,25 @@ fn adt_drop_tys_helper<'tcx>(
             debug!("adt_drop_tys: `{:?}` is a union", adt_def);
             return Ok(Vec::new().into_iter());
         }
-        Ok(adt_def.all_fields().map(|field| tcx.type_of(field.did)).collect::<Vec<_>>().into_iter())
+        debug!("Path");
+        Ok(adt_def
+            .all_fields()
+            .map(|field| {
+                let r = tcx.type_of(field.did).subst(tcx, substs);
+                debug!("Subst into {:?} with {:?} gettng {:?}", field, substs, r);
+                r
+            })
+            .collect::<Vec<_>>()
+            .into_iter())
     };
 
-    let adt_ty = tcx.type_of(def_id);
-    let param_env = tcx.param_env(def_id);
-    let res: Result<Vec<_>, _> =
-        NeedsDropTypes::new(tcx, param_env, adt_ty, adt_components).collect();
-
-    debug!("adt_drop_tys(`{}`) = `{:?}`", tcx.def_path_str(def_id), res);
-    res.map(|components| tcx.intern_type_list(&components))
+    NeedsDropTypes::new(tcx, param_env, ty, adt_components)
 }
 
-fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
-    // This is for the "needs_drop" query, that considers all `Drop` impls, therefore all dtors are
-    // significant.
-    let adt_has_dtor =
-        |adt_def: &ty::AdtDef| adt_def.destructor(tcx).map(|_| DtorType::Significant);
-    adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
-}
-
-fn adt_significant_drop_tys(
-    tcx: TyCtxt<'_>,
-    def_id: DefId,
-) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
-    let adt_has_dtor = |adt_def: &ty::AdtDef| {
+fn adt_consider_insignificant_dtor<'tcx>(
+    tcx: TyCtxt<'tcx>,
+) -> impl Fn(&ty::AdtDef) -> Option<DtorType> + 'tcx {
+    move |adt_def: &ty::AdtDef| {
         let is_marked_insig = tcx.has_attr(adt_def.did, sym::rustc_insignificant_dtor);
         if is_marked_insig {
             // In some cases like `std::collections::HashMap` where the struct is a wrapper around
@@ -256,8 +246,31 @@ fn adt_significant_drop_tys(
             // treat this as the simple case of Drop impl for type.
             None
         }
-    };
-    adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
+    }
+}
+
+fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
+    // This is for the "adt_drop_tys" query, that considers all `Drop` impls, therefore all dtors are
+    // significant.
+    let adt_has_dtor =
+        |adt_def: &ty::AdtDef| adt_def.destructor(tcx).map(|_| DtorType::Significant);
+    drop_tys_helper(tcx, tcx.type_of(def_id), tcx.param_env(def_id), adt_has_dtor)
+        .collect::<Result<Vec<_>, _>>()
+        .map(|components| tcx.intern_type_list(&components))
+}
+
+fn adt_significant_drop_tys(
+    tcx: TyCtxt<'_>,
+    def_id: DefId,
+) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
+    drop_tys_helper(
+        tcx,
+        tcx.type_of(def_id),
+        tcx.param_env(def_id),
+        adt_consider_insignificant_dtor(tcx),
+    )
+    .collect::<Result<Vec<_>, _>>()
+    .map(|components| tcx.intern_type_list(&components))
 }
 
 pub(crate) fn provide(providers: &mut ty::query::Providers) {

src/test/ui/closures/2229_closure_analysis/migrations/issue-90024-adt-correct-subst.rs

@@ -0,0 +1,37 @@
+// Test that rustc doesn't ICE as in #90024.
+// check-pass
+// edition=2018
+
+#![warn(rust_2021_incompatible_closure_captures)]
+
+// Checks there's no double-subst into the generic args, otherwise we get OOB
+// MCVE by @lqd
+pub struct Graph<N, E, Ix> {
+    _edges: E,
+    _nodes: N,
+    _ix: Vec<Ix>,
+}
+fn graph<N, E>() -> Graph<N, E, i32> {
+    todo!()
+}
+fn first_ice() {
+    let g = graph::<i32, i32>();
+    let _ = || g;
+}
+
+// Checks that there is a subst into the fields, otherwise we get normalization error
+// MCVE by @cuviper
+use std::iter::Empty;
+struct Foo<I: Iterator> {
+    data: Vec<I::Item>,
+}
+pub fn second_ice() {
+    let v = Foo::<Empty<()>> { data: vec![] };
+
+    (|| v.data[0])();
+}
+
+pub fn main() {
+    first_ice();
+    second_ice();
+}