Fix elided lifetime resolution & trait object lifetime defaulting for enum variant paths#154918
Fix elided lifetime resolution & trait object lifetime defaulting for enum variant paths#154918fmease wants to merge 2 commits intorust-lang:mainfrom
Conversation
| let type_def_id = match res { | ||
| Res::Def(DefKind::AssocTy, def_id) if depth == 1 => Some(self.tcx.parent(def_id)), | ||
| Res::Def(DefKind::Variant, def_id) if depth == 0 => Some(self.tcx.parent(def_id)), | ||
| Res::Def(DefKind::Variant, def_id) if depth == 0 || depth == 1 => { |
There was a problem hiding this comment.
We are in a position to fix this without any bad repercussions (namely breaking user code) due to this bug being covered by the lifetime elision bug that's fixed in the first commit and which dates all the way back to 2016 (#108224 (comment)).
That's because the new depth == 1 case can only be reached if the enum path segment has generic arguments (otherwise this function, visit_segment_args, wouldn't've been called in the first place) but due to #108224 that always leads to an error on stable/main.
Of course, if we pivot to dropping object lifetime defaulting in bodies (#129543 (review)), this issue should go away automatically.
There was a problem hiding this comment.
Once or if we stop utilizing object lifetime defaulting in bodies (cc #129543 (review)), this fix will become moot.
Admittedly, in the last paragraph of #129543 (comment) I stated that this fix would still be relevant for mGCA but I'm no longer super sure:
Indeed, trait object defaulting is generally relevant to mGCA, too (see open issue #151649), and so are enum variants in (direct) const args (which aren't backed by a body in which we could perform inference). However, I was unable to come up with a snippet where this fix is observable, at least not without "cheating" since dyn Trait can't be used as the type of const args (not sure if that'll change eventually?).
This commit makes the following artificial (!) mGCA snippet compile but I don't think it "counts".
#![feature(min_generic_const_args)]
#![feature(const_param_ty_unchecked)] // (!) internal, cherry-picked from open PR #153536
#![feature(generic_const_parameter_types, generic_const_items)]
#![feature(adt_const_params, unsized_const_params)]
#![allow(dead_code, incomplete_features, internal_features)]
enum E<'a, T: ?Sized + 'a> {
V,
H(&'a T),
}
// Before said commit, this would've failed with a type mismatch where
// expected: `E<'_, (dyn Trait + 'static)>`
// found: `E<'_, (dyn Trait + 'any)>`
type const _<'any>:
E<'any, dyn Trait + 'any> =
E::<'any, dyn Trait>::V {};
trait Trait {}There was a problem hiding this comment.
Well, I was able to replace the usage of const_param_ty_unchecked (internal) with structual_match (perma? unstable) but that's hardly better:
#![feature(min_generic_const_args)]
#![feature(generic_const_parameter_types, generic_const_items)]
#![feature(adt_const_params, unsized_const_params)]
#![feature(structural_match)] // (!)
#![allow(dead_code, incomplete_features)]
enum E<'a, T: ?Sized + 'a> {
V,
H(&'a Discard<T>), // H(&'a ())
}
impl<'a, T: ?Sized> PartialEq for E<'a, T> {
fn eq(&self, other: &Self) -> bool {
// indeed identical to native structural equality
std::mem::discriminant(self) == std::mem::discriminant(other)
}
}
impl<'a, T: ?Sized> Eq for E<'a, T> {}
impl<'a, T: ?Sized> std::marker::StructuralPartialEq for E<'a, T> {}
impl<'a, T: ?Sized> std::marker::ConstParamTy_ for E<'a, T> {}
type Discard<T> = <T as DiscardT>::Output;
trait DiscardT { type Output; }
impl<T: ?Sized> DiscardT for T { type Output = (); }
// Before said commit, this would've failed with a type mismatch where
// expected: `E<'_, (dyn Trait + 'static)>`
// found: `E<'_, (dyn Trait + 'any)>`
type const _<'any>:
E<'any, dyn Trait + 'any> =
E::<'any, dyn Trait>::V {};
trait Trait {}I guess I could add that as a test? Hmmm
| Res::Def(DefKind::AssocTy, def_id) if depth == 1 => Some(self.tcx.parent(def_id)), | ||
| Res::Def(DefKind::Variant, def_id) if depth == 0 => Some(self.tcx.parent(def_id)), | ||
| Res::Def(DefKind::Variant, def_id) if depth == 0 || depth == 1 => { | ||
| Some(self.tcx.parent(def_id)) |
There was a problem hiding this comment.
FYI, after #129543, this will look something like:
Res::Def(DefKind::Variant, def_id) => match rev_seg_idx {
0 => Some((self.tcx.parent(def_id), path.segments)),
1 => Some((self.tcx.parent(def_id), &path.segments[..=seg_idx])),
_ => None,
},… passed to the enum itself already
…enum path segment
|
r? types |
Context: #129543 (comment).
Fixes #108224.
I consider this to be a minor bug fix that doesn't demand a (T-types) FCP.
r? @lcnr or reassign