about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-05-27 06:36:57 +0000
committerbors <bors@rust-lang.org>2024-05-27 06:36:57 +0000
commit8f23de1f4a829ae12822f3476f239458c4d9a65d (patch)
tree48c8424dd2f96614a6ac1e328720c2e250088cde
parenta456300af569750232afc0e98da28c4786fce19b (diff)
parent714e172ef215f6c57a2449175278f6e0c330a2af (diff)
downloadrust-8f23de1f4a829ae12822f3476f239458c4d9a65d.tar.gz
rust-8f23de1f4a829ae12822f3476f239458c4d9a65d.zip
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)
-rw-r--r--clippy_lints/src/ptr.rs19
1 files changed, 9 insertions, 10 deletions
diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs
index 65929cd5fea..38580dc5822 100644
--- a/clippy_lints/src/ptr.rs
+++ b/clippy_lints/src/ptr.rs
@@ -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;