bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

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

Browse source 
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)
This commit is contained in:
bors 2024-05-27 06:36:57 +00:00
commit fec98b3bbc
31 changed files with 320 additions and 212 deletions
Download patch file Download diff file Expand all files Collapse all files

14
compiler/rustc_middle/src/ty/context.rs
View file

@ -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"),

2
compiler/rustc_middle/src/ty/generics.rs
View file

@ -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")
}

53
compiler/rustc_middle/src/ty/region.rs
View file

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