Skip to content

Commit 84e03b6

Browse files
JarchoJarcho
committed
Don't lint explicit_auto_deref on dyn Trait return
1 parent 9ce0f82 commit 84e03b6

File tree

5 files changed

+119
-80
lines changed

5 files changed

+119
-80
lines changed

‎clippy_lints/src/dereference.rs‎

Lines changed: 63 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -668,14 +668,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
668668
..
669669
}) if span.ctxt() == ctxt => {
670670
let ty = cx.tcx.type_of(def_id);
671-
Some(if let ty::Ref(_, ty, _) = *ty.kind() {
672-
Position::DerefStable(
673-
precedence,
674-
ty.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
675-
)
676-
} else {
677-
Position::Other(precedence)
678-
})
671+
Some(ty_auto_deref_stability(cx, ty, precedence).position_for_result(cx))
679672
},
680673

681674
Node::Item(&Item {
@@ -699,18 +692,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
699692
let output = cx
700693
.tcx
701694
.erase_late_bound_regions(cx.tcx.fn_sig(def_id.to_def_id()).output());
702-
Some(if let ty::Ref(_, ty, _) = *output.kind() {
703-
if ty.has_placeholders() || ty.has_opaque_types() {
704-
Position::ReborrowStable(precedence)
705-
} else {
706-
Position::DerefStable(
707-
precedence,
708-
ty.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
709-
)
710-
}
711-
} else {
712-
Position::Other(precedence)
713-
})
695+
Some(ty_auto_deref_stability(cx, output, precedence).position_for_result(cx))
714696
},
715697

716698
Node::Expr(parent) if parent.span.ctxt() == ctxt => match parent.kind {
@@ -730,18 +712,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
730712
let output = cx
731713
.tcx
732714
.erase_late_bound_regions(cx.tcx.fn_sig(cx.tcx.hir().local_def_id(owner_id)).output());
733-
if let ty::Ref(_, ty, _) = *output.kind() {
734-
if ty.has_placeholders() || ty.has_opaque_types() {
735-
Position::ReborrowStable(precedence)
736-
} else {
737-
Position::DerefStable(
738-
precedence,
739-
ty.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
740-
)
741-
}
742-
} else {
743-
Position::Other(precedence)
744-
}
715+
ty_auto_deref_stability(cx, output, precedence).position_for_result(cx)
745716
},
746717
)
747718
},
@@ -757,7 +728,8 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
757728
// Type inference for closures can depend on how they're called. Only go by the explicit
758729
// types here.
759730
Some(ty) => binding_ty_auto_deref_stability(cx, ty, precedence),
760-
None => param_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence),
731+
None => ty_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence)
732+
.position_for_arg(),
761733
}),
762734
ExprKind::MethodCall(_, args, _) => {
763735
let id = cx.typeck_results().type_dependent_def_id(parent.hir_id).unwrap();
@@ -798,11 +770,12 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
798770
Position::MethodReceiver
799771
}
800772
} else {
801-
param_auto_deref_stability(
773+
ty_auto_deref_stability(
802774
cx,
803775
cx.tcx.erase_late_bound_regions(cx.tcx.fn_sig(id).input(i)),
804776
precedence,
805777
)
778+
.position_for_arg()
806779
}
807780
})
808781
},
@@ -813,7 +786,9 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
813786
.find(|f| f.expr.hir_id == child_id)
814787
.zip(variant)
815788
.and_then(|(field, variant)| variant.fields.iter().find(|f| f.name == field.ident.name))
816-
.map(|field| param_auto_deref_stability(cx, cx.tcx.type_of(field.did), precedence))
789+
.map(|field| {
790+
ty_auto_deref_stability(cx, cx.tcx.type_of(field.did), precedence).position_for_arg()
791+
})
817792
},
818793
ExprKind::Field(child, name) if child.hir_id == e.hir_id => Some(Position::FieldAccess(name.name)),
819794
ExprKind::Unary(UnOp::Deref, child) if child.hir_id == e.hir_id => Some(Position::Deref),
@@ -890,17 +865,17 @@ fn binding_ty_auto_deref_stability(cx: &LateContext<'_>, ty: &hir::Ty<'_>, prece
890865
| TyKind::Never
891866
| TyKind::Tup(_)
892867
| TyKind::Ptr(_)
893-
| TyKind::TraitObject(..)
894868
| TyKind::Path(_) => Position::DerefStable(
895869
precedence,
896870
cx
897-
.typeck_results()
898-
.node_type(ty.ty.hir_id)
899-
.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
871+
.typeck_results()
872+
.node_type(ty.ty.hir_id)
873+
.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
900874
),
901875
TyKind::OpaqueDef(..)
902876
| TyKind::Infer
903877
| TyKind::Typeof(..)
878+
| TyKind::TraitObject(..)
904879
| TyKind::Err => Position::ReborrowStable(precedence),
905880
};
906881
}
@@ -937,10 +912,39 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool {
937912
v.0
938913
}
939914

