diff options
| author | Guillaume Gomez <guillaume1.gomez@gmail.com> | 2024-01-20 20:06:34 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-01-20 20:06:34 +0100 |
| commit | 8b0931a2ba7bc5a4705ffba485a76c34ff02fc49 (patch) | |
| tree | f9133b2dd8b946b586b840d721e5ee90a80e8c50 | |
| parent | 21d719d90c6bf2a3a61aadab2f3e6020a5cac7f7 (diff) | |
| parent | 6a331e37fbe85cf69fa783bfc2fb2aba90d20bf6 (diff) | |
| download | rust-8b0931a2ba7bc5a4705ffba485a76c34ff02fc49.tar.gz rust-8b0931a2ba7bc5a4705ffba485a76c34ff02fc49.zip | |
Rollup merge of #120000 - smoelius:fix-clippy, r=fee1-dead
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. #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](https://github.com/rust-lang/rust/pull/118661#discussion_r1449013176) 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``
| -rw-r--r-- | clippy_lints/src/derive.rs | 4 | ||||
| -rw-r--r-- | clippy_lints/src/loops/explicit_iter_loop.rs | 2 | ||||
| -rw-r--r-- | clippy_utils/src/ty.rs | 32 |
3 files changed, 15 insertions, 23 deletions
diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index d1fe9f5cbec..df596338b95 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -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, diff --git a/clippy_lints/src/loops/explicit_iter_loop.rs b/clippy_lints/src/loops/explicit_iter_loop.rs index c7980060807..814ccaa36f5 100644 --- a/clippy_lints/src/loops/explicit_iter_loop.rs +++ b/clippy_lints/src/loops/explicit_iter_loop.rs @@ -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) diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 61d0663aa83..59ebe685c11 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -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. @@ -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; |
