-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Expand weak alias types before collecting constrained/referenced late bound regions + refactorings #121344
Conversation
query normalize_weak_ty( | ||
goal: CanonicalProjectionGoal<'tcx> | ||
/// <div class="warning">Do not call this query directly: Invoke `normalize` instead.</div> | ||
query normalize_canonicalized_weak_ty( |
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.
As discussed I've renamed those queries to something “less appealing” (well I could also add a leading _
for good measure :P) but since I actually chose to use the term expand
for the new normalization routine, I can revert this change since their names no longer really clash.
/// | ||
/// [weak]: ty::Weak | ||
pub fn expand_weak_alias_tys<T: TypeFoldable<TyCtxt<'tcx>>>(self, value: T) -> T { | ||
value.fold_with(&mut WeakAliasTypeExpander { tcx: self, depth: 0 }) |
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.
I wonder if I need to track binders. I mean I didn't do that before either when I did the equivalent of expand_weak_alias_tys
inline in an ad hoc fashion but yeah, feels weird to not track binders even though instantiate
should take care of that (it does at least shift_bound_vars
).
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.
Binders do not matter here. You always instantiate the args from the same level, and you do not invoke the full projection normalizer at all.
@@ -1002,6 +1060,42 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for OpaqueTypeExpander<'tcx> { | |||
} | |||
} | |||
|
|||
struct WeakAliasTypeExpander<'tcx> { | |||
tcx: TyCtxt<'tcx>, | |||
depth: usize, |
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.
I'd like to note that I'm still using a plain depth counter over a hash set of DefId
s to detect overflows since that feels more memory efficient to me.
return ty.super_fold_with(self); | ||
}; | ||
if !self.tcx.recursion_limit().value_within_limit(self.depth) { | ||
let guar = self.tcx.dcx().delayed_bug("overflow expanding weak alias type"); |
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.
I'm still delaying a bug instead of reporting an overflow error since we don't have access to a Span
and neither do any the callers of expand_weak_alias_tys
and it would be a bit foolish to use a dummy span for this since it wouldn't get deduplicated with the other overflow errors and would actually show up in the user's stderr.
/// Peel off all [weak alias types] in this type until there are none left. | ||
/// | ||
/// This only expands weak alias types in “head” / outermost positions. It can | ||
/// be used over [expand_weak_alias_tys] as an optimization in situations where |
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.
“optimization modulo overflow behavior” I guess
@@ -258,12 +260,8 @@ struct LateBoundRegionsCollector { | |||
} | |||
|
|||
impl LateBoundRegionsCollector { | |||
fn new(tcx: TyCtxt<'tcx>, just_constrained: bool) -> Self { |
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.
huh why did this not cause a lint?
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.
Ah because it turns out my commits weren't as cleanly separated as I thought they would be ^^'. An earlier commit adds this unused binding and a later one removes it again. Let me fix that real quick.
Yea, I like the way this looks |
@bors r+ |
Fixing up the commit history |
…late bound regions
9bf9861
to
f515f99
Compare
@bors r=oli-obk |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#119203 (Correct the simd_masked_{load,store} intrinsic docs) - rust-lang#121277 (Refactor trait implementations in `core::convert::num`.) - rust-lang#121322 (Don't ICE when hitting overflow limit in fulfillment loop in next solver) - rust-lang#121323 (Don't use raw parameter types in `find_builder_fn`) - rust-lang#121344 (Expand weak alias types before collecting constrained/referenced late bound regions + refactorings) - rust-lang#121350 (Fix stray trait mismatch in `resolve_associated_item` for `AsyncFn`) - rust-lang#121352 (docs: add missing "the" to `str::strip_prefix` doc) Failed merges: - rust-lang#121340 (bootstrap: apply most of clippy's suggestions) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#121344 - fmease:lta-constr-by-input, r=oli-obk Expand weak alias types before collecting constrained/referenced late bound regions + refactorings Fixes rust-lang#114220. Follow-up to rust-lang#120780. r? `@oli-obk`
…cts, r=compiler-errors Also expand weak alias tys inside consts inside `expand_weak_alias_tys` Ever since rust-lang#121344 has been merged, I couldn't let go of the fear that I might've slipped a tiny bug into rustc (:P). Checking the type flags of the `Const` is strictly more correct than only checking the ones of the `Const`'s `Ty`. I don't think it's possible to trigger an ICE rn (i.e., one of the two `bug!("unexpected weak alias type")` I added in branches where `expand_weak_alias_tys` should've expanded *all* weak alias tys) because presently const exprs aren't allowed to capture late-bound vars. To be future-proof however, we should iron this out. A possible reproducer would be the following if I'm not mistaken (currently fails to compile due to the aforementioned restriction): ```rs #![feature(lazy_type_alias, adt_const_params, generic_const_exprs)] type F = for<'a> fn(A<{ S::<Weak<'a>>(loop {}) }>) -> &'a (); type A<const N: S<Weak<'static>>> = (); #[derive(PartialEq, Eq, std::marker::ConstParamTy)] struct S<T>(T); type Weak<'a> = &'a (); ``` Whether a late-bound region should actually be considered constrained by a const expr is a separate question — one which we don't need to answer until / unless we actually allow them in such contexts (probable answer: only inside the return exprs of a block but not inside the stmts). r? oli-obk (he's not available rn but that's fine) or types or compiler
Rollup merge of rust-lang#124990 - fmease:expand-weak-aliases-within-cts, r=compiler-errors Also expand weak alias tys inside consts inside `expand_weak_alias_tys` Ever since rust-lang#121344 has been merged, I couldn't let go of the fear that I might've slipped a tiny bug into rustc (:P). Checking the type flags of the `Const` is strictly more correct than only checking the ones of the `Const`'s `Ty`. I don't think it's possible to trigger an ICE rn (i.e., one of the two `bug!("unexpected weak alias type")` I added in branches where `expand_weak_alias_tys` should've expanded *all* weak alias tys) because presently const exprs aren't allowed to capture late-bound vars. To be future-proof however, we should iron this out. A possible reproducer would be the following if I'm not mistaken (currently fails to compile due to the aforementioned restriction): ```rs #![feature(lazy_type_alias, adt_const_params, generic_const_exprs)] type F = for<'a> fn(A<{ S::<Weak<'a>>(loop {}) }>) -> &'a (); type A<const N: S<Weak<'static>>> = (); #[derive(PartialEq, Eq, std::marker::ConstParamTy)] struct S<T>(T); type Weak<'a> = &'a (); ``` Whether a late-bound region should actually be considered constrained by a const expr is a separate question — one which we don't need to answer until / unless we actually allow them in such contexts (probable answer: only inside the return exprs of a block but not inside the stmts). r? oli-obk (he's not available rn but that's fine) or types or compiler
Fixes #114220.
Follow-up to #120780.
r? @oli-obk