fix supposedly unreachable bug! being reachable

Project: rust-lang/rust | Language: Rust | Concepts: type-systems, error-handling, compiler-internals | Difficulty: senior
Link: https://github.com/rust-lang/rust/pull/162173

The problem

A bug! macro in the compiler's well-formedness (WF) checking logic was assumed to be unreachable, but could actually be triggered in practice, causing an internal compiler error (ICE). This happened because the normalization step in wfcheck.rs could produce an InherentImpl form, which wasn't handled by the WF code and thus hit the bug!.

Why it's non-trivial

A naive fix might just remove the bug! or add a catch-all, but that could miss subtle invariants in the type system. The code must distinguish between different kinds of type aliases, handle normalization side-effects, and ensure that the right forms are processed in the right places—without introducing soundness holes or masking deeper bugs.

The approach

1. Update the handling of AliasConstKind::InherentImpl in WF checking:

Previously, encountering InherentImpl in this context would trigger a bug!:

-                        // please ping khyperia and/or BoxyUwU if this `bug!` fires
-                        ty::AliasConstKind::InherentImpl { .. } => bug!(
-                            "This ought to be unreachable, the entrypoints of WF should still have InherentSelf-form alias consts."
-                        ),
+                        // FIXME: This should be unreachable but isn't because we normalize in item
+                        // wfck before computing wf requirements
+                        ty::AliasConstKind::InherentImpl { .. } => {
+                            self.add_wf_preds_for_inherent_projection(alias_const.into());
+                            return;
+                        }

Annotation:
Instead of panicking, the code now handles InherentImpl by delegating to add_wf_preds_for_inherent_projection, just like with InherentSelf. The comment acknowledges that this situation ideally shouldn't occur, but normalization makes it possible.

2. Make self_ty access safer and more explicit:

Added an assertion to ensure self_ty is only called on valid variants:

+    fn debug_assert_has_self(self) {
+        // InherentConstImpl is deliberately omitted here, it is not self-format args
+        debug_assert_matches!(
+            self.kind,
+            AliasTermKind::ProjectionTy { .. }
+                | AliasTermKind::ProjectionConst { .. }
+                | AliasTermKind::InherentTy { .. }
+                | AliasTermKind::InherentConstSelf { .. },
+            "AliasTerm::self_ty is only valid on projection and inherent aliases"
+        );
+    }
+
     pub fn self_ty(self) -> I::Ty {
+        self.debug_assert_has_self();
         self.args.type_at(0)
     }

Annotation:
This ensures that self_ty is never called on InherentImpl, which doesn't have a "self" type in the same sense. This guards against future misuse and clarifies the intended invariants.

3. Adjust logic for when to compute inherent assoc term args:

-        if !data.self_ty().has_escaping_bound_vars() {
+        // In an ideal world, there are no escaping bound vars here. However, WF is jank, and
+        // sometimes there are. We can only `compute_inherent_assoc_term_args` if the Self ty in the
+        // args has no escaping bound vars. If we already have impl format args, though,
+        // `compute_inherent_assoc_term_args` is a no-op (and we have no Self type), so no need to
+        // check for escaping bound vars.
+        let can_compute_impl_args =
+            matches!(data.kind, ty::AliasTermKind::InherentConstImpl { .. })
+                || !data.self_ty().has_escaping_bound_vars();
+
+        if can_compute_impl_args {

Annotation:
This logic allows the code to skip the escaping bound vars check for the InherentConstImpl form, since it's already in the correct format and doesn't require further normalization.

4. Add regression tests:

Two new files ensure the bug is covered:

• wf-inherentimpl.rs (test case)
• wf-inherentimpl.old.stderr (expected error output)

From the review thread

[review] BoxyUwU:

                         // FIXME: This should be unreachable but isn't because we
                         // normalize in item wfck before computing wf requirements
                        | ty::AliasConstKind::InherentImpl { .. } => {

[review] BoxyUwU: comment on why ignoring escaping bound vars is fine with inherent impl form

Takeaways

• Handle all possible variants, even those believed unreachable, if upstream logic can produce them.
• Assert invariants close to where they're relied on, not just at API boundaries.
• When normalization or transformation steps change the shape of data, audit all downstream consumers for new cases.