Skip to content

Commit c990e29

Browse files
JarchoJarcho
committed
Don't suggest using auto deref for block expressions
1 parent 3a4e457 commit c990e29

File tree

4 files changed

+167
-66
lines changed

4 files changed

+167
-66
lines changed

‎clippy_lints/src/dereference.rs‎

Lines changed: 94 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use rustc_lint::{LateContext, LateLintPass};
1717
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
1818
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitable, TypeckResults};
1919
use rustc_session::{declare_tool_lint, impl_lint_pass};
20-
use rustc_span::{symbol::sym, Span, Symbol};
20+
use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
2121
use rustc_trait_selection::infer::InferCtxtExt;
2222

2323
declare_clippy_lint! {
@@ -609,26 +609,29 @@ enum Position {
609609
Postfix,
610610
Deref,
611611
/// Any other location which will trigger auto-deref to a specific time.
612-
DerefStable(i8),
612+
/// Contains the precedence of the parent expression and whether the target type is sized.
613+
DerefStable(i8, bool),
613614
/// Any other location which will trigger auto-reborrowing.
615+
/// Contains the precedence of the parent expression.
614616
ReborrowStable(i8),
617+
/// Contains the precedence of the parent expression.
615618
Other(i8),
616619
}
617620
impl Position {
618621
fn is_deref_stable(self) -> bool {
619-
matches!(self, Self::DerefStable(_))
622+
matches!(self, Self::DerefStable(..))
620623
}
621624

622625
fn is_reborrow_stable(self) -> bool {
623-
matches!(self, Self::DerefStable(_) | Self::ReborrowStable(_))
626+
matches!(self, Self::DerefStable(..) | Self::ReborrowStable(_))
624627
}
625628

626629
fn can_auto_borrow(self) -> bool {
627630
matches!(self, Self::MethodReceiver | Self::FieldAccess(_) | Self::Callee)
628631
}
629632

630633
fn lint_explicit_deref(self) -> bool {
631-
matches!(self, Self::Other(_) | Self::DerefStable(_) | Self::ReborrowStable(_))
634+
matches!(self, Self::Other(_) | Self::DerefStable(..) | Self::ReborrowStable(_))
632635
}
633636

634637
fn precedence(self) -> i8 {
@@ -639,7 +642,7 @@ impl Position {
639642
| Self::FieldAccess(_)
640643
| Self::Postfix => PREC_POSTFIX,
641644
Self::Deref => PREC_PREFIX,
642-
Self::DerefStable(p) | Self::ReborrowStable(p) | Self::Other(p) => p,
645+
Self::DerefStable(p, _) | Self::ReborrowStable(p) | Self::Other(p) => p,
643646
}
644647
}
645648
}
@@ -659,7 +662,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
659662
}
660663
match parent {
661664
Node::Local(Local { ty: Some(ty), span, .. }) if span.ctxt() == ctxt => {
662-
Some(binding_ty_auto_deref_stability(ty, precedence))
665+
Some(binding_ty_auto_deref_stability(cx, ty, precedence))
663666
},
664667
Node::Item(&Item {
665668
kind: ItemKind::Static(..) | ItemKind::Const(..),
@@ -680,8 +683,11 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
680683
..
681684
}) if span.ctxt() == ctxt => {
682685
let ty = cx.tcx.type_of(def_id);
683-
Some(if ty.is_ref() {
684-
Position::DerefStable(precedence)
686+
Some(if let ty::Ref(_, ty, _) = *ty.kind() {
687+
Position::DerefStable(
688+
precedence,
689+
ty.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
690+
)
685691
} else {
686692
Position::Other(precedence)
687693
})
@@ -705,13 +711,20 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
705711
span,
706712
..
707713
}) if span.ctxt() == ctxt => {
708-
let output = cx.tcx.fn_sig(def_id.to_def_id()).skip_binder().output();
709-
Some(if !output.is_ref() {
710-
Position::Other(precedence)
711-
} else if output.has_placeholders() || output.has_opaque_types() {
712-
Position::ReborrowStable(precedence)
714+
let output = cx
715+
.tcx
716+
.erase_late_bound_regions(cx.tcx.fn_sig(def_id.to_def_id()).output());
717+
Some(if let ty::Ref(_, ty, _) = *output.kind() {
718+
if ty.has_placeholders() || ty.has_opaque_types() {
719+
Position::ReborrowStable(precedence)
720+
} else {
721+
Position::DerefStable(
722+
precedence,
723+
ty.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
724+
)
725+
}
713726
} else {
714-
Position::DerefStable(precedence)
727+
Position::Other(precedence)
715728
})
716729
},
717730

@@ -725,21 +738,24 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
725738
}) = cx.tcx.hir().get(owner_id)
726739
{
727740
match fn_decl.output {
728-
FnRetTy::Return(ty) => binding_ty_auto_deref_stability(ty, precedence),
741+
FnRetTy::Return(ty) => binding_ty_auto_deref_stability(cx, ty, precedence),
729742
FnRetTy::DefaultReturn(_) => Position::Other(precedence),
730743
}
731744
} else {
732745
let output = cx
733746
.tcx
734-
.fn_sig(cx.tcx.hir().local_def_id(owner_id))
735-
.skip_binder()
736-
.output();
737-
if !output.is_ref() {
738-
Position::Other(precedence)
739-
} else if output.has_placeholders() || output.has_opaque_types() {
740-
Position::ReborrowStable(precedence)
747+
.erase_late_bound_regions(cx.tcx.fn_sig(cx.tcx.hir().local_def_id(owner_id)).output());
748+
if let ty::Ref(_, ty, _) = *output.kind() {
749+
if ty.has_placeholders() || ty.has_opaque_types() {
750+
Position::ReborrowStable(precedence)
751+
} else {
752+
Position::DerefStable(
753+
precedence,
754+
ty.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
755+
)
756+
}
741757
} else {
742-
Position::DerefStable(precedence)
758+
Position::Other(precedence)
743759
}
744760
},
745761
)
@@ -755,8 +771,8 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
755771
.map(|(hir_ty, ty)| match hir_ty {
756772
// Type inference for closures can depend on how they're called. Only go by the explicit
757773
// types here.
758-
Some(ty) => binding_ty_auto_deref_stability(ty, precedence),
759-
None => param_auto_deref_stability(ty.skip_binder(), precedence),
774+
Some(ty) => binding_ty_auto_deref_stability(cx, ty, precedence),
775+
None => param_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence),
760776
}),
761777
ExprKind::MethodCall(_, args, _) => {
762778
let id = cx.typeck_results().type_dependent_def_id(parent.hir_id).unwrap();
@@ -797,7 +813,11 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
797813
Position::MethodReceiver
798814
}
799815
} else {
800-
param_auto_deref_stability(cx.tcx.fn_sig(id).skip_binder().inputs()[i], precedence)
816+
param_auto_deref_stability(
817+
cx,
818+
cx.tcx.erase_late_bound_regions(cx.tcx.fn_sig(id).input(i)),
819+
precedence,
820+
)
801821
}
802822
})
803823
},
@@ -808,7 +828,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
808828
.find(|f| f.expr.hir_id == child_id)
809829
.zip(variant)
810830
.and_then(|(field, variant)| variant.fields.iter().find(|f| f.name == field.ident.name))
811-
.map(|field| param_auto_deref_stability(cx.tcx.type_of(field.did), precedence))
831+
.map(|field| param_auto_deref_stability(cx, cx.tcx.type_of(field.did), precedence))
812832
},
813833
ExprKind::Field(child, name) if child.hir_id == e.hir_id => Some(Position::FieldAccess(name.name)),
814834
ExprKind::Unary(UnOp::Deref, child) if child.hir_id == e.hir_id => Some(Position::Deref),
@@ -840,7 +860,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
840860
//
841861
// Here `y1` and `y2` would resolve to different types, so the type `&Box<_>` is not stable when
842862
// switching to auto-dereferencing.
843-
fn binding_ty_auto_deref_stability(ty: &hir::Ty<'_>, precedence: i8) -> Position {
863+
fn binding_ty_auto_deref_stability(cx: &LateContext<'_>, ty: &hir::Ty<'_>, precedence: i8) -> Position {
844864
let TyKind::Rptr(_, ty) = &ty.kind else {
845865
return Position::Other(precedence);
846866
};
@@ -870,7 +890,13 @@ fn binding_ty_auto_deref_stability(ty: &hir::Ty<'_>, precedence: i8) -> Position
870890
{
871891
Position::ReborrowStable(precedence)
872892
} else {
873-
Position::DerefStable(precedence)
893+
Position::DerefStable(
894+
precedence,
895+
cx
896+
.typeck_results()
897+
.node_type(ty.ty.hir_id)
898+
.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
899+
)
874900
}
875901
},
876902
TyKind::Slice(_)
@@ -880,7 +906,13 @@ fn binding_ty_auto_deref_stability(ty: &hir::Ty<'_>, precedence: i8) -> Position
880906
| TyKind::Tup(_)
881907
| TyKind::Ptr(_)
882908
| TyKind::TraitObject(..)
883-
| TyKind::Path(_) => Position::DerefStable(precedence),
909+
| TyKind::Path(_) => Position::DerefStable(
910+
precedence,
911+
cx
912+
.typeck_results()
913+
.node_type(ty.ty.hir_id)
914+
.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
915+
),
884916
TyKind::OpaqueDef(..)
885917
| TyKind::Infer
886918
| TyKind::Typeof(..)
@@ -921,7 +953,7 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool {
921953
}
922954

923955
// Checks whether a type is stable when switching to auto dereferencing,
924-
fn param_auto_deref_stability(ty: Ty<'_>, precedence: i8) -> Position {
956+
fn param_auto_deref_stability<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, precedence: i8) -> Position {
925957
let ty::Ref(_, mut ty, _) = *ty.kind() else {
926958
return Position::Other(precedence);
927959
};
@@ -960,7 +992,10 @@ fn param_auto_deref_stability(ty: Ty<'_>, precedence: i8) -> Position {
960992
| ty::GeneratorWitness(..)
961993
| ty::Never
962994
| ty::Tuple(_)
963-
| ty::Projection(_) => Position::DerefStable(precedence),
995+
| ty::Projection(_) => Position::DerefStable(
996+
precedence,
997+
ty.is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()),
998+
),
964999
};
9651000
}
9661001
}
@@ -1040,6 +1075,19 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
10401075
});
10411076
},
10421077
State::ExplicitDeref { deref_span_id } => {
1078+
if matches!(
1079+
expr.kind,
1080+
ExprKind::Block(..)
1081+
| ExprKind::ConstBlock(_)
1082+
| ExprKind::If(..)
1083+
| ExprKind::Loop(..)
1084+
| ExprKind::Match(..)
1085+
) && matches!(data.position, Position::DerefStable(_, true))
1086+
{
1087+
// Rustc bug: auto deref doesn't work on block expression when targeting sized types.
1088+
return;
1089+
}
1090+
10431091
let (span, hir_id, precedence) = if let Some((span, hir_id)) = deref_span_id
10441092
&& !cx.typeck_results().expr_ty(expr).is_ref()
10451093
{
@@ -1067,6 +1115,19 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
10671115
);
10681116
},
10691117
State::ExplicitDerefField { .. } => {
1118+
if matches!(
1119+
expr.kind,
1120+
ExprKind::Block(..)
1121+
| ExprKind::ConstBlock(_)
1122+
| ExprKind::If(..)
1123+
| ExprKind::Loop(..)
1124+
| ExprKind::Match(..)
1125+
) && matches!(data.position, Position::DerefStable(_, true))
1126+
{
1127+
// Rustc bug: auto deref doesn't work on block expression when targeting sized types.
1128+
return;
1129+
}
1130+
10701131
span_lint_hir_and_then(
10711132
cx,
10721133
EXPLICIT_AUTO_DEREF,

‎tests/ui/explicit_auto_deref.fixed‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ fn main() {
6767
let s = String::new();
6868

6969
let _: &str = &s;
70+
let _: &str = &{ String::new() };
7071
let _ = &*s; // Don't lint. Inferred type would change.
7172
let _: &_ = &*s; // Don't lint. Inferred type would change.
7273

@@ -215,4 +216,20 @@ fn main() {
215216
let s = &"str";
216217
let _ = || return *s;
217218
let _ = || -> &'static str { return s };
219+
220+
struct X;
221+
struct Y(X);
222+
impl core::ops::Deref for Y {
223+
type Target = X;
224+
fn deref(&self) -> &Self::Target {
225+
&self.0
226+
}
227+
}
228+
let _: &X = &*{ Y(X) };
229+
let _: &X = &*match 0 {
230+
#[rustfmt::skip]
231+
0 => { Y(X) },
232+
_ => panic!(),
233+
};
234+
let _: &X = &*if true { Y(X) } else { panic!() };
218235
}

‎tests/ui/explicit_auto_deref.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ fn main() {
6767
let s = String::new();
6868

6969
let _: &str = &*s;
70+
let _: &str = &*{ String::new() };
7071
let _ = &*s; // Don't lint. Inferred type would change.
7172
let _: &_ = &*s; // Don't lint. Inferred type would change.
7273

@@ -215,4 +216,20 @@ fn main() {
215216
let s = &"str";
216217
let _ = || return *s;
217218
let _ = || -> &'static str { return *s };
219+
220+
struct X;
221+
struct Y(X);
222+
impl core::ops::Deref for Y {
223+
type Target = X;
224+
fn deref(&self) -> &Self::Target {
225+
&self.0
226+
}
227+
}
228+
let _: &X = &*{ Y(X) };
229+
let _: &X = &*match 0 {
230+
#[rustfmt::skip]
231+
0 => { Y(X) },
232+
_ => panic!(),
233+
};
234+
let _: &X = &*if true { Y(X) } else { panic!() };
218235
}

0 commit comments

Comments
 (0)