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 8f23de1f4a
1 changed files with 9 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

19
clippy_lints/src/ptr.rs
View file

@ -460,13 +460,19 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
}
None
}) {
if !lifetime.is_anonymous()
if let LifetimeName::Param(param_def_id) = lifetime.res
&& !lifetime.is_anonymous()
&& fn_sig
.output()
.walk()
.filter_map(|arg| {
arg.as_region().and_then(|lifetime| match lifetime.kind() {
ty::ReEarlyParam(r) => Some(r.def_id),
ty::ReEarlyParam(r) => Some(
cx.tcx
.generics_of(cx.tcx.parent(param_def_id.to_def_id()))
.region_param(r, cx.tcx)
.def_id,
),
ty::ReBound(_, r) => r.kind.get_id(),
ty::ReLateParam(r) => r.bound_region.get_id(),
ty::ReStatic
@ -476,14 +482,7 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
| ty::ReError(_) => None,
})
})
.any(|def_id| {
matches!(
lifetime.res,
LifetimeName::Param(param_def_id) if def_id
.as_local()
.is_some_and(|def_id| def_id == param_def_id),
)
})
.any(|def_id| def_id.as_local().is_some_and(|def_id| def_id == param_def_id))
{
// `&Cow<'a, T>` when the return type uses 'a is okay
return None;