Fix bound var resolution for trait aliases#152309
Fix bound var resolution for trait aliases#152309rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
78141c3 to
6c244da
Compare
|
r? me |
There was a problem hiding this comment.
Your simple patch does seem to fix #152244, too. I didn't expect it to be that simple… I'm still gonna investigate a bit more whether it truly fixes the reported issue at its core.
There was a problem hiding this comment.
Ah, now I see. I've missed that supertrait_hrtb_vars uses explicit_supertraits_containing_assoc_item under the hood which "expands" / deals with trait aliases just fine.
| | DefKind::TyAlias | ||
| | DefKind::Trait, | ||
| | DefKind::Trait | ||
| | DefKind::TraitAlias, |
There was a problem hiding this comment.
Adding TraitAlias to this list does constitute a behavioral change. Namely, it affects trait object lifetime defaulting. Namely, it makes trait alias refs eligible containers which would sort of fix #140710, too, except that there's still the same off-by-one error that would be fixed by my PR #129543.
I want to leave that error in for now since I hope that that PR will go into FCP soon. It just means that your PR leads us to reject certain programs that use the unstable feature trait_alias. E.g., it'll reject:
trait Trait<'a, T: ?Sized + 'a> = std::fmt::Debug;
type X<'r> = dyn Trait<'r, dyn std::fmt::Debug>;That's fine by me. I'll get fixed by me soon anyway 🤞
6c244da to
867b1a8
Compare
There was a problem hiding this comment.
Thanks! Could you please
- also add Fixes #152244 to the PR description
- generalize the PR title as it actually fixes the RBV trait alias issue as a whole (not just for RTNs but for all assoc item kinds)
- add the two reproducers from #152244 as test cases
- basically remove all sections from the PR description (Summary + Problem + Root Cause + Fix) and only leave the two Fixes #NNN
- currently, the whole description is extremely verbose and its contents self-evident from the patch
Regarding (3), I'm thinking of something like
// Check that we're successfully collecting bound vars behind trait aliases.
// Regression test for <https://github.com/rust-lang/rust/issues/152244>.
//@ check-pass
//@ needs-rustc-debug-assertions
#![feature(trait_alias)]
trait A<'a> { type X; }
trait B: for<'a> A<'a> {}
trait C = B;
fn f<T>() where T: C<X: Copy> {}
fn g<T>() where T: C<X: for<'r> A<'r>> {}Re. #140710, while your PR does render trait aliases eligible containers wrt. trait object lifetime defaulting (which the linked issue requests), it ultimately doesn't fix the provided snippet due to the preexisting off-by-one error that'll be fixed by me in #129543. Therefore, let's keep that open, I'll update it once this PR is merged and add the actual regression tests to PR #129543.
tests/ui/associated-type-bounds/return-type-notation/trait-alias.rs
Outdated
Show resolved
Hide resolved
867b1a8 to
9433d7b
Compare
|
Updated PR description and title, fixed repro format and added new repro. |
|
(You've linked to the wrong PRs but I've fixed it myself) @bors r+ rollup |
…alias, r=fmease Fix bound var resolution for trait aliases Fixes rust-lang#152158 Fixes rust-lang#152244
This comment has been minimized.
This comment has been minimized.
|
Ah, my bad. The new test needs a @bors r- |
Handle DefKind::TraitAlias in resolve_bound_vars so that associated item constraints and return type notation work through trait aliases.
9433d7b to
7c5ea7f
Compare
|
@bors r+ |
…alias, r=fmease Fix bound var resolution for trait aliases Fixes rust-lang#152158 Fixes rust-lang#152244
…alias, r=fmease Fix bound var resolution for trait aliases Fixes rust-lang#152158 Fixes rust-lang#152244
…alias, r=fmease Fix bound var resolution for trait aliases Fixes rust-lang#152158 Fixes rust-lang#152244
…uwer Rollup of 7 pull requests Successful merges: - #151455 (Fix `SourceFile::normalized_byte_pos`) - #152250 (Remove support for slugs in diagnostic messages) - #152322 (Replace some `feature(core_intrinsics)` with stable hints) - #152328 (Fix a few diagnostics) - #151640 (Cleanup offload datatransfer) - #152212 (Port some attributes to the attr parser) - #152309 (Fix bound var resolution for trait aliases)
…alias, r=fmease Fix bound var resolution for trait aliases Fixes rust-lang#152158 Fixes rust-lang#152244
…uwer Rollup of 8 pull requests Successful merges: - #151455 (Fix `SourceFile::normalized_byte_pos`) - #152250 (Remove support for slugs in diagnostic messages) - #152322 (Replace some `feature(core_intrinsics)` with stable hints) - #151640 (Cleanup offload datatransfer) - #152212 (Port some attributes to the attr parser) - #152309 (Fix bound var resolution for trait aliases) - #152339 (diagnostics: fix ICE in closure signature mismatch) - #152341 (`cfg_select!`: allow optional comma after `{ /* ... */ }`)
…uwer Rollup of 9 pull requests Successful merges: - #151455 (Fix `SourceFile::normalized_byte_pos`) - #152250 (Remove support for slugs in diagnostic messages) - #152322 (Replace some `feature(core_intrinsics)` with stable hints) - #152328 (Fix a few diagnostics) - #151640 (Cleanup offload datatransfer) - #152212 (Port some attributes to the attr parser) - #152309 (Fix bound var resolution for trait aliases) - #152339 (diagnostics: fix ICE in closure signature mismatch) - #152341 (`cfg_select!`: allow optional comma after `{ /* ... */ }`)
Fixes #152158
Fixes #152244