Skip to content

Commit

Permalink
Rollup merge of rust-lang#120000 - smoelius:fix-clippy, r=fee1-dead
Browse files Browse the repository at this point in the history
Ensure `callee_id`s are body owners

This PR makes the `callee_id` argument of Clippy's `implements_trait_with_env` optional, and when it is passed, ensures it is a body owner.

rust-lang#118661 added the `callee_id` parameter to alleviate an ICE. Specifically, the `callee_id` is used to determine an "effect arg" in certain situations.

Frankly, I [do not completely understand](rust-lang#118661 (comment)) what an "effect arg" is. But the code that determines it seems to require that `callee_id` is a body owner:
- https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/src/tools/clippy/clippy_utils/src/ty.rs#L286-L288
- https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/ty/util.rs#L834
- https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/hir/map/mod.rs#L372

In the current head, some def ids passed as `callee_id`s are not body owners. This PR fixes that.

cc ``@rust-lang/clippy``

r? ``@fee1-dead``
  • Loading branch information
GuillaumeGomez authored Jan 20, 2024
2 parents 1236fa2 + 96b8eb7 commit 71cef76
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 23 deletions.
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,12 +451,12 @@ fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_r
&& let Some(def_id) = trait_ref.trait_def_id()
&& cx.tcx.is_diagnostic_item(sym::PartialEq, def_id)
&& let param_env = param_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id)
&& !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, adt.did(),&[])
&& !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, None, &[])
// If all of our fields implement `Eq`, we can implement `Eq` too
&& adt
.all_fields()
.map(|f| f.ty(cx.tcx, args))
.all(|ty| implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, adt.did(), &[]))
.all(|ty| implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, None, &[]))
{
span_lint_and_sugg(
cx,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ fn is_ref_iterable<'tcx>(
.liberate_late_bound_regions(fn_id, cx.tcx.fn_sig(fn_id).skip_binder())
&& let &[req_self_ty, req_res_ty] = &**sig.inputs_and_output
&& let param_env = cx.tcx.param_env(fn_id)
&& implements_trait_with_env(cx.tcx, param_env, req_self_ty, trait_id, fn_id, &[])
&& implements_trait_with_env(cx.tcx, param_env, req_self_ty, trait_id, Some(fn_id), &[])
&& let Some(into_iter_ty) =
make_normalized_projection_with_regions(cx.tcx, param_env, trait_id, sym!(IntoIter), [req_self_ty])
&& let req_res_ty = normalize_with_regions(cx.tcx, param_env, req_res_ty)
Expand Down
32 changes: 12 additions & 20 deletions src/tools/clippy/clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,36 +214,21 @@ pub fn implements_trait<'tcx>(
trait_id: DefId,
args: &[GenericArg<'tcx>],
) -> bool {
let callee_id = cx
.enclosing_body
.map(|body| cx.tcx.hir().body_owner(body).owner.to_def_id());
implements_trait_with_env_from_iter(
cx.tcx,
cx.param_env,
ty,
trait_id,
callee_id,
args.iter().map(|&x| Some(x)),
)
implements_trait_with_env_from_iter(cx.tcx, cx.param_env, ty, trait_id, None, args.iter().map(|&x| Some(x)))
}

/// Same as `implements_trait` but allows using a `ParamEnv` different from the lint context.
///
/// The `callee_id` argument is used to determine whether this is a function call in a `const fn` environment, used for checking const traits.
pub fn implements_trait_with_env<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
ty: Ty<'tcx>,
trait_id: DefId,
callee_id: DefId,
callee_id: Option<DefId>,
args: &[GenericArg<'tcx>],
) -> bool {
implements_trait_with_env_from_iter(
tcx,
param_env,
ty,
trait_id,
Some(callee_id),
args.iter().map(|&x| Some(x)),
)
implements_trait_with_env_from_iter(tcx, param_env, ty, trait_id, callee_id, args.iter().map(|&x| Some(x)))
}

/// Same as `implements_trait_from_env` but takes the arguments as an iterator.
Expand All @@ -258,6 +243,13 @@ pub fn implements_trait_with_env_from_iter<'tcx>(
// Clippy shouldn't have infer types
assert!(!ty.has_infer());

// If a `callee_id` is passed, then we assert that it is a body owner
// through calling `body_owner_kind`, which would panic if the callee
// does not have a body.
if let Some(callee_id) = callee_id {
let _ = tcx.hir().body_owner_kind(callee_id);
}

let ty = tcx.erase_regions(ty);
if ty.has_escaping_bound_vars() {
return false;
Expand Down

0 comments on commit 71cef76

Please sign in to comment.