From 344b7bca860ff9559b60d54350312e4ac1e88481 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Fri, 14 Oct 2022 13:21:59 -0400 Subject: [PATCH] Don't lint `ptr_arg` when used as an incompatible trait object --- clippy_lints/src/ptr.rs | 46 ++++++++++++++++++++++++++++++++++++++++- tests/ui/ptr_arg.rs | 30 ++++++++++++++++++++++++++- tests/ui/ptr_arg.stderr | 20 +++++++++++++++++- 3 files changed, 93 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 2d80236327a..b589ac4a122 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -15,13 +15,17 @@ ImplItemKind, ItemKind, Lifetime, LifetimeName, Mutability, Node, Param, ParamName, PatKind, QPath, TraitFn, TraitItem, TraitItemKind, TyKind, Unsafety, }; +use rustc_infer::infer::TyCtxtInferExt; +use rustc_infer::traits::{Obligation, ObligationCause}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, Binder, ExistentialPredicate, List, PredicateKind, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use rustc_span::sym; use rustc_span::symbol::Symbol; +use rustc_trait_selection::infer::InferCtxtExt as _; +use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; use std::fmt; use std::iter; @@ -384,6 +388,17 @@ enum DerefTy<'tcx> { Slice(Option, Ty<'tcx>), } impl<'tcx> DerefTy<'tcx> { + fn ty(&self, cx: &LateContext<'tcx>) -> Ty<'tcx> { + match *self { + Self::Str => cx.tcx.types.str_, + Self::Path => cx.tcx.mk_adt( + cx.tcx.adt_def(cx.tcx.get_diagnostic_item(sym::Path).unwrap()), + List::empty(), + ), + Self::Slice(_, ty) => cx.tcx.mk_slice(ty), + } + } + fn argless_str(&self) -> &'static str { match *self { Self::Str => "str", @@ -581,6 +596,7 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) { let i = expr_args.iter().position(|arg| arg.hir_id == child_id).unwrap_or(0); if expr_sig(self.cx, f).and_then(|sig| sig.input(i)).map_or(true, |ty| { match *ty.skip_binder().peel_refs().kind() { + ty::Dynamic(preds, _, _) => !matches_preds(self.cx, args.deref_ty.ty(self.cx), preds), ty::Param(_) => true, ty::Adt(def, _) => def.did() == args.ty_did, _ => false, @@ -614,6 +630,9 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) { }; match *self.cx.tcx.fn_sig(id).skip_binder().inputs()[i].peel_refs().kind() { + ty::Dynamic(preds, _, _) if !matches_preds(self.cx, args.deref_ty.ty(self.cx), preds) => { + set_skip_flag(); + }, ty::Param(_) => { set_skip_flag(); }, @@ -665,6 +684,31 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) { v.results } +fn matches_preds<'tcx>( + cx: &LateContext<'tcx>, + ty: Ty<'tcx>, + preds: &'tcx [Binder<'tcx, ExistentialPredicate<'tcx>>], +) -> bool { + cx.tcx.infer_ctxt().enter(|infcx| { + preds.iter().all(|&p| match cx.tcx.erase_late_bound_regions(p) { + ExistentialPredicate::Trait(p) => infcx + .type_implements_trait(p.def_id, ty, p.substs, cx.param_env) + .must_apply_modulo_regions(), + ExistentialPredicate::Projection(p) => infcx.predicate_must_hold_modulo_regions(&Obligation::new( + ObligationCause::dummy(), + cx.param_env, + cx.tcx.mk_predicate(Binder::bind_with_vars( + PredicateKind::Projection(p.with_self_ty(cx.tcx, ty)), + List::empty(), + )), + )), + ExistentialPredicate::AutoTrait(p) => infcx + .type_implements_trait(p, ty, List::empty(), cx.param_env) + .must_apply_modulo_regions(), + }) + }) +} + fn get_rptr_lm<'tcx>(ty: &'tcx hir::Ty<'tcx>) -> Option<(&'tcx Lifetime, Mutability, Span)> { if let TyKind::Rptr(lt, ref m) = ty.kind { Some((lt, m.mutbl, ty.span)) diff --git a/tests/ui/ptr_arg.rs b/tests/ui/ptr_arg.rs index fd15001e540..5f54101ca15 100644 --- a/tests/ui/ptr_arg.rs +++ b/tests/ui/ptr_arg.rs @@ -3,7 +3,7 @@ #![warn(clippy::ptr_arg)] use std::borrow::Cow; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; fn do_vec(x: &Vec) { //Nothing here @@ -207,3 +207,31 @@ fn cow_conditional_to_mut(a: &mut Cow) { a.to_mut().push_str("foo"); } } + +// Issue #9542 +fn dyn_trait_ok(a: &mut Vec, b: &mut String, c: &mut PathBuf) { + trait T {} + impl T for Vec {} + impl T for String {} + impl T for PathBuf {} + fn takes_dyn(_: &mut dyn T) {} + + takes_dyn(a); + takes_dyn(b); + takes_dyn(c); +} + +fn dyn_trait(a: &mut Vec, b: &mut String, c: &mut PathBuf) { + trait T {} + impl T for Vec {} + impl T for [U] {} + impl T for String {} + impl T for str {} + impl T for PathBuf {} + impl T for Path {} + fn takes_dyn(_: &mut dyn T) {} + + takes_dyn(a); + takes_dyn(b); + takes_dyn(c); +} diff --git a/tests/ui/ptr_arg.stderr b/tests/ui/ptr_arg.stderr index d64b5f454a5..6b4de98ce88 100644 --- a/tests/ui/ptr_arg.stderr +++ b/tests/ui/ptr_arg.stderr @@ -162,5 +162,23 @@ error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a sl LL | fn mut_vec_slice_methods(v: &mut Vec) { | ^^^^^^^^^^^^^ help: change this to: `&mut [u32]` -error: aborting due to 17 previous errors +error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do + --> $DIR/ptr_arg.rs:224:17 + | +LL | fn dyn_trait(a: &mut Vec, b: &mut String, c: &mut PathBuf) { + | ^^^^^^^^^^^^^ help: change this to: `&mut [u32]` + +error: writing `&mut String` instead of `&mut str` involves a new object where a slice will do + --> $DIR/ptr_arg.rs:224:35 + | +LL | fn dyn_trait(a: &mut Vec, b: &mut String, c: &mut PathBuf) { + | ^^^^^^^^^^^ help: change this to: `&mut str` + +error: writing `&mut PathBuf` instead of `&mut Path` involves a new object where a slice will do + --> $DIR/ptr_arg.rs:224:51 + | +LL | fn dyn_trait(a: &mut Vec, b: &mut String, c: &mut PathBuf) { + | ^^^^^^^^^^^^ help: change this to: `&mut Path` + +error: aborting due to 20 previous errors