Don't substitute a GAT that has mismatched generics in OpaqueTypeCollector
#112876
Conversation
rustbot
added
S-waiting-on-review
rust-cloud-vms
bot
force-pushed
the
check-subst-compat-in-OpaqueTypeCollector
branch
from
65f0898
to
9932eab
Compare
| // Verify that the trait item and its implementation have compatible substs lists | ||
| fn check_substs_compatible<'tcx>( | ||
| tcx: TyCtxt<'tcx>, | ||
| assoc_item: ty::AssocItem, | ||
| substs: ty::SubstsRef<'tcx>, | ||
| ) -> bool { |
There was a problem hiding this comment.
Future nit: please do such moves in separate commits
| // If the trait ref of the associated item and the impl differs, | ||
| // then we can't use the impl's identity substitutions below, so | ||
| // just skip. | ||
| if alias_ty.trait_ref(self.tcx) == parent_trait_ref { |
There was a problem hiding this comment.
you can have something like -> <Self as Trait<NotIdentitySubsts...>>::Assoc, though I'm not sure if it ever results in anything but a cycle.
There was a problem hiding this comment.
Actually I think I can come up with an example where the old implementation results in surprising behavior.
There was a problem hiding this comment.
Yeah, here's an example that should error but does not:
#![feature(impl_trait_in_assoc_type)]
trait Foo<T> {
type Assoc;
fn test() -> u32;
}
struct DefinesOpaque;
impl Foo<DefinesOpaque> for () {
type Assoc = impl Sized;
fn test() -> <() as Foo<NoOpaques>>::Assoc {
let _: <Self as Foo<DefinesOpaque>>::Assoc = "";
1
}
}
struct NoOpaques;
impl Foo<NoOpaques> for () {
type Assoc = u32;
fn test() -> u32 { 1 }
}| // If the type is further specializable, then the type_of | ||
| // is not actually correct below. | ||
| if !assoc.defaultness(self.tcx).is_final() { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
At this point of complexity, should we just figure out how to normalize?
I tried before, but it requires duplicating some of the normalization logic 🙃
Basically I'd want exactly what the normalizer does, but be able to hook into fold_ty
There was a problem hiding this comment.
Hm, I don't think there is a good way of doing it right now...
Do you have any examples where this current approach of manually substituting is not enough? Like code that isn't currently passing but we want it to?
There was a problem hiding this comment.
Nope. I was just hoping to always go the normalization route like I need to with TAITs
rust-cloud-vms
bot
force-pushed
the
check-subst-compat-in-OpaqueTypeCollector
branch
from
9932eab
to
c4cd607
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#112876 (Don't substitute a GAT that has mismatched generics in `OpaqueTypeCollector`) - rust-lang#112906 (rustdoc: render the body of associated types before the where-clause) - rust-lang#112907 (Update cargo) - rust-lang#112908 (Print def_id on EarlyBoundRegion debug) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #111828
I didn't put up minimized UI tests for #112510 or #112873 because they'd minimize to literally the same code, but with different substs on the trait/impl. I don't think that warrants duplicate tests given the nature of the fix.
r? @oli-obk
Side-note: I checked, and this isn't fixed by #112652 -- I think we discussed whether or not that PR fixed it either intentionally or by accident. The code here isn't really touched by that PR either as far as I can tell?
Also, sorry, did some other drive-bys. Hope it doesn't make rebasing #112652 too difficult 😅