Replace all item identification utils #15682
Conversation
Jarcho
force-pushed
the
util_1
branch
2 times, most recently
from
73804d2 to
bdd440a
Compare
rustbot
added
the
S-waiting-on-review
|
Some changes occurred in clippy_lints/src/doc cc @notriddle |
|
rustbot has assigned @samueltardieu. Use |
| } | ||
|
|
||
| if let Some(id) = path_to_local(cast_expr) | ||
| if let Some(id) = cast_expr.res_local_id() |
There was a problem hiding this comment.
Shouldn't it be opt_res_local_id() for consistency with opt_diag_name() and others?
There was a problem hiding this comment.
I originally had it as opt_local_id, but changed it to res_local_id to match all the other names to get a path's resolution. I'm not strongly against opt_res_local_id, but it's getting a little on the wordy side. I'm not really attached to many of the names here.
File you want to look at for the new functions is clippy_utils/src/res.rs. Just noticed this is 279 files.
|
I've opened this Zulip thread for the non-technical discussion. |
| // It's possible to get the `TypeckResults` for any other body, but | ||
| // attempting to lookup the type of something across bodies like this | ||
| // is a good indication of a bug. | ||
| debug_assert!(false, "attempted type-dependent lookup in a non-body context"); |
There was a problem hiding this comment.
Maybe indicate in comments that if this is not a bug, then TyCtxt::typeck_body() can be used to get the appropriate TypeCkResults, so that developpers hit by the assertion can use it if justified.
| } | ||
|
|
||
| /// A `QPath` with the `HirId` of the node containing it. | ||
| type QPathId<'tcx> = (&'tcx QPath<'tcx>, HirId); |
There was a problem hiding this comment.
Could be made pub since it is used in pub trait method signatures.
There was a problem hiding this comment.
These aliases only exist to make the trait signatures shorter since they're written quite a few times. Outside the module it shouldn't even come up and if it does it will be written once.
| } | ||
|
|
||
| /// A resolved path and the explicit `Self` type if there is one. | ||
| type OptResPath<'tcx> = (Option<&'tcx hir::Ty<'tcx>>, Option<&'tcx Path<'tcx>>); |
There was a problem hiding this comment.
Could be useful if pub as well
This comment has been minimized.
This comment has been minimized.
|
@Jarcho Can you rebase? |
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
rustbot
removed
the
S-waiting-on-review
3 participants
This introduces a new way of identifying items/paths using extension traits with a composable set of functions rather than unique functions for various combinations of starting points and target items. Altogether this is a set of five traits:
MaybeTypeckRes: Allows bothLateContextandTypeckResultsto be used for type-dependent lookup. The implementation here will avoid ICEs by returningNonewhen debug assertions are disabled. With assertions this will assert that we don't silently lookup anything from a different body than the current one and that a definition actually exists.HasHirId: Simply a convenience to allow not typing.hir_idat call sites.MaybeQPath: This is the oldMaybePath. Extension functions for type-dependent path lookups exist here. A lot of these functions aren't used in the current PR, but what they accomplish is done in various places I haven't cleaned up yet.MaybeResPath: LikeMaybeQPath, but only does non-type-dependent lookup (QPath::Resolved).MaybeDef: Extension functions for identifying the current definition and accessing properties. Implemented for several types for convenience.MaybeDefis implemented forOptionto allow chaining methods together. e.g.cx.ty_based_def(e).opt_parent(cx).opt_impl_ty(cx).is_diag_item(..)would chainingand_thenorif leton every step.MaybeQPathandMaybeResPathare also implemented forOptionfor the same reason.ty_based_defis just a shorter name fortype_dependent_def. I'm not really attached to it, but it's nice that it's a little shorter.changelog: none