From 7a931574e32ad87db295a97293bae0755694d2ba Mon Sep 17 00:00:00 2001 From: Johannes Muenzel Date: Tue, 28 Jan 2014 01:28:50 -0500 Subject: [PATCH] Check enum and struct representability properly (issues #3008 and #3779) --- src/librustc/middle/ty.rs | 138 ++++++++++++++++-------- src/librustc/middle/typeck/check/mod.rs | 51 +++++---- 2 files changed, 127 insertions(+), 62 deletions(-) diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 469df730a129..4a166be9aae4 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -2348,53 +2348,103 @@ 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; } - } - } - 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; } - } - return false; - } - ty_vec(ref mt, vstore_fixed(_)) => { - return type_structurally_contains(cx, mt.ty, test); - } - _ => return false - } +/// Describes whether a type is representable. For types that are not +/// representable, 'SelfRecursive' and 'ContainsRecursive' are used to +/// distinguish between types that are recursive with themselves and types that +/// contain a different recursive type. These cases can therefore be treated +/// differently when reporting errors. +#[deriving(Eq)] +pub enum Representability { + Representable, + SelfRecursive, + ContainsRecursive, } -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, +/// Check whether a type is representable. This means it cannot contain unboxed +/// structural recursion. This check is needed for structs and enums. +pub fn is_type_representable(cx: ctxt, ty: t) -> Representability { + + // Iterate until something non-representable is found + fn find_nonrepresentable>(cx: ctxt, seen: &mut ~[DefId], + mut iter: It) -> Representability { + for ty in iter { + let r = type_structurally_recursive(cx, seen, ty); + if r != Representable { + return r + } } - }); + Representable + } + + // Does the type `ty` directly (without indirection through a pointer) + // contain any types on stack `seen`? + fn type_structurally_recursive(cx: ctxt, seen: &mut ~[DefId], + ty: t) -> Representability { + debug!("type_structurally_recursive: {}", + ::util::ppaux::ty_to_str(cx, ty)); + + // Compare current type to previously seen types + 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 } + } + } + } + _ => (), + } + + // Check inner types + match get(ty).sty { + // Tuples + ty_tup(ref ts) => { + find_nonrepresentable(cx, seen, ts.iter().map(|t| *t)) + } + // Non-zero fixed-length vectors. + ty_vec(mt, vstore_fixed(len)) if len != 0 => { + type_structurally_recursive(cx, seen, mt.ty) + } + + // Push struct and enum def-ids onto `seen` before recursing. + ty_struct(did, ref substs) => { + seen.push(did); + let fields = struct_fields(cx, did, substs); + let r = find_nonrepresentable(cx, seen, + fields.iter().map(|f| f.mt.ty)); + seen.pop(); + r + } + ty_enum(did, ref substs) => { + seen.push(did); + let vs = enum_variants(cx, did); + + let mut r = Representable; + for variant in vs.iter() { + let iter = variant.args.iter().map(|aty| subst(cx, substs, *aty)); + r = find_nonrepresentable(cx, seen, iter); + + if r != Representable { break } + } + + seen.pop(); + r + } + + _ => Representable, + } + } + + debug!("is_type_representable: {}", + ::util::ppaux::ty_to_str(cx, ty)); + + // To avoid a stack overflow when checking an enum variant or struct that + // contains a different, structurally recursive type, maintain a stack + // of seen types and check recursion for each of them (issues #3008, #3779). + let mut seen: ~[DefId] = ~[]; + type_structurally_recursive(cx, &mut seen, ty) } pub fn type_is_trait(ty: t) -> bool { diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index fe9998786f35..cefda4372749 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/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)) { @@ -3410,6 +3413,33 @@ pub fn check_const_with_ty(fcx: @FnCtxt, writeback::resolve_type_vars_in_expr(fcx, e); } +/// Checks whether a type can be represented in memory. In particular, it +/// identifies types that contain themselves without indirection through a +/// pointer, which would mean their size is unbounded. This is different from +/// the question of whether a type can be instantiated. See the definition of +/// `check_instantiable`. +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: @@ -3565,7 +3595,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", @@ -3580,22 +3609,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: //