Auto merge of #125468 - BoxyUwU:remove_defid_from_regionparam, r=compiler-errors

Remove `DefId` from `EarlyParamRegion`

Currently we represent usages of `Region` parameters via the `ReEarlyParam` or `ReLateParam` variants. The `ReEarlyParam` is effectively equivalent to `TyKind::Param` and `ConstKind::Param` (i.e. it stores a `Symbol` and a `u32` index) however it also stores a `DefId` for the definition of the lifetime parameter.

This was used in roughly two places:
- Borrowck diagnostics instead of threading the appropriate `body_id` down to relevant locations. Interestingly there were already some places that had to pass down a `DefId` manually.
- Some opaque type checking logic was using the `DefId` field to track captured lifetimes

I've split this PR up into a commit for generate rote changes to diagnostics code to pass around a `DefId` manually everywhere, and another commit for the opaque type related changes which likely require more careful review as they might change the semantics of lints/errors.

Instead of manually passing the `DefId` around everywhere I previously tried to bundle it in with `TypeErrCtxt` but ran into issues with some call sites of `infcx.err_ctxt` being unable to provide a `DefId`, particularly places involved with trait solving and normalization. It might be worth investigating adding some new wrapper type to pass this around everywhere but I think this might be acceptable for now.

This pr also has the effect of reducing the size of `EarlyParamRegion` from 16 bytes -> 8 bytes. I wouldn't expect this to have any direct performance improvement however, other variants of `RegionKind` over `8` bytes are all because they contain a `BoundRegionKind` which is, as far as I know, mostly there for diagnostics. If we're ever able to remove this it would shrink the `RegionKind` type from `24` bytes to `12` (and with clever bit packing we might be able to get it to `8` bytes). I am curious what the performance impact would be of removing interning of `Region`'s if we ever manage to shrink `RegionKind` that much.

Sidenote: by removing the `DefId` the `Debug` output for `Region` has gotten significantly nicer. As an example see this opaque type debug print before vs after this PR:
`Opaque(DefId(0:13 ~ impl_trait_captures[aeb9]::foo::{opaque#0}), [DefId(0:9 ~ impl_trait_captures[aeb9]::foo::'a)_'a/#0, T, DefId(0:9 ~ impl_trait_captures[aeb9]::foo::'a)_'a/#0])`
`Opaque(DefId(0:13 ~ impl_trait_captures[aeb9]::foo::{opaque#0}), ['a/#0, T, 'a/#0])`

r? `@compiler-errors` (I would like someone who understands the opaque type setup to atleast review the type system commit, but the rest is likely reviewable by anyone)

3 files changed, 30 insertions, 39 deletions

diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs
index d138cd9ceb9..33ce08991bb 100644
--- a/compiler/rustc_middle/src/ty/context.rs
+++ b/compiler/rustc_middle/src/ty/context.rs
@@ -1489,13 +1489,14 @@ impl<'tcx> TyCtxt<'tcx> {
     }
 
     /// Returns the `DefId` and the `BoundRegionKind` corresponding to the given region.
