-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Opaque type collection: Guard against endlessly recursing free alias types #143793
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
Conversation
@@ -223,7 +223,10 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> { | |||
} | |||
// Skips type aliases, as they are meant to be transparent. | |||
// FIXME(type_alias_impl_trait): can we require mentioning nested type aliases explicitly? | |||
ty::Alias(ty::Free, alias_ty) if alias_ty.def_id.is_local() => { | |||
ty::Alias(ty::Free, alias_ty) if let Some(def_id) = alias_ty.def_id.as_local() => { | |||
if !self.seen.insert(def_id) { |
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.
Of course, this only guards against cyclic alias types, not against "overly deep" ones. It's within the realm of possibility that this still overflows the stack on inputs like type T${n} = T${n+1}; type T${N} = ();
where 0 ≤ n
< N
where N
is large (like >10_000 which is the maximum I tested and which took a while to compile but didn't exhibit a stack overflow).
However, rustc struggles on such inputs anyway today whether eager or lazy (I already found a bunch of boring hangs).
Keeping track of depth instead of visited types would address that but I didn't want to make large modifications to this collector. I thought about utilizing tcx.expand_free_alias_tys
in appropriate places which can deal with such inputs. However, that might be incompatible with // FIXME(type_alias_impl_trait): can we require mentioning nested type aliases explicitly?
which I guess is referring to constructions like:
#![feature(type_alias_impl_trait)]
// #[define_opaque(Inner)] // <-- (?)
type Outer = Inner;
type Inner = impl Sized;
// ...
or sth. like that >.<
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.
Yea, those are fine. I think we can address generated worst-case input code situations as they occur. Most likely we have enough ensure_sufficient_stack
in place already.
@bors r+ rollup |
Opaque type collection: Guard against endlessly recursing free alias types See test description for technical details. Fixes rust-lang#131994. r? oli-obk (sry, your queue is large, so no rush & feel free to reassign)
Opaque type collection: Guard against endlessly recursing free alias types See test description for technical details. Fixes rust-lang#131994. r? oli-obk (sry, your queue is large, so no rush & feel free to reassign)
Opaque type collection: Guard against endlessly recursing free alias types See test description for technical details. Fixes rust-lang#131994. r? oli-obk (sry, your queue is large, so no rush & feel free to reassign)
Rollup of 11 pull requests Successful merges: - #143595 (add `const_make_global`; err for `const_allocate` ptrs if didn't call) - #143678 (Added error for invalid char cast) - #143793 (Opaque type collection: Guard against endlessly recursing free alias types) - #143820 (Fixed a core crate compilation failure when enabling the `optimize_for_size` feature on some targets) - #143829 (Trim `BorrowedCursor` API) - #143856 (Linting public reexport of private dependencies) - #143891 (Port `#[coverage]` to the new attribute system) - #143914 (Reword mismatched-lifetime-syntaxes text based on feedback) - #143922 (Improve path segment joining) - #143926 (Remove deprecated fields in bootstrap) - #143975 (type_id_eq: check that the hash fully matches the type) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 11 pull requests Successful merges: - #143326 (Remove deprecated `Error::description` impl from `c_str::FromBytesWithNulError`) - #143431 (Use relative visibility when noting sealed trait to reduce false positive) - #143550 (resolve: Use interior mutability for extern module map) - #143631 (update to literal-escaper-0.0.5) - #143793 (Opaque type collection: Guard against endlessly recursing free alias types) - #143880 (tests: Test line debuginfo for linebreaked function parameters) - #143914 (Reword mismatched-lifetime-syntaxes text based on feedback) - #143926 (Remove deprecated fields in bootstrap) - #143955 (Make frame spans appear on a separate trace line) - #143975 (type_id_eq: check that the hash fully matches the type) - #143984 (Fix ice for feature-gated `cfg` attributes applied to the crate) r? `@ghost` `@rustbot` modify labels: rollup
See test description for technical details.
Fixes #131994.
r? oli-obk (sry, your queue is large, so no rush & feel free to reassign)