915+
struct TyPosition<'tcx> {
916+
position: Position,
917+
ty: Option<Ty<'tcx>>,
918+
}
919+
impl From<Position> for TyPosition<'_> {
920+
fn from(position: Position) -> Self {
921+
Self { position, ty: None }
922+
}
923+
}
924+
impl<'tcx> TyPosition<'tcx> {
925+
fn new_deref_stable_for_result(precedence: i8, ty: Ty<'tcx>) -> Self {
926+
Self {
927+
position: Position::ReborrowStable(precedence),
928+
ty: Some(ty),
929+
}
930+
}
931+
fn position_for_result(self, cx: &LateContext<'tcx>) -> Position {
932+
match (self.position, self.ty) {
933+
(Position::ReborrowStable(precedence), Some(ty)) => {
934+
Position::DerefStable(precedence, ty.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env))
935+
},
936+
(position, _) => position,
937+
}
938+
}
939+
fn position_for_arg(self) -> Position {
940+
self.position
941+
}
942+
}
943+
940944
// Checks whether a type is stable when switching to auto dereferencing,
941-
fn param_auto_deref_stability<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, precedence: i8) -> Position {
945+
fn ty_auto_deref_stability<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, precedence: i8) -> TyPosition<'tcx> {
942946
let ty::Ref(_, mut ty, _) = *ty.kind() else {
943-
return Position::Other(precedence);
947+
return Position::Other(precedence).into();
944948
};
945949

946950
loop {
@@ -949,38 +953,38 @@ fn param_auto_deref_stability<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, preced
949953
ty = ref_ty;
950954
continue;
951955
},
952-
ty::Infer(_)
953-
| ty::Error(_)
954-
| ty::Param(_)
955-
| ty::Bound(..)
956-
| ty::Opaque(..)
957-
| ty::Placeholder(_)
958-
| ty::Dynamic(..) => Position::ReborrowStable(precedence),
959-
ty::Adt(..) if ty.has_placeholders() || ty.has_param_types_or_consts() => {
960-
Position::ReborrowStable(precedence)
956+
ty::Param(_) => TyPosition::new_deref_stable_for_result(precedence, ty),
957+
ty::Infer(_) | ty::Error(_) | ty::Bound(..) | ty::Opaque(..) | ty::Placeholder(_) | ty::Dynamic(..) => {
958+
Position::ReborrowStable(precedence).into()
961959
},
962-
ty::Adt(..)
963-
| ty::Bool
960+
ty::Adt(..) if ty.has_placeholders() || ty.has_opaque_types() => {
961+
Position::ReborrowStable(precedence).into()
962+
},
963+
ty::Adt(_, substs) if substs.has_param_types_or_consts() => {
964+
TyPosition::new_deref_stable_for_result(precedence, ty)
965+
},
966+
ty::Bool
964967
| ty::Char
965968
| ty::Int(_)
966969
| ty::Uint(_)
967-
| ty::Float(_)
968-
| ty::Foreign(_)
969-
| ty::Str
970970
| ty::Array(..)
971-
| ty::Slice(..)
971+
| ty::Float(_)
972972
| ty::RawPtr(..)
973+
| ty::FnPtr(_) => Position::DerefStable(precedence, true).into(),
974+
ty::Str | ty::Slice(..) => Position::DerefStable(precedence, false).into(),
975+
ty::Adt(..)
976+
| ty::Foreign(_)
973977
| ty::FnDef(..)
974-
| ty::FnPtr(_)
975-
| ty::Closure(..)
976978
| ty::Generator(..)
977979
| ty::GeneratorWitness(..)
980+
| ty::Closure(..)
978981
| ty::Never
979982
| ty::Tuple(_)
980983
| ty::Projection(_) => Position::DerefStable(
981984
precedence,
982985
ty.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
983-
),
986+
)
987+
.into(),
984988
};
985989
}
986990
}

‎clippy_utils/src/ty.rs‎

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ pub fn all_predicates_of(tcx: TyCtxt<'_>, id: DefId) -> impl Iterator<Item = &(P
503503
pub enum ExprFnSig<'tcx> {
504504
Sig(Binder<'tcx, FnSig<'tcx>>, Option<DefId>),
505505
Closure(Option<&'tcx FnDecl<'tcx>>, Binder<'tcx, FnSig<'tcx>>),
506-
Trait(Binder<'tcx, Ty<'tcx>>, Option<Binder<'tcx, Ty<'tcx>>>),
506+
Trait(Binder<'tcx, Ty<'tcx>>, Option<Binder<'tcx, Ty<'tcx>>>, Option<DefId>),
507507
}
508508
impl<'tcx> ExprFnSig<'tcx> {
509509
/// Gets the argument type at the given offset. This will return `None` when the index is out of
@@ -518,7 +518,7 @@ impl<'tcx> ExprFnSig<'tcx> {
518518
}
519519
},
520520
Self::Closure(_, sig) => Some(sig.input(0).map_bound(|ty| ty.tuple_fields()[i])),
521-
Self::Trait(inputs, _) => Some(inputs.map_bound(|ty| ty.tuple_fields()[i])),
521+
Self::Trait(inputs, _, _) => Some(inputs.map_bound(|ty| ty.tuple_fields()[i])),
522522
}
523523
}
524524

