Refactor away TraitRef::trait_def_id

[oli-obk]

The function

rust/src/librustc/hir/mod.rs

Lines 2150 to 2155 in 9b67bd4

	pub fn trait_def_id(&self) -> DefId {
	match self.path.def {
	Def::Trait(did) => did,
	Def::TraitAlias(did) => did,
	Def::Err => {
	FatalError.raise();

can abort compilation in case of errors. We're trying to eliminate as many (if not all) early aborts, so this needs to go. I haven't looked into how to fix this yet at all.

Originally reported in https://github.com/rust-lang/rust/pull/60462/files#r280262620

cc @estebank

[marmeladema]

Hello! I tried to look at that issue but it looks like its already fixed?

[bjorn3]

The code was moved to

rust/src/librustc_hir/hir.rs

Lines 2206 to 2212 in 34f7f55

	pub fn trait_def_id(&self) -> Option<DefId> {
	match self.path.res {
	Res::Def(DefKind::Trait | DefKind::TraitAlias, did) => Some(did),
	Res::Err => None,
	_ => unreachable!(),
	}
	}

We recently split the librustc crate into multiple smaller crates like librustc_hir. Then librustc got renamed to librustc_middle.

[marmeladema]

In this code, the call to FatalError.raise(); for the Res::Err arm is gone. That's i thought it was fixed. Is the issue about the call to unreachable!() or about the caller that might fatal error when None is returned?

[bjorn3]

unreachable!() has even worse ergonomics than fatal errors as it causes a compiler panic (ICE). I think this issue if about the _ => ... arm of the match that should be avoided.

[bugadani]

I've started working on this. So far, I don't understand why some of the downstream code can FatalError.raise(); if the DefId is None, while other parts ignore it.

[estebank]

@bugadani without knowing the specifics, the answer to that is usually just historical baggage: the compiler for a long time just relied on FatalError.raise() to avoid over-complicating later stages of the compiler. Now, in an effort to improve the user experience, we've tried to make errors non-fatal and later stages resilient to incomplete data. In some cases making that kind of change would be so big that we have punted on doing it and left FatalError.raise() in place instead of making their scope fallible.