]> git.lizzy.rs Git - rust.git/commitdiff
Fix stack overflow for recursive types.
authorjumbatm <30644300+jumbatm@users.noreply.github.com>
Sat, 15 Aug 2020 05:05:18 +0000 (15:05 +1000)
committerjumbatm <30644300+jumbatm@users.noreply.github.com>
Mon, 17 Aug 2020 16:01:04 +0000 (02:01 +1000)
Adds a seen set to structurally_same_type to avoid recursing
indefinitely when a reference or pointer member introduces a cycle in
the visited types.

src/librustc_lint/builtin.rs

index 3859d0f163ad5c52798eb35c2604ec8392289f96..0e578ac5034db2e577cceea74722313344273304 100644 (file)
@@ -2153,44 +2153,99 @@ fn structurally_same_type<'tcx>(
         b: Ty<'tcx>,
         ckind: CItemKind,
     ) -> bool {
-        debug!("structurally_same_type(cx, a = {:?}, b = {:?})", a, b);
-        let tcx = cx.tcx;
-        if a == b || rustc_middle::ty::TyS::same_type(a, b) {
-            // All nominally-same types are structurally same, too.
-            true
-        } else {
-            // Do a full, depth-first comparison between the two.
-            use rustc_middle::ty::TyKind::*;
-            let a_kind = &a.kind;
-            let b_kind = &b.kind;
-
-            let compare_layouts = |a, b| -> bool {
-                let a_layout = &cx.layout_of(a).unwrap().layout.abi;
-                let b_layout = &cx.layout_of(b).unwrap().layout.abi;
-                debug!("{:?} == {:?} = {}", a_layout, b_layout, a_layout == b_layout);
-                a_layout == b_layout
-            };
+        // In order to avoid endlessly recursing on recursive types, we maintain a "seen" set.
+        // We'll need to store every combination of types we encounter anyway, so we also memoize
+        // the result.
+        struct SeenSet<'tcx>(FxHashMap<(Ty<'tcx>, Ty<'tcx>), Option<bool>>);
+
+        enum SeenSetResult {
+            /// We've never seen this combination of types.
+            Unseen,
+            /// We've seen this combination of types, but are still computing the result.
+            Computing,
+            /// We've seen this combination of types, and have already computed the result.
+            Computed(bool),
+        }
+
+        impl<'tcx> SeenSet<'tcx> {
+            fn new() -> Self {
+                SeenSet(FxHashMap::default())
+            }
+            /// Mark (a, b) as `Computing`.
+            fn mark_computing(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) {
+                self.0.insert((a, b), None);
+            }
+            /// Mark (a, b) as `Computed(result)`.
+            fn mark_computed(&mut self, a: Ty<'tcx>, b: Ty<'tcx>, result: bool) {
+                *self.0.get_mut(&(a, b)).expect("Missing prior call to mark_computing") =
+                    Some(result);
+            }
+            fn get(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> SeenSetResult {
+                match self.0.get(&(a, b)) {
+                    None => SeenSetResult::Unseen,
+                    Some(None) => SeenSetResult::Computing,
+                    Some(Some(b)) => SeenSetResult::Computed(*b),
+                }
+            }
+        }
+        fn structurally_same_type_impl<'tcx>(
+            seen_types: &mut SeenSet<'tcx>,
+            cx: &LateContext<'tcx>,
+            a: Ty<'tcx>,
+            b: Ty<'tcx>,
+            ckind: CItemKind,
+        ) -> bool {
+            debug!("structurally_same_type_impl(cx, a = {:?}, b = {:?})", a, b);
+            match seen_types.get(a, b) {
+                // If we've already computed the result, just return the memoized result.
+                SeenSetResult::Computed(result) => result,
+                // We are already in the process of computing structural sameness for this type,
+                // meaning we've found a cycle. The types are structurally same, then.
+                SeenSetResult::Computing => true,
+                // We haven't seen this combination of types at all -- compute their sameness.
+                SeenSetResult::Unseen => {
+                    seen_types.mark_computing(a, b);
+                    let tcx = cx.tcx;
+                    let result = if a == b || rustc_middle::ty::TyS::same_type(a, b) {
+                        // All nominally-same types are structurally same, too.
+                        true
+                    } else {
+                        // Do a full, depth-first comparison between the two.
+                        use rustc_middle::ty::TyKind::*;
+                        let a_kind = &a.kind;
+                        let b_kind = &b.kind;
+
+                        let compare_layouts = |a, b| -> bool {
+                            let a_layout = &cx.layout_of(a).unwrap().layout.abi;
+                            let b_layout = &cx.layout_of(b).unwrap().layout.abi;
+                            debug!("{:?} == {:?} = {}", a_layout, b_layout, a_layout == b_layout);
+                            a_layout == b_layout
+                        };
+
+                        #[allow(rustc::usage_of_ty_tykind)]
+                        let is_primitive_or_pointer = |kind: &ty::TyKind<'_>| {
+                            kind.is_primitive() || matches!(kind, RawPtr(..))
+                        };
 
-            #[allow(rustc::usage_of_ty_tykind)]
-            let is_primitive_or_pointer =
-                |kind: &ty::TyKind<'_>| kind.is_primitive() || matches!(kind, RawPtr(..));
-
-            match (a_kind, b_kind) {
-                (Adt(_, a_substs), Adt(_, b_substs)) => {
-                    let a = a.subst(cx.tcx, a_substs);
-                    let b = b.subst(cx.tcx, b_substs);
-                    debug!("Comparing {:?} and {:?}", a, b);
-
-                    if let (Adt(a_def, ..), Adt(b_def, ..)) = (&a.kind, &b.kind) {
-                        // Grab a flattened representation of all fields.
-                        let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter());
-                        let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter());
-                        compare_layouts(a, b)
+                        match (a_kind, b_kind) {
+                            (Adt(_, a_substs), Adt(_, b_substs)) => {
+                                let a = a.subst(cx.tcx, a_substs);
+                                let b = b.subst(cx.tcx, b_substs);
+                                debug!("Comparing {:?} and {:?}", a, b);
+
+                                if let (Adt(a_def, ..), Adt(b_def, ..)) = (&a.kind, &b.kind) {
+                                    // Grab a flattened representation of all fields.
+                                    let a_fields =
+                                        a_def.variants.iter().flat_map(|v| v.fields.iter());
+                                    let b_fields =
+                                        b_def.variants.iter().flat_map(|v| v.fields.iter());
+                                    compare_layouts(a, b)
                             && a_fields.eq_by(
                                 b_fields,
                                 |&ty::FieldDef { did: a_did, .. },
                                  &ty::FieldDef { did: b_did, .. }| {
-                                    Self::structurally_same_type(
+                                    structurally_same_type_impl(
+                                        seen_types,
                                         cx,
                                         tcx.type_of(a_did),
                                         tcx.type_of(b_did),
@@ -2198,78 +2253,108 @@ fn structurally_same_type<'tcx>(
                                     )
                                 },
                             )
-                    } else {
-                        unreachable!()
-                    }
-                }
-                (Array(a_ty, a_const), Array(b_ty, b_const)) => {
-                    // For arrays, we also check the constness of the type.
-                    a_const.val == b_const.val
-                        && Self::structurally_same_type(cx, a_const.ty, b_const.ty, ckind)
-                        && Self::structurally_same_type(cx, a_ty, b_ty, ckind)
-                }
-                (Slice(a_ty), Slice(b_ty)) => Self::structurally_same_type(cx, a_ty, b_ty, ckind),
-                (RawPtr(a_tymut), RawPtr(b_tymut)) => {
-                    a_tymut.mutbl == b_tymut.mutbl
-                        && Self::structurally_same_type(cx, &a_tymut.ty, &b_tymut.ty, ckind)
-                }
-                (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => {
-                    // For structural sameness, we don't need the region to be same.
-                    a_mut == b_mut && Self::structurally_same_type(cx, a_ty, b_ty, ckind)
-                }
-                (FnDef(..), FnDef(..)) => {
-                    let a_poly_sig = a.fn_sig(tcx);
-                    let b_poly_sig = b.fn_sig(tcx);
-
-                    // As we don't compare regions, skip_binder is fine.
-                    let a_sig = a_poly_sig.skip_binder();
-                    let b_sig = b_poly_sig.skip_binder();
-
-                    (a_sig.abi, a_sig.unsafety, a_sig.c_variadic)
-                        == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic)
-                        && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| {
-                            Self::structurally_same_type(cx, a, b, ckind)
-                        })
-                        && Self::structurally_same_type(cx, a_sig.output(), b_sig.output(), ckind)
-                }
-                (Tuple(a_substs), Tuple(b_substs)) => {
-                    a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| {
-                        Self::structurally_same_type(cx, a_ty, b_ty, ckind)
-                    })
-                }
-                // For these, it's not quite as easy to define structural-sameness quite so easily.
-                // For the purposes of this lint, take the conservative approach and mark them as
-                // not structurally same.
-                (Dynamic(..), Dynamic(..))
-                | (Error(..), Error(..))
-                | (Closure(..), Closure(..))
-                | (Generator(..), Generator(..))
-                | (GeneratorWitness(..), GeneratorWitness(..))
-                | (Projection(..), Projection(..))
-                | (Opaque(..), Opaque(..)) => false,
-
-                // These definitely should have been caught above.
-                (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(),
-
-                // An Adt and a primitive type. This can be FFI-safe is the ADT is an enum with a
-                // non-null field.
-                (Adt(..), other_kind) | (other_kind, Adt(..))
-                    if is_primitive_or_pointer(other_kind) =>
-                {
-                    let (primitive, adt) =
-                        if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) };
-                    if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) {
-                        ty == primitive
-                    } else {
-                        compare_layouts(a, b)
-                    }
+                                } else {
+                                    unreachable!()
+                                }
+                            }
+                            (Array(a_ty, a_const), Array(b_ty, b_const)) => {
+                                // For arrays, we also check the constness of the type.
+                                a_const.val == b_const.val
+                                    && structurally_same_type_impl(
+                                        seen_types, cx, a_const.ty, b_const.ty, ckind,
+                                    )
+                                    && structurally_same_type_impl(
+                                        seen_types, cx, a_ty, b_ty, ckind,
+                                    )
+                            }
+                            (Slice(a_ty), Slice(b_ty)) => {
+                                structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind)
+                            }
+                            (RawPtr(a_tymut), RawPtr(b_tymut)) => {
+                                a_tymut.mutbl == b_tymut.mutbl
+                                    && structurally_same_type_impl(
+                                        seen_types,
+                                        cx,
+                                        &a_tymut.ty,
+                                        &b_tymut.ty,
+                                        ckind,
+                                    )
+                            }
+                            (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => {
+                                // For structural sameness, we don't need the region to be same.
+                                a_mut == b_mut
+                                    && structurally_same_type_impl(
+                                        seen_types, cx, a_ty, b_ty, ckind,
+                                    )
+                            }
+                            (FnDef(..), FnDef(..)) => {
+                                let a_poly_sig = a.fn_sig(tcx);
+                                let b_poly_sig = b.fn_sig(tcx);
+
+                                // As we don't compare regions, skip_binder is fine.
+                                let a_sig = a_poly_sig.skip_binder();
+                                let b_sig = b_poly_sig.skip_binder();
+
+                                (a_sig.abi, a_sig.unsafety, a_sig.c_variadic)
+                                    == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic)
+                                    && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| {
+                                        structurally_same_type_impl(seen_types, cx, a, b, ckind)
+                                    })
+                                    && structurally_same_type_impl(
+                                        seen_types,
+                                        cx,
+                                        a_sig.output(),
+                                        b_sig.output(),
+                                        ckind,
+                                    )
+                            }
+                            (Tuple(a_substs), Tuple(b_substs)) => {
+                                a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| {
+                                    structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind)
+                                })
+                            }
+                            // For these, it's not quite as easy to define structural-sameness quite so easily.
+                            // For the purposes of this lint, take the conservative approach and mark them as
+                            // not structurally same.
+                            (Dynamic(..), Dynamic(..))
+                            | (Error(..), Error(..))
+                            | (Closure(..), Closure(..))
+                            | (Generator(..), Generator(..))
+                            | (GeneratorWitness(..), GeneratorWitness(..))
+                            | (Projection(..), Projection(..))
+                            | (Opaque(..), Opaque(..)) => false,
+
+                            // These definitely should have been caught above.
+                            (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => {
+                                unreachable!()
+                            }
+
+                            // An Adt and a primitive type. This can be FFI-safe is the ADT is an enum with a
+                            // non-null field.
+                            (Adt(..), other_kind) | (other_kind, Adt(..))
+                                if is_primitive_or_pointer(other_kind) =>
+                            {
+                                let (primitive, adt) =
+                                    if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) };
+                                if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) {
+                                    ty == primitive
+                                } else {
+                                    compare_layouts(a, b)
+                                }
+                            }
+                            // Otherwise, just compare the layouts. This may fail to lint for some
+                            // incompatible types, but at the very least, will stop reads into
+                            // uninitialised memory.
+                            _ => compare_layouts(a, b),
+                        }
+                    };
+                    seen_types.mark_computed(a, b, result);
+                    result
                 }
-                // Otherwise, just compare the layouts. This may fail to lint for some
-                // incompatible types, but at the very least, will stop reads into
-                // uninitialised memory.
-                _ => compare_layouts(a, b),
             }
         }
+        let mut seen_types = SeenSet::new();
+        structurally_same_type_impl(&mut seen_types, cx, a, b, ckind)
     }
 }