@@ -541,7 +541,7 @@ impl<'tcx> ExprFnSig<'tcx> {
541541
decl.and_then(|decl| decl.inputs.get(i)),
542542
sig.input(0).map_bound(|ty| ty.tuple_fields()[i]),
543543
)),
544-
Self::Trait(inputs, _) => Some((None, inputs.map_bound(|ty| ty.tuple_fields()[i]))),
544+
Self::Trait(inputs, _, _) => Some((None, inputs.map_bound(|ty| ty.tuple_fields()[i]))),
545545
}
546546
}
547547

@@ -550,12 +550,16 @@ impl<'tcx> ExprFnSig<'tcx> {
550550
pub fn output(self) -> Option<Binder<'tcx, Ty<'tcx>>> {
551551
match self {
552552
Self::Sig(sig, _) | Self::Closure(_, sig) => Some(sig.output()),
553-
Self::Trait(_, output) => output,
553+
Self::Trait(_, output, _) => output,
554554
}
555555
}
556556

557557
pub fn predicates_id(&self) -> Option<DefId> {
558-
if let ExprFnSig::Sig(_, id) = *self { id } else { None }
558+
if let ExprFnSig::Sig(_, id) | ExprFnSig::Trait(_, _, id) = *self {
559+
id
560+
} else {
561+
None
562+
}
559563
}
560564
}
561565

@@ -580,7 +584,7 @@ fn ty_sig<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<ExprFnSig<'tcx>>
580584
Some(ExprFnSig::Closure(decl, subs.as_closure().sig()))
581585
},
582586
ty::FnDef(id, subs) => Some(ExprFnSig::Sig(cx.tcx.bound_fn_sig(id).subst(cx.tcx, subs), Some(id))),
583-
ty::Opaque(id, _) => ty_sig(cx, cx.tcx.type_of(id)),
587+
ty::Opaque(id, _) => sig_from_bounds(cx, ty, cx.tcx.item_bounds(id), cx.tcx.opt_parent(id)),
584588
ty::FnPtr(sig) => Some(ExprFnSig::Sig(sig, None)),
585589
ty::Dynamic(bounds, _) => {
586590
let lang_items = cx.tcx.lang_items();
@@ -594,38 +598,44 @@ fn ty_sig<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<ExprFnSig<'tcx>>
594598
.projection_bounds()
595599
.find(|p| lang_items.fn_once_output().map_or(false, |id| id == p.item_def_id()))
596600
.map(|p| p.map_bound(|p| p.term.ty().unwrap()));
597-
Some(ExprFnSig::Trait(bound.map_bound(|b| b.substs.type_at(0)), output))
601+
Some(ExprFnSig::Trait(bound.map_bound(|b| b.substs.type_at(0)), output, None))
598602
},
599603
_ => None,
600604
}
601605
},
602606
ty::Projection(proj) => match cx.tcx.try_normalize_erasing_regions(cx.param_env, ty) {
603607
Ok(normalized_ty) if normalized_ty != ty => ty_sig(cx, normalized_ty),
604-
_ => sig_for_projection(cx, proj).or_else(|| sig_from_bounds(cx, ty)),
608+
_ => sig_for_projection(cx, proj).or_else(|| sig_from_bounds(cx, ty, cx.param_env.caller_bounds(), None)),
605609
},
606-
ty::Param(_) => sig_from_bounds(cx, ty),
610+
ty::Param(_) => sig_from_bounds(cx, ty, cx.param_env.caller_bounds(), None),
607611
_ => None,
608612
}
609613
}
610614

611-
fn sig_from_bounds<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<ExprFnSig<'tcx>> {
615+
fn sig_from_bounds<'tcx>(
616+
cx: &LateContext<'tcx>,
617+
ty: Ty<'tcx>,
618+
predicates: &'tcx [Predicate<'tcx>],
619+
predicates_id: Option<DefId>,
620+
) -> Option<ExprFnSig<'tcx>> {
612621
let mut inputs = None;
613622
let mut output = None;
614623
let lang_items = cx.tcx.lang_items();
615624

