From 53ddf2e57d59387443a062c4f4348801dea0c9dd Mon Sep 17 00:00:00 2001
From: Johannes Muenzel <jmuenzel@gmail.com>
Date: Mon, 6 Oct 2014 02:20:25 -0400
Subject: [PATCH] Fix several issues with the struct and enum
representability-checking logic.
---
src/librustc/middle/ty.rs | 181 ++++++++++++++++---------
src/test/compile-fail/issue-17431-1.rs | 16 +++
src/test/compile-fail/issue-17431-2.rs | 19 +++
src/test/compile-fail/issue-17431-3.rs | 18 +++
src/test/compile-fail/issue-17431-4.rs | 16 +++
src/test/compile-fail/issue-17431-5.rs | 18 +++
src/test/compile-fail/issue-17431-6.rs | 18 +++
src/test/compile-fail/issue-17431-7.rs | 16 +++
src/test/compile-fail/issue-3008-3.rs | 2 +
9 files changed, 241 insertions(+), 63 deletions(-)
create mode 100644 src/test/compile-fail/issue-17431-1.rs
create mode 100644 src/test/compile-fail/issue-17431-2.rs
create mode 100644 src/test/compile-fail/issue-17431-3.rs
create mode 100644 src/test/compile-fail/issue-17431-4.rs
create mode 100644 src/test/compile-fail/issue-17431-5.rs
create mode 100644 src/test/compile-fail/issue-17431-6.rs
create mode 100644 src/test/compile-fail/issue-17431-7.rs
diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs
index 10bc1da3acb..800a77160db 100644
--- a/src/librustc/middle/ty.rs
+++ b/src/librustc/middle/ty.rs
@@ -2823,11 +2823,14 @@ pub fn is_instantiable(cx: &ctxt, r_ty: t) -> bool {
/// 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(PartialEq)]
+///
+/// The ordering of the cases is significant. They are sorted so that cmp::max
+/// will keep the "more erroneous" of two values.
+#[deriving(PartialOrd, Ord, Eq, PartialEq, Show)]
pub enum Representability {
Representable,
- SelfRecursive,
ContainsRecursive,
+ SelfRecursive,
}
/// Check whether a type is representable. This means it cannot contain unboxed
@@ -2835,98 +2838,150 @@ pub enum Representability {
pub fn is_type_representable(cx: &ctxt, sp: Span, ty: t) -> Representability {
// Iterate until something non-representable is found
- fn find_nonrepresentable<It: Iterator<t>>(cx: &ctxt, sp: Span, seen: &mut Vec<DefId>,
+ fn find_nonrepresentable<It: Iterator<t>>(cx: &ctxt, sp: Span, seen: &mut Vec<t>,
mut iter: It) -> Representability {
- for ty in iter {
- let r = type_structurally_recursive(cx, sp, seen, ty);
- if r != Representable {
- return r
- }
- }
- Representable
+ iter.fold(Representable,
+ |r, ty| cmp::max(r, is_type_structurally_recursive(cx, sp, seen, ty)))
}
- // Does the type `ty` directly (without indirection through a pointer)
- // contain any types on stack `seen`?
- fn type_structurally_recursive(cx: &ctxt, sp: Span, seen: &mut Vec<DefId>,
- ty: t) -> Representability {
- debug!("type_structurally_recursive: {}",
- ::util::ppaux::ty_to_string(cx, ty));
-
- // Compare current type to previously seen types
+ fn are_inner_types_recursive(cx: &ctxt, sp: Span,
+ seen: &mut Vec<t>, ty: t) -> Representability {
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, sp, seen, ts.iter().map(|t| *t))
}
// Fixed-length vectors.
// FIXME(#11924) Behavior undecided for zero-length vectors.
ty_vec(ty, Some(_)) => {
- type_structurally_recursive(cx, sp, seen, ty)
+ is_type_structurally_recursive(cx, sp, seen, 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, sp, seen,
- fields.iter().map(|f| f.mt.ty));
- seen.pop();
- r
+ find_nonrepresentable(cx, sp, seen, fields.iter().map(|f| f.mt.ty))
}
-
ty_enum(did, ref substs) => {
- seen.push(did);
let vs = enum_variants(cx, did);
+ let iter = vs.iter()
+ .flat_map(|variant| { variant.args.iter() })
+ .map(|aty| { aty.subst_spanned(cx, substs, Some(sp)) });
- let mut r = Representable;
- for variant in vs.iter() {
- let iter = variant.args.iter().map(|aty| {
- aty.subst_spanned(cx, substs, Some(sp))
- });
- r = find_nonrepresentable(cx, sp, seen, iter);
-
- if r != Representable { break }
- }
-
- seen.pop();
- r
+ find_nonrepresentable(cx, sp, seen, iter)
}
-
ty_unboxed_closure(did, _) => {
let upvars = unboxed_closure_upvars(cx, did);
- find_nonrepresentable(cx,
- sp,
- seen,
- upvars.iter().map(|f| f.ty))
+ find_nonrepresentable(cx, sp, seen, upvars.iter().map(|f| f.ty))
}
-
_ => Representable,
}
}
+ fn same_struct_or_enum_def_id(ty: t, did: DefId) -> bool {
+ match get(ty).sty {
+ ty_struct(ty_did, _) | ty_enum(ty_did, _) => {
+ ty_did == did
+ }
+ _ => false
+ }
+ }
+
+ fn same_type(a: t, b: t) -> bool {
+ match (&get(a).sty, &get(b).sty) {
+ (&ty_struct(did_a, ref substs_a), &ty_struct(did_b, ref substs_b)) |
+ (&ty_enum(did_a, ref substs_a), &ty_enum(did_b, ref substs_b)) => {
+ if did_a != did_b {
+ return false;
+ }
+
+ let types_a = substs_a.types.get_slice(subst::TypeSpace);
+ let types_b = substs_b.types.get_slice(subst::TypeSpace);
+
+ let mut pairs = types_a.iter().zip(types_b.iter());
+
+ pairs.all(|(&a, &b)| same_type(a, b))
+ }
+ _ => {
+ type_id(a) == type_id(b)
+ }
+ }
+ }
+
+ // Does the type `ty` directly (without indirection through a pointer)
+ // contain any types on stack `seen`?
+ fn is_type_structurally_recursive(cx: &ctxt, sp: Span, seen: &mut Vec<t>,
+ ty: t) -> Representability {
+ debug!("is_type_structurally_recursive: {}",
+ ::util::ppaux::ty_to_string(cx, ty));
+
+ match get(ty).sty {
+ ty_struct(did, _) | ty_enum(did, _) => {
+ {
+ // Iterate through stack of previously seen types.
+ let mut iter = seen.iter();
+
+ // The first item in `seen` is the type we are actually curious about.
+ // We want to return SelfRecursive if this type contains itself.
+ // It is important that we DON'T take generic parameters into account
+ // for this check, so that Bar<T> in this example counts as SelfRecursive:
+ //
+ // struct Foo;
+ // struct Bar<T> { x: Bar<Foo> }
+
+ match iter.next() {
+ Some(&seen_type) => {
+ if same_struct_or_enum_def_id(seen_type, did) {
+ debug!("SelfRecursive: {} contains {}",
+ ::util::ppaux::ty_to_string(cx, seen_type),
+ ::util::ppaux::ty_to_string(cx, ty));
+ return SelfRecursive;
+ }
+ }
+ None => {}
+ }
+
+ // We also need to know whether the first item contains other types that
+ // are structurally recursive. If we don't catch this case, we will recurse
+ // infinitely for some inputs.
+ //
+ // It is important that we DO take generic parameters into account here,
+ // so that code like this is considered SelfRecursive, not ContainsRecursive:
+ //
+ // struct Foo { Option<Option<Foo>> }
+
+ for &seen_type in iter {
+ if same_type(ty, seen_type) {
+ debug!("ContainsRecursive: {} contains {}",
+ ::util::ppaux::ty_to_string(cx, seen_type),
+ ::util::ppaux::ty_to_string(cx, ty));
+ return ContainsRecursive;
+ }
+ }
+ }
+
+ // For structs and enums, track all previously seen types by pushing them
+ // onto the 'seen' stack.
+ seen.push(ty);
+ let out = are_inner_types_recursive(cx, sp, seen, ty);
+ seen.pop();
+ out
+ }
+ _ => {
+ // No need to push in other cases.
+ are_inner_types_recursive(cx, sp, seen, ty)
+ }
+ }
+ }
+
debug!("is_type_representable: {}",
::util::ppaux::ty_to_string(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: Vec<DefId> = Vec::new();
- type_structurally_recursive(cx, sp, &mut seen, ty)
+ let mut seen: Vec<t> = Vec::new();
+ let r = is_type_structurally_recursive(cx, sp, &mut seen, ty);
+ debug!("is_type_representable: {} is {}",
+ ::util::ppaux::ty_to_string(cx, ty), r);
+ r
}
pub fn type_is_trait(ty: t) -> bool {
diff --git a/src/test/compile-fail/issue-17431-1.rs b/src/test/compile-fail/issue-17431-1.rs
new file mode 100644
index 00000000000..896a9c06873
--- /dev/null
+++ b/src/test/compile-fail/issue-17431-1.rs
@@ -0,0 +1,16 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+struct Foo { foo: Option<Option<Foo>> }
+//~^ ERROR illegal recursive struct type; wrap the inner value in a box to make it representable
+
+impl Foo { fn bar(&self) {} }
+
+fn main() {}
diff --git a/src/test/compile-fail/issue-17431-2.rs b/src/test/compile-fail/issue-17431-2.rs
new file mode 100644
index 00000000000..886fe8d771a
--- /dev/null
+++ b/src/test/compile-fail/issue-17431-2.rs
@@ -0,0 +1,19 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+struct Baz { q: Option<Foo> }
+//~^ ERROR illegal recursive struct type; wrap the inner value in a box to make it representable
+
+struct Foo { q: Option<Baz> }
+//~^ ERROR illegal recursive struct type; wrap the inner value in a box to make it representable
+
+impl Foo { fn bar(&self) {} }
+
+fn main() {}
diff --git a/src/test/compile-fail/issue-17431-3.rs b/src/test/compile-fail/issue-17431-3.rs
new file mode 100644
index 00000000000..c1c450935f6
--- /dev/null
+++ b/src/test/compile-fail/issue-17431-3.rs
@@ -0,0 +1,18 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::sync::Mutex;
+
+struct Foo { foo: Mutex<Option<Foo>> }
+//~^ ERROR illegal recursive struct type; wrap the inner value in a box to make it representable
+
+impl Foo { fn bar(&self) {} }
+
+fn main() {}
diff --git a/src/test/compile-fail/issue-17431-4.rs b/src/test/compile-fail/issue-17431-4.rs
new file mode 100644
index 00000000000..1e27f025564
--- /dev/null
+++ b/src/test/compile-fail/issue-17431-4.rs
@@ -0,0 +1,16 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+struct Foo<T> { foo: Option<Option<Foo<T>>> }
+//~^ ERROR illegal recursive struct type; wrap the inner value in a box to make it representable
+
+impl<T> Foo<T> { fn bar(&self) {} }
+
+fn main() {}
diff --git a/src/test/compile-fail/issue-17431-5.rs b/src/test/compile-fail/issue-17431-5.rs
new file mode 100644
index 00000000000..d22d79ecaa5
--- /dev/null
+++ b/src/test/compile-fail/issue-17431-5.rs
@@ -0,0 +1,18 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+struct Foo { foo: Bar<Foo> }
+struct Bar<T> { x: Bar<Foo> }
+//~^ ERROR illegal recursive struct type; wrap the inner value in a box to make it representable
+
+impl Foo { fn foo(&self) {} }
+
+fn main() {
+}
diff --git a/src/test/compile-fail/issue-17431-6.rs b/src/test/compile-fail/issue-17431-6.rs
new file mode 100644
index 00000000000..8eac295353d
--- /dev/null
+++ b/src/test/compile-fail/issue-17431-6.rs
@@ -0,0 +1,18 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::sync::Mutex;
+
+enum Foo { X(Mutex<Option<Foo>>) }
+//~^ ERROR illegal recursive enum type; wrap the inner value in a box to make it representable
+
+impl Foo { fn bar(self) {} }
+
+fn main() {}
diff --git a/src/test/compile-fail/issue-17431-7.rs b/src/test/compile-fail/issue-17431-7.rs
new file mode 100644
index 00000000000..c64c040aa44
--- /dev/null
+++ b/src/test/compile-fail/issue-17431-7.rs
@@ -0,0 +1,16 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+enum Foo { Voo(Option<Option<Foo>>) }
+//~^ ERROR illegal recursive enum type; wrap the inner value in a box to make it representable
+
+impl Foo { fn bar(&self) {} }
+
+fn main() { }
diff --git a/src/test/compile-fail/issue-3008-3.rs b/src/test/compile-fail/issue-3008-3.rs
index b8ef57e2dd3..a338a01690d 100644
--- a/src/test/compile-fail/issue-3008-3.rs
+++ b/src/test/compile-fail/issue-3008-3.rs
@@ -12,5 +12,7 @@ enum E1 { V1(E2<E1>), }
enum E2<T> { V2(E2<E1>), }
//~^ ERROR illegal recursive enum type; wrap the inner value in a box to make it representable
+impl E1 { fn foo(&self) {} }
+
fn main() {
}