Open fmease opened 1 year ago
@rustbot claim
EDIT: just realized that the linked PR is mine lol :sweat_smile:. I'd say it is a regression, albeit on an unstable feature.
@fmease
Implementation is blocked on the following issue (not sure I'll be able to fix this):
Inherent associated types aren't resolving paths properly. This can be observed from the code example provided in the OP with -Zunpretty=hir-tree
:
PathSegment {
ident: Item#0,
hir_id: HirId(DefId(0:3 ~ rust_out[dfb5]::main).5),
res: Err,
args: None,
infer_args: false,
}
(note the res: Err
).
I believe my impl of this issue is blocked on this.
Right, I stopped investigating after noticing Res::Err
. I believe we are not writing back the resolution / typeck result for IATs. Gonna check if I can fix the latter with or without AliasKind::Inherent
(#109410).
Ok, I have a pretty trivial fix ready for when that PR is merged.
As a – not so happy – update, with some changes I was able to update the resolution from Res::Err
to Res::Def(DefKind::AssocTy, _)
but that only works inside FnCtxt
s (used for function bodies, constants, etc.) and not inside ItemCtxt
s (used for type aliases, struct bodies etc.), so this can't be the final solution (I added a method called write_resolution
to trait AstConv
whose FnCtxt
impl delegates to FnCtxt::write_resolution
and whose ItemCtxt
impl does nothing (!)).
As far as I understand, that's not just a limitation of my implementation but a general architectural limitation: There generally isn't a meaningful Res
for those path segments found outside of function-like bodies. handle_res
(which I assume you want to modify) takes a Res
that was acquired via self.typeck_results().qpath_res(…)
and the docs typeck_results
talk about the same thing:
The way I understand it, this isn't a problem for trait associated types as they are unconditionally considered alive (which is also not ideal, see #66543).
There is a similar case where a path segment has Res::Err
and which doesn't involve inherent associated types:
fn f<T: Trait>() {
let _: T::Item /* Res::Err */ = loop {};
}
fn main() {
f::<Struct>();
}
struct Struct;
trait Trait { type Item; }
impl Trait for Struct { type Item = (); }
Since (trait) associated types are always alive, this works out "fine".
I might be slightly wrong with my analysis but I think this roughly sums it. I'm gonna drop a message on Zulip later though to double-check.
As a temporary mitigation, we can consider IATs always alive, too.
@fmease IIUC (and I don't know a lot about the compiler), it should be enough for now to only have the paths in the function body. We can then resolve it to a DefId
and mark the inherent associated type live.
For the case where the type isn't referenced from within a function, eg the following:
fn main() {
f::<Struct>();
}
struct Struct;
impl Struct { type Item = usize; }
struct Foo { s: Struct::Item }
we can leave it as is for now, until a better solution is found.
As an aside (and this might be naive), we should never have Res::Err
, right? If we truly can't resolve a path, then we have to emit a error, else there is a correct resolution?
Sorry for the delay. I'm gonna look into the matter one more time to see what we can do about paths outside of FnCtxt
s.
If I don't find anything useful, I will open a PR to update the Res
of paths in FnCtxt
s so you can swoop in and solve half the issue. For that, personally I'd prefer to consider the assoc tys as always alive if we cannot tell for sure (less noise in the most common case).
The presence of Res::Err
in legitimate paths after hir_typeck
has run defo sounds fishy (both for IATs & for the stable example I gave above). Afaik, type-dependent paths are Res::Err
after rustc_resolve
has run and they are then updated on a case by case basis during typeck.
All good, time as much time as you need. I agree we should consider Res::Err
IATs live if there isn't a good fix, however that should probably be looked at again before IATs are stabilized.
On nightly, the following code triggers
dead_code
even though it should not.Actually until recently,
dead_code
was never triggered at all for inherent associated types. PR #110277 was just merged which started visiting more associated items exposing this bug. Thedead_code
pass never considers IATs as live symbols (live_symbols
) even in cases where they should be.This feels like a regression but technically speaking it is not: We switched from one extreme to the other. Thus not marking as such.
@rustbot label A-lint F-inherent_associated_types requires-nightly