dead_code: Inherent associated types are unconditionally considered dead

[fmease]

On nightly, the following code triggers dead_code even though it should not.

#![feature(inherent_associated_types)]
#![allow(incomplete_features)]

fn main() {
    let _: Struct::Item = ();
}

struct Struct;
impl Struct { type Item = (); }

warning: associated type `Item` is never used
 --> min.rs:9:20
  |
9 | impl Struct { type Item = (); }
  | -----------        ^^^^
  | |
  | associated type in this implementation
  |
  = note: `#[warn(dead_code)]` on by default

warning: 1 warning emitted

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.
The dead_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

[Ezrashaw]

@rustbot claim

EDIT: just realized that the linked PR is mine lol 😅. I'd say it is a regression, albeit on an unstable feature.

[Ezrashaw]

@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.

[fmease]

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).

[Ezrashaw]

Ok, I have a pretty trivial fix ready for when that PR is merged.

[fmease]

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 FnCtxts (used for function bodies, constants, etc.) and not inside ItemCtxts (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:

rust/compiler/rustc_passes/src/dead.rs

Lines 63 to 70 in de96f3d

	/// Gets the type-checking results for the current body.
	/// As this will ICE if called outside bodies, only call when working with
	/// `Expr` or `Pat` nodes (they are guaranteed to be found only in bodies).
	#[track_caller]
	fn typeck_results(&self) -> &'tcx ty::TypeckResults<'tcx> {
	self.maybe_typeck_results
	.expect("`MarkSymbolVisitor::typeck_results` called outside of body")
	}

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".

[fmease]

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.

[Ezrashaw]

@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?

[fmease]

Sorry for the delay. I'm gonna look into the matter one more time to see what we can do about paths outside of FnCtxts.

If I don't find anything useful, I will open a PR to update the Res of paths in FnCtxts 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.

[Ezrashaw]

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.