616-
for (pred, _) in all_predicates_of(cx.tcx, cx.typeck_results().hir_owner.to_def_id()) {
625+
for pred in predicates {
617626
match pred.kind().skip_binder() {
618627
PredicateKind::Trait(p)
619628
if (lang_items.fn_trait() == Some(p.def_id())
620629
|| lang_items.fn_mut_trait() == Some(p.def_id())
621630
|| lang_items.fn_once_trait() == Some(p.def_id()))
622631
&& p.self_ty() == ty =>
623632
{
624-
if inputs.is_some() {
633+
let i = pred.kind().rebind(p.trait_ref.substs.type_at(1));
634+
if inputs.map_or(false, |inputs| i != inputs) {
625635
// Multiple different fn trait impls. Is this even allowed?
626636
return None;
627637
}
628-
inputs = Some(pred.kind().rebind(p.trait_ref.substs.type_at(1)));
638+
inputs = Some(i);
629639
},
630640
PredicateKind::Projection(p)
631641
if Some(p.projection_ty.item_def_id) == lang_items.fn_once_output()
@@ -641,7 +651,7 @@ fn sig_from_bounds<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<ExprFnS
641651
}
642652
}
643653

644-
inputs.map(|ty| ExprFnSig::Trait(ty, output))
654+
inputs.map(|ty| ExprFnSig::Trait(ty, output, predicates_id))
645655
}
646656

647657
fn sig_for_projection<'tcx>(cx: &LateContext<'tcx>, ty: ProjectionTy<'tcx>) -> Option<ExprFnSig<'tcx>> {
@@ -661,14 +671,15 @@ fn sig_for_projection<'tcx>(cx: &LateContext<'tcx>, ty: ProjectionTy<'tcx>) -> O
661671
|| lang_items.fn_mut_trait() == Some(p.def_id())
662672
|| lang_items.fn_once_trait() == Some(p.def_id())) =>
663673
{
664-
if inputs.is_some() {
674+
let i = pred
675+
.map_bound(|pred| pred.kind().rebind(p.trait_ref.substs.type_at(1)))
676+
.subst(cx.tcx, ty.substs);
677+
678+
if inputs.map_or(false, |inputs| inputs != i) {
665679
// Multiple different fn trait impls. Is this even allowed?
666680
return None;
667681
}
668-
inputs = Some(
669-
pred.map_bound(|pred| pred.kind().rebind(p.trait_ref.substs.type_at(1)))
670-
.subst(cx.tcx, ty.substs),
671-
);
682+
inputs = Some(i);
672683
},
673684
PredicateKind::Projection(p) if Some(p.projection_ty.item_def_id) == lang_items.fn_once_output() => {
674685
if output.is_some() {
@@ -684,7 +695,7 @@ fn sig_for_projection<'tcx>(cx: &LateContext<'tcx>, ty: ProjectionTy<'tcx>) -> O
684695
}
685696
}
686697

687-
inputs.map(|ty| ExprFnSig::Trait(ty, output))
698+
inputs.map(|ty| ExprFnSig::Trait(ty, output, None))
688699
}
689700

690701
#[derive(Clone, Copy)]

‎tests/ui/explicit_auto_deref.fixed‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,4 +233,13 @@ fn main() {
233233
_ => panic!(),
234234
};
235235
let _: &X = &*if true { Y(X) } else { panic!() };
236+
237+
fn deref_to_u<U, T: core::ops::Deref<Target = U>>(x: &T) -> &U {
238+
x
239+
}
240+
241+
let _ = |x: &'static Box<dyn Iterator<Item = u32>>| -> &'static dyn Iterator<Item = u32> { &**x };
242+
fn ret_any(x: &Box<dyn std::any::Any>) -> &dyn std::any::Any {
243+
&**x
244+
}
236245
}

‎tests/ui/explicit_auto_deref.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,4 +233,13 @@ fn main() {
233233
_ => panic!(),
234234
};
235235
let _: &X = &*if true { Y(X) } else { panic!() };
236+
237+
fn deref_to_u<U, T: core::ops::Deref<Target = U>>(x: &T) -> &U {
238+
&**x
239+
}
240+
241+
let _ = |x: &'static Box<dyn Iterator<Item = u32>>| -> &'static dyn Iterator<Item = u32> { &**x };
242+
fn ret_any(x: &Box<dyn std::any::Any>) -> &dyn std::any::Any {
243+
&**x
244+
}
236245
}

‎tests/ui/explicit_auto_deref.stderr‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,5 +210,11 @@ error: deref which would be done by auto-deref
210210
LL | let _ = || -> &'static str { return *s };
211211
| ^^ help: try this: `s`
212212

213-
error: aborting due to 35 previous errors
213+
error: deref which would be done by auto-deref
214+
--> $DIR/explicit_auto_deref.rs:238:9
215+
|
216+
LL | &**x
217+
| ^^^^ help: try this: `x`
218+
219+
error: aborting due to 36 previous errors
214220

0 commit comments

Comments
 (0)