Check enum and struct representability properly (issues rust-lang#3008 and rust-lang#3779)

Author: isabelmu

src/librustc/middle/ty.rs

@@ -2348,53 +2348,95 @@ pub fn is_instantiable(cx: ctxt, r_ty: t) -> bool {
     !subtypes_require(cx, &mut seen, r_ty, r_ty)
 }
 
-pub fn type_structurally_contains(cx: ctxt, ty: t, test: |x: &sty| -> bool)
-                                  -> bool {
-    let sty = &get(ty).sty;
-    debug!("type_structurally_contains: {}",
-           ::util::ppaux::ty_to_str(cx, ty));
-    if test(sty) { return true; }
-    match *sty {
-      ty_enum(did, ref substs) => {
-        for variant in (*enum_variants(cx, did)).iter() {
-            for aty in variant.args.iter() {
-                let sty = subst(cx, substs, *aty);
-                if type_structurally_contains(cx, sty, |x| test(x)) { return true; }
+// Needed to distinguish between types that are recursive with themselves,
+// and types that contain a different recursive type.
+#[deriving(Eq)]
+pub enum Repr {
+    Representable,
+    SelfRecursive,
+    ContainsRecursive,
+}
+
+// True if `ty` contains no structural recursion. This is necessary
+// for structs and enums to have finite size.
+pub fn is_type_representable(cx: ctxt, ty: t) -> Repr {
+
+    // Does the type `ty` directly (without indirection through a pointer)
+    // contain any types on stack `seen`?
+    fn type_structurally_recursive(cx: ctxt, ty: t,
+                                   seen: &mut ~[DefId]) -> Repr {
+        debug!("type_structurally_recursive: {}",
+               ::util::ppaux::ty_to_str(cx, ty));
+
+        // Compare current type to previously seen types (stack 'seen')
+        match get(ty).sty {
+            ty_struct(did, _) |
+            ty_enum(did, _) => {
+                for (i, &seen_did) in seen.iter().enumerate() {
+                    if did == seen_did {
+                        return if i == 0 { SelfRecursive }
+                               else { ContainsRecursive }
+                    }
+                }
             }
+            _ => (),
         }
-        return false;
-      }
-      ty_struct(did, ref substs) => {
-        let r = lookup_struct_fields(cx, did);
-        for field in r.iter() {
-            let ft = lookup_field_type(cx, did, field.id, substs);
-            if type_structurally_contains(cx, ft, |x| test(x)) { return true; }
-        }
-        return false;
-      }
 
-      ty_tup(ref ts) => {
-        for tt in ts.iter() {
-            if type_structurally_contains(cx, *tt, |x| test(x)) { return true; }
+        // Check inner types
+        match get(ty).sty {
+            // Tuples
+            ty_tup(ref ts) => {
+                let mut r = Representable;
+                for t in ts.iter() {
+                    r = type_structurally_recursive(cx, *t, seen);
+                    if r != Representable { break; }
+                }
+                r
+            }
+            // Non-zero fixed-length vectors.
+            ty_vec(mt, vstore_fixed(len)) if len != 0 => {
+                type_structurally_recursive(cx, mt.ty, seen)
+            }
+
+            // Push struct and enum def-ids/substs onto `seen` before recursing.
+            ty_struct(did, ref substs) => {
+                seen.push(did);
+                let fields = struct_fields(cx, did, substs);
+                let mut r = Representable;
+                for f in fields.iter() {
+                    r = type_structurally_recursive(cx, f.mt.ty, seen);
+                    if r != Representable { break; }
+                }
+                seen.pop();
+                r
+            }
+            ty_enum(did, ref substs) => {
+                seen.push(did);
+                let vs = enum_variants(cx, did);
+                let mut r = Representable;
+                'outer: for variant in vs.iter() {
+                    for aty in variant.args.iter() {
+                        let sty = subst(cx, substs, *aty);
+                        r = type_structurally_recursive(cx, sty, seen);
+                        if r != Representable { break 'outer }
+                    }
+                };
+                seen.pop();
+                r
+            }
+
+            _ => Representable,
         }
-        return false;
-      }
-      ty_vec(ref mt, vstore_fixed(_)) => {
-        return type_structurally_contains(cx, mt.ty, test);
-      }
-      _ => return false
     }
-}
 
-pub fn type_structurally_contains_uniques(cx: ctxt, ty: t) -> bool {
-    return type_structurally_contains(cx, ty, |sty| {
-        match *sty {
-          ty_uniq(_) |
-          ty_vec(_, vstore_uniq) |
-          ty_str(vstore_uniq) => true,
-          _ => false,
-        }
-    });
+    debug!("is_type_representable: {}",
+           ::util::ppaux::ty_to_str(cx, ty));
+
+    // To avoid a stack overflow when checking an enum with a variant that
+    // that contains a different, structurally recursive type, maintain a
+    // stack of seen types and check recursion for each of them (issue #3008).
+    let mut seen: ~[DefId] = ~[];
+    type_structurally_recursive(cx, ty, &mut seen)
 }
 
 pub fn type_is_trait(ty: t) -> bool {

src/librustc/middle/typeck/check/mod.rs

@@ -531,7 +531,10 @@ pub fn check_no_duplicate_fields(tcx: ty::ctxt,
 pub fn check_struct(ccx: @CrateCtxt, id: ast::NodeId, span: Span) {
     let tcx = ccx.tcx;
 
-    // Check that the class is instantiable
+    // Check that the struct is representable
+    check_representable(tcx, span, id, "struct");
+
+    // Check that the struct is instantiable
     check_instantiable(tcx, span, id);
 
     if ty::lookup_simd(tcx, local_def(id)) {
@@ -3451,6 +3454,28 @@ pub fn check_const_with_ty(fcx: @FnCtxt,
     writeback::resolve_type_vars_in_expr(fcx, e);
 }
 
+pub fn check_representable(tcx: ty::ctxt,
+                           sp: Span,
+                           item_id: ast::NodeId,
+                           designation: &str) {
+    let rty = ty::node_id_to_type(tcx, item_id);
+
+    // Check that it is possible to represent this type. This call identifies
+    // (1) types that contain themselves and (2) types that contain a different
+    // recursive type. It is only necessary to throw an error on those that
+    // contain themselves. For case 2, there must be an inner type that will be
+    // caught by case 1.
+    match ty::is_type_representable(tcx, rty) {
+      ty::SelfRecursive => {
+        tcx.sess.span_err(
+          sp, format!("illegal recursive {} type; \
+                       wrap the inner value in a box to make it representable",
+                      designation));
+      }
+      ty::Representable | ty::ContainsRecursive => (),
+    }
+}
+
 /// Checks whether a type can be created without an instance of itself.
 /// This is similar but different from the question of whether a type
 /// can be represented.  For example, the following type:
@@ -3606,7 +3631,6 @@ pub fn check_enum_variants(ccx: @CrateCtxt,
         return variants;
     }
 
-    let rty = ty::node_id_to_type(ccx.tcx, id);
     let hint = ty::lookup_repr_hint(ccx.tcx, ast::DefId { crate: ast::LOCAL_CRATE, node: id });
     if hint != attr::ReprAny && vs.len() <= 1 {
         ccx.tcx.sess.span_err(sp, format!("unsupported representation for {}variant enum",
@@ -3621,22 +3645,8 @@ pub fn check_enum_variants(ccx: @CrateCtxt,
         enum_var_cache.get().insert(local_def(id), @variants);
     }
 
-    // Check that it is possible to represent this enum:
-    let mut outer = true;
-    let did = local_def(id);
-    if ty::type_structurally_contains(ccx.tcx, rty, |sty| {
-        match *sty {
-          ty::ty_enum(id, _) if id == did => {
-            if outer { outer = false; false }
-            else { true }
-          }
-          _ => false
-        }
-    }) {
-        ccx.tcx.sess.span_err(sp,
-                              "illegal recursive enum type; \
-                               wrap the inner value in a box to make it representable");
-    }
+    // Check that it is possible to represent this enum.
+    check_representable(ccx.tcx, sp, id, "enum");
 
     // Check that it is possible to instantiate this enum:
     //