-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Fix stack overflow when checking for structural recursion #85012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,8 +28,18 @@ pub fn ty_is_representable<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, sp: Span) -> R | |
// 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<Ty<'_>> = Vec::new(); | ||
let mut shadow_seen: Vec<Ty<'_>> = Vec::new(); | ||
let mut representable_cache = FxHashMap::default(); | ||
let r = is_type_structurally_recursive(tcx, sp, &mut seen, &mut representable_cache, ty); | ||
let mut f_res = false; | ||
let r = is_type_structurally_recursive( | ||
tcx, | ||
sp, | ||
&mut seen, | ||
&mut shadow_seen, | ||
&mut representable_cache, | ||
ty, | ||
&mut f_res, | ||
); | ||
debug!("is_type_representable: {:?} is {:?}", ty, r); | ||
r | ||
} | ||
|
@@ -48,21 +58,38 @@ fn are_inner_types_recursive<'tcx>( | |
tcx: TyCtxt<'tcx>, | ||
sp: Span, | ||
seen: &mut Vec<Ty<'tcx>>, | ||
shadow_seen: &mut Vec<Ty<'tcx>>, | ||
representable_cache: &mut FxHashMap<Ty<'tcx>, Representability>, | ||
ty: Ty<'tcx>, | ||
f_res: &mut bool, | ||
) -> Representability { | ||
debug!("are_inner_types_recursive({:?}, {:?}, {:?})", ty, seen, shadow_seen); | ||
match ty.kind() { | ||
ty::Tuple(..) => { | ||
// Find non representable | ||
fold_repr( | ||
ty.tuple_fields().map(|ty| { | ||
is_type_structurally_recursive(tcx, sp, seen, representable_cache, ty) | ||
}), | ||
) | ||
fold_repr(ty.tuple_fields().map(|ty| { | ||
is_type_structurally_recursive( | ||
tcx, | ||
sp, | ||
seen, | ||
shadow_seen, | ||
representable_cache, | ||
ty, | ||
f_res, | ||
) | ||
})) | ||
} | ||
// Fixed-length vectors. | ||
// FIXME(#11924) Behavior undecided for zero-length vectors. | ||
ty::Array(ty, _) => is_type_structurally_recursive(tcx, sp, seen, representable_cache, ty), | ||
ty::Array(ty, _) => is_type_structurally_recursive( | ||
tcx, | ||
sp, | ||
seen, | ||
shadow_seen, | ||
representable_cache, | ||
ty, | ||
f_res, | ||
), | ||
ty::Adt(def, substs) => { | ||
// Find non representable fields with their spans | ||
fold_repr(def.all_fields().map(|field| { | ||
|
@@ -76,12 +103,142 @@ fn are_inner_types_recursive<'tcx>( | |
Some(hir::Node::Field(field)) => field.ty.span, | ||
_ => sp, | ||
}; | ||
match is_type_structurally_recursive(tcx, span, seen, representable_cache, ty) { | ||
Representability::SelfRecursive(_) => { | ||
Representability::SelfRecursive(vec![span]) | ||
|
||
let mut result = None; | ||
|
||
// First, we check whether the field type per se is representable. | ||
// This catches cases as in #74224 and #84611. There is a special | ||
// case related to mutual recursion, though; consider this example: | ||
// | ||
// struct A<T> { | ||
// z: T, | ||
// x: B<T>, | ||
// } | ||
// | ||
// struct B<T> { | ||
// y: A<T> | ||
// } | ||
// | ||
// Here, without the following special case, both A and B are | ||
// ContainsRecursive, which is a problem because we only report | ||
// errors for SelfRecursive. We fix this by detecting this special | ||
// case (shadow_seen.first() is the type we are originally | ||
// interested in, and if we ever encounter the same AdtDef again, | ||
// we know that it must be SelfRecursive) and "forcibly" returning | ||
// SelfRecursive (by setting f_res, which tells the calling | ||
// invocations of are_inner_types_representable to forward the | ||
// result without adjusting). | ||
if shadow_seen.len() > 1 && shadow_seen.len() > seen.len() { | ||
match shadow_seen.first().map(|ty| ty.kind()) { | ||
Some(ty::Adt(f_def, _)) => { | ||
if f_def == def { | ||
*f_res = true; | ||
result = Some(Representability::SelfRecursive(vec![span])); | ||
} | ||
} | ||
Some(_) => { | ||
bug!("shadow_seen stack contains non-ADT type: {:?}", ty); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe shadow stack could store already extracted |
||
None => unreachable!(), | ||
} | ||
} | ||
|
||
if result == None { | ||
result = Some(Representability::Representable); | ||
|
||
// Now, we check whether the field types per se are representable, e.g. | ||
// for struct Foo { x: Option<Foo> }, we first check whether Option<_> | ||
// by itself is representable (which it is), and the nesting of Foo | ||
// will be detected later. This is necessary for #74224 and #84611. | ||
|
||
// If we have encountered an ADT definition that we have not seen | ||
// before (no need to check them twice), recurse to see whether that | ||
// definition is SelfRecursive. If so, we must be ContainsRecursive. | ||
if shadow_seen.iter().len() > 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
&& !shadow_seen.iter().take(shadow_seen.iter().len() - 1).any(|seen_ty| { | ||
match seen_ty.kind() { | ||
ty::Adt(seen_def, _) => seen_def == def, | ||
_ => { | ||
bug!("seen stack contains non-ADT type: {:?}", seen_ty); | ||
} | ||
} | ||
}) | ||
{ | ||
let adt_def_id = def.did; | ||
let raw_adt_ty = tcx.type_of(adt_def_id); | ||
debug!("are_inner_types_recursive: checking nested type: {:?}", raw_adt_ty); | ||
|
||
// Check independently whether the ADT is SelfRecursive. If so, | ||
// we must be ContainsRecursive (except for the special case | ||
// mentioned above). | ||
let mut nested_seen: Vec<Ty<'_>> = vec![]; | ||
result = Some( | ||
match is_type_structurally_recursive( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This independent check for structural recursion belongs somewhere outside this fold, since it not specific to a field. Outside this function most likely, since the check is not strictly about the inner types like the name |
||
tcx, | ||
span, | ||
&mut nested_seen, | ||
shadow_seen, | ||
representable_cache, | ||
raw_adt_ty, | ||
f_res, | ||
) { | ||
Representability::SelfRecursive(_) => { | ||
if *f_res { | ||
Representability::SelfRecursive(vec![span]) | ||
} else { | ||
Representability::ContainsRecursive | ||
} | ||
} | ||
x => x, | ||
}, | ||
); | ||
} | ||
|
||
// We only enter the following block if the type looks representable | ||
// so far. This is necessary for cases such as this one (#74224): | ||
// | ||
// struct A<T> { | ||
// x: T, | ||
// y: A<A<T>>, | ||
// } | ||
// | ||
// struct B { | ||
// z: A<usize> | ||
// } | ||
// | ||
// When checking B, we recurse into A and check field y of type | ||
// A<A<usize>>. We haven't seen this exact type before, so we recurse | ||
// into A<A<usize>>, which contains, A<A<A<usize>>>, and so forth, | ||
// ad infinitum. We can prevent this from happening by first checking | ||
// A separately (the code above) and only checking for nested Bs if | ||
// A actually looks representable (which it wouldn't in this example). | ||
if result == Some(Representability::Representable) { | ||
// Now, even if the type is representable (e.g. Option<_>), | ||
// it might still contribute to a recursive type, e.g.: | ||
// struct Foo { x: Option<Option<Foo>> } | ||
// These cases are handled by passing the full `seen` | ||
// stack to is_type_structurally_recursive (instead of the | ||
// empty `nested_seen` above): | ||
result = Some( | ||
match is_type_structurally_recursive( | ||
tcx, | ||
span, | ||
seen, | ||
shadow_seen, | ||
representable_cache, | ||
ty, | ||
f_res, | ||
) { | ||
Representability::SelfRecursive(_) => { | ||
Representability::SelfRecursive(vec![span]) | ||
} | ||
x => x, | ||
}, | ||
); | ||
} | ||
x => x, | ||
} | ||
|
||
result.unwrap() | ||
})) | ||
} | ||
ty::Closure(..) => { | ||
|
@@ -106,8 +263,10 @@ fn is_type_structurally_recursive<'tcx>( | |
tcx: TyCtxt<'tcx>, | ||
sp: Span, | ||
seen: &mut Vec<Ty<'tcx>>, | ||
shadow_seen: &mut Vec<Ty<'tcx>>, | ||
representable_cache: &mut FxHashMap<Ty<'tcx>, Representability>, | ||
ty: Ty<'tcx>, | ||
f_res: &mut bool, | ||
) -> Representability { | ||
debug!("is_type_structurally_recursive: {:?} {:?}", ty, sp); | ||
if let Some(representability) = representable_cache.get(ty) { | ||
|
@@ -118,8 +277,15 @@ fn is_type_structurally_recursive<'tcx>( | |
return representability.clone(); | ||
} | ||
|
||
let representability = | ||
is_type_structurally_recursive_inner(tcx, sp, seen, representable_cache, ty); | ||
let representability = is_type_structurally_recursive_inner( | ||
tcx, | ||
sp, | ||
seen, | ||
shadow_seen, | ||
representable_cache, | ||
ty, | ||
f_res, | ||
); | ||
|
||
representable_cache.insert(ty, representability.clone()); | ||
representability | ||
|
@@ -129,12 +295,16 @@ fn is_type_structurally_recursive_inner<'tcx>( | |
tcx: TyCtxt<'tcx>, | ||
sp: Span, | ||
seen: &mut Vec<Ty<'tcx>>, | ||
shadow_seen: &mut Vec<Ty<'tcx>>, | ||
representable_cache: &mut FxHashMap<Ty<'tcx>, Representability>, | ||
ty: Ty<'tcx>, | ||
f_res: &mut bool, | ||
) -> Representability { | ||
match ty.kind() { | ||
ty::Adt(def, _) => { | ||
{ | ||
debug!("is_type_structurally_recursive_inner: adt: {:?}, seen: {:?}", ty, seen); | ||
|
||
// Iterate through stack of previously seen types. | ||
let mut iter = seen.iter(); | ||
|
||
|
@@ -158,8 +328,10 @@ fn is_type_structurally_recursive_inner<'tcx>( | |
// 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: | ||
// here, because nesting e.g. Options is allowed (as long as the | ||
// definition of Option doesn't itself include an Option field, which | ||
// would be a case of SelfRecursive above). The following, too, counts | ||
// as SelfRecursive: | ||
// | ||
// struct Foo { Option<Option<Foo>> } | ||
|
||
|
@@ -174,13 +346,23 @@ fn is_type_structurally_recursive_inner<'tcx>( | |
// 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(tcx, sp, seen, representable_cache, ty); | ||
shadow_seen.push(ty); | ||
let out = are_inner_types_recursive( | ||
tcx, | ||
sp, | ||
seen, | ||
shadow_seen, | ||
representable_cache, | ||
ty, | ||
f_res, | ||
); | ||
shadow_seen.pop(); | ||
seen.pop(); | ||
out | ||
} | ||
_ => { | ||
// No need to push in other cases. | ||
are_inner_types_recursive(tcx, sp, seen, representable_cache, ty) | ||
are_inner_types_recursive(tcx, sp, seen, shadow_seen, representable_cache, ty, f_res) | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
struct A<T> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There has been an effort to organize tests by the area of compiler they exercise, and we try to avoid additional tests into Maybe we could have a new directory for structural recursion check, if no other directory is appropriate? |
||
//~^ ERROR recursive type `A` has infinite size | ||
x: T, | ||
y: A<A<T>>, | ||
} | ||
|
||
struct B { | ||
z: A<usize> | ||
} | ||
|
||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
error[E0072]: recursive type `A` has infinite size | ||
--> $DIR/issue-74224.rs:1:1 | ||
| | ||
LL | struct A<T> { | ||
| ^^^^^^^^^^^ recursive type has infinite size | ||
... | ||
LL | y: A<A<T>>, | ||
| ------- recursive without indirection | ||
| | ||
help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `A` representable | ||
| | ||
LL | y: Box<A<A<T>>>, | ||
| ^^^^ ^ | ||
|
||
error: aborting due to previous error | ||
|
||
For more information about this error, try `rustc --explain E0072`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
struct Foo<T> { | ||
//~^ ERROR recursive type `Foo` has infinite size | ||
x: Foo<[T; 1]>, | ||
y: T, | ||
} | ||
|
||
struct Bar { | ||
x: Foo<Bar>, | ||
} | ||
|
||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
error[E0072]: recursive type `Foo` has infinite size | ||
--> $DIR/issue-84611.rs:1:1 | ||
| | ||
LL | struct Foo<T> { | ||
| ^^^^^^^^^^^^^ recursive type has infinite size | ||
LL | | ||
LL | x: Foo<[T; 1]>, | ||
| ----------- recursive without indirection | ||
| | ||
help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Foo` representable | ||
| | ||
LL | x: Box<Foo<[T; 1]>>, | ||
| ^^^^ ^ | ||
|
||
error: aborting due to previous error | ||
|
||
For more information about this error, try `rustc --explain E0072`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
struct A<T> { | ||
//~^ ERROR recursive type `A` has infinite size | ||
x: T, | ||
y: B<T>, | ||
} | ||
|
||
struct B<T> { | ||
//~^ ERROR recursive type `B` has infinite size | ||
z: A<T> | ||
} | ||
|
||
struct C<T> { | ||
//~^ ERROR recursive type `C` has infinite size | ||
x: T, | ||
y: Option<Option<D<T>>>, | ||
} | ||
|
||
struct D<T> { | ||
//~^ ERROR recursive type `D` has infinite size | ||
z: Option<Option<C<T>>>, | ||
} | ||
|
||
fn main() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update the comment above and describe what does it mean for a type to be present on
seen
stack andshadow_seen
stack.