Skip to content

Commit 97a2613

Browse files
committed
Refactor and rename some functions in ref casting lint
1 parent 07e6224 commit 97a2613

File tree

1 file changed

+54
-40
lines changed

1 file changed

+54
-40
lines changed

compiler/rustc_lint/src/reference_casting.rs

+54-40
Original file line numberDiff line numberDiff line change
@@ -37,59 +37,73 @@ declare_lint_pass!(InvalidReferenceCasting => [INVALID_REFERENCE_CASTING]);
3737

3838
impl<'tcx> LateLintPass<'tcx> for InvalidReferenceCasting {
3939
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
40-
let Some((is_assignment, e)) = is_operation_we_care_about(cx, expr) else {
41-
return;
42-
};
43-
44-
let init = cx.expr_or_init(e);
45-
46-
let Some(ty_has_interior_mutability) = is_cast_from_const_to_mut(cx, init) else {
47-
return;
48-
};
49-
let orig_cast = if init.span != e.span { Some(init.span) } else { None };
50-
let ty_has_interior_mutability = ty_has_interior_mutability.then_some(());
51-
52-
cx.emit_spanned_lint(
53-
INVALID_REFERENCE_CASTING,
54-
expr.span,
55-
if is_assignment {
56-
InvalidReferenceCastingDiag::AssignToRef { orig_cast, ty_has_interior_mutability }
57-
} else {
58-
InvalidReferenceCastingDiag::BorrowAsMut { orig_cast, ty_has_interior_mutability }
59-
},
60-
);
40+
if let Some((e, pat)) = borrow_or_assign(cx, expr) {
41+
if matches!(pat, PatternKind::Borrow { mutbl: Mutability::Mut } | PatternKind::Assign) {
42+
let init = cx.expr_or_init(e);
43+
44+
let Some(ty_has_interior_mutability) = is_cast_from_ref_to_mut_ptr(cx, init) else {
45+
return;
46+
};
47+
let orig_cast = if init.span != e.span { Some(init.span) } else { None };
48+
let ty_has_interior_mutability = ty_has_interior_mutability.then_some(());
49+
50+
cx.emit_spanned_lint(
51+
INVALID_REFERENCE_CASTING,
52+
expr.span,
53+
if pat == PatternKind::Assign {
54+
InvalidReferenceCastingDiag::AssignToRef {
55+
orig_cast,
56+
ty_has_interior_mutability,
57+
}
58+
} else {
59+
InvalidReferenceCastingDiag::BorrowAsMut {
60+
orig_cast,
61+
ty_has_interior_mutability,
62+
}
63+
},
64+
);
65+
}
66+
}
6167
}
6268
}
6369

64-
fn is_operation_we_care_about<'tcx>(
70+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
71+
enum PatternKind {
72+
Borrow { mutbl: Mutability },
73+
Assign,
74+
}
75+
76+
fn borrow_or_assign<'tcx>(
6577
cx: &LateContext<'tcx>,
6678
e: &'tcx Expr<'tcx>,
67-
) -> Option<(bool, &'tcx Expr<'tcx>)> {
68-
fn deref_assign_or_addr_of<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<(bool, &'tcx Expr<'tcx>)> {
69-
// &mut <expr>
70-
let inner = if let ExprKind::AddrOf(_, Mutability::Mut, expr) = expr.kind {
71-
expr
79+
) -> Option<(&'tcx Expr<'tcx>, PatternKind)> {
80+
fn deref_assign_or_addr_of<'tcx>(
81+
expr: &'tcx Expr<'tcx>,
82+
) -> Option<(&'tcx Expr<'tcx>, PatternKind)> {
83+
// &(mut) <expr>
84+
let (inner, pat) = if let ExprKind::AddrOf(_, mutbl, expr) = expr.kind {
85+
(expr, PatternKind::Borrow { mutbl })
7286
// <expr> = ...
7387
} else if let ExprKind::Assign(expr, _, _) = expr.kind {
74-
expr
88+
(expr, PatternKind::Assign)
7589
// <expr> += ...
7690
} else if let ExprKind::AssignOp(_, expr, _) = expr.kind {
77-
expr
91+
(expr, PatternKind::Assign)
7892
} else {
7993
return None;
8094
};
8195

82-
if let ExprKind::Unary(UnOp::Deref, e) = &inner.kind {
83-
Some((!matches!(expr.kind, ExprKind::AddrOf(..)), e))
84-
} else {
85-
None
86-
}
96+
// *<inner>
97+
let ExprKind::Unary(UnOp::Deref, e) = &inner.kind else {
98+
return None;
99+
};
100+
Some((e, pat))
87101
}
88102

89103
fn ptr_write<'tcx>(
90104
cx: &LateContext<'tcx>,
91105
e: &'tcx Expr<'tcx>,
92-
) -> Option<(bool, &'tcx Expr<'tcx>)> {
106+
) -> Option<(&'tcx Expr<'tcx>, PatternKind)> {
93107
if let ExprKind::Call(path, [arg_ptr, _arg_val]) = e.kind
94108
&& let ExprKind::Path(ref qpath) = path.kind
95109
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
@@ -98,7 +112,7 @@ fn is_operation_we_care_about<'tcx>(
98112
Some(sym::ptr_write | sym::ptr_write_volatile | sym::ptr_write_unaligned)
99113
)
100114
{
101-
Some((true, arg_ptr))
115+
Some((arg_ptr, PatternKind::Assign))
102116
} else {
103117
None
104118
}
@@ -107,7 +121,7 @@ fn is_operation_we_care_about<'tcx>(
107121
deref_assign_or_addr_of(e).or_else(|| ptr_write(cx, e))
108122
}
109123

110-
fn is_cast_from_const_to_mut<'tcx>(
124+
fn is_cast_from_ref_to_mut_ptr<'tcx>(
111125
cx: &LateContext<'tcx>,
112126
orig_expr: &'tcx Expr<'tcx>,
113127
) -> Option<bool> {
@@ -122,12 +136,12 @@ fn is_cast_from_const_to_mut<'tcx>(
122136

123137
let start_ty = cx.typeck_results().node_type(e.hir_id);
124138
if let ty::Ref(_, inner_ty, Mutability::Not) = start_ty.kind() {
125-
// If an UnsafeCell method is involved we need to additionally check the
139+
// If an UnsafeCell method is involved, we need to additionally check the
126140
// inner type for the presence of the Freeze trait (ie does NOT contain
127141
// an UnsafeCell), since in that case we would incorrectly lint on valid casts.
128142
//
129-
// We also consider non concrete skeleton types (ie generics)
130-
// to be an issue since there is no way to make it safe for abitrary types.
143+
// Except on the presence of non concrete skeleton types (ie generics)
144+
// since there is no way to make it safe for arbitrary types.
131145
let inner_ty_has_interior_mutability =
132146
!inner_ty.is_freeze(cx.tcx, cx.param_env) && inner_ty.has_concrete_skeleton();
133147
(!need_check_freeze || !inner_ty_has_interior_mutability)

0 commit comments

Comments
 (0)