-    pub fn is_suitable_region(self, mut region: Region<'tcx>) -> Option<FreeRegionInfo> {
+    pub fn is_suitable_region(
+        self,
+        generic_param_scope: LocalDefId,
+        mut region: Region<'tcx>,
+    ) -> Option<FreeRegionInfo> {
         let (suitable_region_binding_scope, bound_region) = loop {
-            let def_id = match region.kind() {
-                ty::ReLateParam(fr) => fr.bound_region.get_id()?.as_local()?,
-                ty::ReEarlyParam(ebr) => ebr.def_id.as_local()?,
-                _ => return None, // not a free region
-            };
+            let def_id =
+                region.opt_param_def_id(self, generic_param_scope.to_def_id())?.as_local()?;
             let scope = self.local_parent(def_id);
             if self.def_kind(scope) == DefKind::OpaqueTy {
                 // Lifetime params of opaque types are synthetic and thus irrelevant to
@@ -2634,7 +2635,6 @@ impl<'tcx> TyCtxt<'tcx> {
                     return ty::Region::new_early_param(
                         self,
                         ty::EarlyParamRegion {
-                            def_id: ebv,
                             index: generics
                                 .param_def_id_to_index(self, ebv)
                                 .expect("early-bound var should be present in fn generics"),
diff --git a/compiler/rustc_middle/src/ty/generics.rs b/compiler/rustc_middle/src/ty/generics.rs
index 1c4462fc1dc..69a86bda43b 100644
--- a/compiler/rustc_middle/src/ty/generics.rs
+++ b/compiler/rustc_middle/src/ty/generics.rs
@@ -66,7 +66,7 @@ pub struct GenericParamDef {
 impl GenericParamDef {
     pub fn to_early_bound_region_data(&self) -> ty::EarlyParamRegion {
         if let GenericParamDefKind::Lifetime = self.kind {
-            ty::EarlyParamRegion { def_id: self.def_id, index: self.index, name: self.name }
+            ty::EarlyParamRegion { index: self.index, name: self.name }
         } else {
             bug!("cannot convert a non-lifetime parameter def to an early bound region")
         }
diff --git a/compiler/rustc_middle/src/ty/region.rs b/compiler/rustc_middle/src/ty/region.rs
index 958e7e40168..0807cbf91d1 100644
--- a/compiler/rustc_middle/src/ty/region.rs
+++ b/compiler/rustc_middle/src/ty/region.rs
@@ -265,33 +265,6 @@ impl<'tcx> Region<'tcx> {
         flags
     }
 
-    /// Given an early-bound or free region, returns the `DefId` where it was bound.
-    /// For example, consider the regions in this snippet of code:
-    ///
-    /// ```ignore (illustrative)
-    /// impl<'a> Foo {
-    /// //   ^^ -- early bound, declared on an impl
-    ///
-    ///     fn bar<'b, 'c>(x: &self, y: &'b u32, z: &'c u64) where 'static: 'c
-    /// //         ^^  ^^     ^ anonymous, late-bound
-    /// //         |   early-bound, appears in where-clauses
-    /// //         late-bound, appears only in fn args
-    ///     {..}
-    /// }
-    /// ```
-    ///
-    /// Here, `free_region_binding_scope('a)` would return the `DefId`
-    /// of the impl, and for all the other highlighted regions, it
-    /// would return the `DefId` of the function. In other cases (not shown), this
-    /// function might return the `DefId` of a closure.
-    pub fn free_region_binding_scope(self, tcx: TyCtxt<'_>) -> DefId {
-        match *self {
-            ty::ReEarlyParam(br) => tcx.parent(br.def_id),
-            ty::ReLateParam(fr) => fr.scope,
-            _ => bug!("free_region_binding_scope invoked on inappropriate region: {:?}", self),
-        }
-    }
-
     /// True for free regions other than `'static`.
     pub fn is_param(self) -> bool {
         matches!(*self, ty::ReEarlyParam(_) | ty::ReLateParam(_))
@@ -321,6 +294,21 @@ impl<'tcx> Region<'tcx> {
             _ => bug!("expected region {:?} to be of kind ReVar", self),
         }
     }
+
+    /// Given some item `binding_item`, check if this region is a generic parameter introduced by it
+    /// or one of the parent generics. Returns the `DefId` of the parameter definition if so.
+    pub fn opt_param_def_id(self, tcx: TyCtxt<'tcx>, binding_item: DefId) -> Option<DefId> {
+        match self.kind() {
+            ty::ReEarlyParam(ebr) => {
+                Some(tcx.generics_of(binding_item).region_param(ebr, tcx).def_id)
+            }
+            ty::ReLateParam(ty::LateParamRegion {
+                bound_region: ty::BoundRegionKind::BrNamed(def_id, _),
+                ..
+            }) => Some(def_id),
+            _ => None,
+        }
+    }
 }
 
 impl<'tcx> Deref for Region<'tcx> {
@@ -335,16 +323,13 @@ impl<'tcx> Deref for Region<'tcx> {
 #[derive(Copy, Clone, PartialEq, Eq, Hash, TyEncodable, TyDecodable)]
 #[derive(HashStable)]
 pub struct EarlyParamRegion {
-    pub def_id: DefId,
     pub index: u32,
     pub name: Symbol,
 }
 
 impl std::fmt::Debug for EarlyParamRegion {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        // FIXME(BoxyUwU): self.def_id goes first because of `erased-regions-in-hidden-ty.rs` being impossible to write
-        // error annotations for otherwise. :). Ideally this would be `self.name, self.index, self.def_id`.
-        write!(f, "{:?}_{}/#{}", self.def_id, self.name, self.index)
+        write!(f, "{}/#{}", self.name, self.index)
     }
 }
 
@@ -352,6 +337,12 @@ impl std::fmt::Debug for EarlyParamRegion {
 #[derive(HashStable)]
 /// The parameter representation of late-bound function parameters, "some region
 /// at least as big as the scope `fr.scope`".
+///
+/// Similar to a placeholder region as we create `LateParam` regions when entering a binder
+/// except they are always in the root universe and instead of using a boundvar to distinguish
+/// between others we use the `DefId` of the parameter. For this reason the `bound_region` field
+/// should basically always be `BoundRegionKind::BrNamed` as otherwise there is no way of telling
+/// different parameters apart.
 pub struct LateParamRegion {
     pub scope: DefId,
     pub bound_region: BoundRegionKind,