Do not suggest to use implicit DerefMut on ManuallyDrop reached through unions (#14387)

This requires making the `deref_addrof` lint a late lint instead of an
early lint to check for types.

changelog: [`deref_addrof`]: do not suggest implicit `DerefMut` on
`ManuallyDrop` union fields

Fix rust-lang/rust-clippy#14386
This commit is contained in:
Alejandra González
2025-08-18 15:10:31 +00:00
committed by GitHub
7 changed files with 289 additions and 119 deletions
+1 -10
View File
@@ -1,12 +1,11 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
use clippy_utils::sugg::has_enclosing_paren;
use clippy_utils::ty::{implements_trait, is_manually_drop};
use clippy_utils::ty::{adjust_derefs_manually_drop, implements_trait, is_manually_drop};
use clippy_utils::{
DefinedTy, ExprUseNode, expr_use_ctxt, get_parent_expr, is_block_like, is_lint_allowed, path_to_local,
peel_middle_ty_refs,
};
use core::mem;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::Applicability;
@@ -707,14 +706,6 @@ fn try_parse_ref_op<'tcx>(
))
}
// Checks if the adjustments contains a deref of `ManuallyDrop<_>`
fn adjust_derefs_manually_drop<'tcx>(adjustments: &'tcx [Adjustment<'tcx>], mut ty: Ty<'tcx>) -> bool {
adjustments.iter().any(|a| {
let ty = mem::replace(&mut ty, a.target);
matches!(a.kind, Adjust::Deref(Some(ref op)) if op.mutbl == Mutability::Mut) && is_manually_drop(ty)
})
}
// Checks whether the type for a deref call actually changed the type, not just the mutability of
// the reference.
fn deref_method_same_type<'tcx>(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool {
+1 -1
View File
@@ -600,7 +600,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
store.register_late_pass(move |_| Box::new(trait_bounds::TraitBounds::new(conf)));
store.register_late_pass(|_| Box::new(comparison_chain::ComparisonChain));
store.register_late_pass(move |tcx| Box::new(mut_key::MutableKeyType::new(tcx, conf)));
store.register_early_pass(|| Box::new(reference::DerefAddrOf));
store.register_late_pass(|_| Box::new(reference::DerefAddrOf));
store.register_early_pass(|| Box::new(double_parens::DoubleParens));
let format_args = format_args_storage.clone();
store.register_late_pass(move |_| Box::new(format_impl::FormatImpl::new(format_args.clone())));
+85 -64
View File
@@ -1,10 +1,11 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{SpanRangeExt, snippet_with_applicability};
use rustc_ast::ast::{Expr, ExprKind, Mutability, UnOp};
use clippy_utils::source::snippet;
use clippy_utils::sugg::{Sugg, has_enclosing_paren};
use clippy_utils::ty::adjust_derefs_manually_drop;
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_hir::{Expr, ExprKind, HirId, Node, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::{BytePos, Span};
declare_clippy_lint! {
/// ### What it does
@@ -37,17 +38,12 @@
declare_lint_pass!(DerefAddrOf => [DEREF_ADDROF]);
fn without_parens(mut e: &Expr) -> &Expr {
while let ExprKind::Paren(ref child_e) = e.kind {
e = child_e;
}
e
}
impl EarlyLintPass for DerefAddrOf {
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &Expr) {
if let ExprKind::Unary(UnOp::Deref, ref deref_target) = e.kind
&& let ExprKind::AddrOf(_, ref mutability, ref addrof_target) = without_parens(deref_target).kind
impl LateLintPass<'_> for DerefAddrOf {
fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
if !e.span.from_expansion()
&& let ExprKind::Unary(UnOp::Deref, deref_target) = e.kind
&& !deref_target.span.from_expansion()
&& let ExprKind::AddrOf(_, _, addrof_target) = deref_target.kind
// NOTE(tesuji): `*&` forces rustc to const-promote the array to `.rodata` section.
// See #12854 for details.
&& !matches!(addrof_target.kind, ExprKind::Array(_))
@@ -55,57 +51,82 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &Expr) {
&& !addrof_target.span.from_expansion()
{
let mut applicability = Applicability::MachineApplicable;
let sugg = if e.span.from_expansion() {
if let Some(macro_source) = e.span.get_source_text(cx) {
// Remove leading whitespace from the given span
// e.g: ` $visitor` turns into `$visitor`
let trim_leading_whitespaces = |span: Span| {
span.get_source_text(cx)
.and_then(|snip| {
#[expect(clippy::cast_possible_truncation)]
snip.find(|c: char| !c.is_whitespace())
.map(|pos| span.lo() + BytePos(pos as u32))
})
.map_or(span, |start_no_whitespace| e.span.with_lo(start_no_whitespace))
};
let mut sugg = || Sugg::hir_with_applicability(cx, addrof_target, "_", &mut applicability);
let mut generate_snippet = |pattern: &str| {
#[expect(clippy::cast_possible_truncation)]
macro_source.rfind(pattern).map(|pattern_pos| {
let rpos = pattern_pos + pattern.len();
let span_after_ref = e.span.with_lo(BytePos(e.span.lo().0 + rpos as u32));
let span = trim_leading_whitespaces(span_after_ref);
snippet_with_applicability(cx, span, "_", &mut applicability)
})
};
if *mutability == Mutability::Mut {
generate_snippet("mut")
} else {
generate_snippet("&")
}
} else {
Some(snippet_with_applicability(cx, e.span, "_", &mut applicability))
}
} else {
Some(snippet_with_applicability(
cx,
addrof_target.span,
"_",
&mut applicability,
))
// If this expression is an explicit `DerefMut` of a `ManuallyDrop` reached through a
// union, we may remove the reference if we are at the point where the implicit
// dereference would take place. Otherwise, we should not lint.
let sugg = match is_manually_drop_through_union(cx, e.hir_id, addrof_target) {
ManuallyDropThroughUnion::Directly => sugg().deref(),
ManuallyDropThroughUnion::Indirect => return,
ManuallyDropThroughUnion::No => sugg(),
};
if let Some(sugg) = sugg {
span_lint_and_sugg(
cx,
DEREF_ADDROF,
e.span,
"immediately dereferencing a reference",
"try",
sugg.to_string(),
applicability,
);
}
let sugg = if has_enclosing_paren(snippet(cx, e.span, "")) {
sugg.maybe_paren()
} else {
sugg
};
span_lint_and_sugg(
cx,
DEREF_ADDROF,
e.span,
"immediately dereferencing a reference",
"try",
sugg.to_string(),
applicability,
);
}
}
}
/// Is this a `ManuallyDrop` reached through a union, and when is `DerefMut` called on it?
enum ManuallyDropThroughUnion {
/// `ManuallyDrop` reached through a union and immediately explicitely dereferenced
Directly,
/// `ManuallyDrop` reached through a union, and dereferenced later on
Indirect,
/// Any other situation
No,
}
/// Check if `addrof_target` is part of an access to a `ManuallyDrop` entity reached through a
/// union, and when it is dereferenced using `DerefMut` starting from `expr_id` and going up.
fn is_manually_drop_through_union(
cx: &LateContext<'_>,
expr_id: HirId,
addrof_target: &Expr<'_>,
) -> ManuallyDropThroughUnion {
if is_reached_through_union(cx, addrof_target) {
let typeck = cx.typeck_results();
for (idx, id) in std::iter::once(expr_id)
.chain(cx.tcx.hir_parent_id_iter(expr_id))
.enumerate()
{
if let Node::Expr(expr) = cx.tcx.hir_node(id) {
if adjust_derefs_manually_drop(typeck.expr_adjustments(expr), typeck.expr_ty(expr)) {
return if idx == 0 {
ManuallyDropThroughUnion::Directly
} else {
ManuallyDropThroughUnion::Indirect
};
}
} else {
break;
}
}
}
ManuallyDropThroughUnion::No
}
/// Checks whether `expr` denotes an object reached through a union
fn is_reached_through_union(cx: &LateContext<'_>, mut expr: &Expr<'_>) -> bool {
while let ExprKind::Field(parent, _) | ExprKind::Index(parent, _, _) = expr.kind {
if cx.typeck_results().expr_ty_adjusted(parent).is_union() {
return true;
}
expr = parent;
}
false
}
+10 -2
View File
@@ -18,6 +18,7 @@
use rustc_middle::mir::ConstValue;
use rustc_middle::mir::interpret::Scalar;
use rustc_middle::traits::EvaluationResult;
use rustc_middle::ty::adjustment::{Adjust, Adjustment};
use rustc_middle::ty::layout::ValidityRequirement;
use rustc_middle::ty::{
self, AdtDef, AliasTy, AssocItem, AssocTag, Binder, BoundRegion, FnSig, GenericArg, GenericArgKind, GenericArgsRef,
@@ -31,7 +32,7 @@
use rustc_trait_selection::traits::{Obligation, ObligationCause};
use std::assert_matches::debug_assert_matches;
use std::collections::hash_map::Entry;
use std::iter;
use std::{iter, mem};
use crate::path_res;
use crate::paths::{PathNS, lookup_path_str};
@@ -1382,7 +1383,6 @@ pub fn is_slice_like<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
|| matches!(ty.kind(), ty::Adt(adt_def, _) if cx.tcx.is_diagnostic_item(sym::Vec, adt_def.did()))
}
/// Gets the index of a field by name.
pub fn get_field_idx_by_name(ty: Ty<'_>, name: Symbol) -> Option<usize> {
match *ty.kind() {
ty::Adt(def, _) if def.is_union() || def.is_struct() => {
@@ -1392,3 +1392,11 @@ pub fn get_field_idx_by_name(ty: Ty<'_>, name: Symbol) -> Option<usize> {
_ => None,
}
}
/// Checks if the adjustments contain a mutable dereference of a `ManuallyDrop<_>`.
pub fn adjust_derefs_manually_drop<'tcx>(adjustments: &'tcx [Adjustment<'tcx>], mut ty: Ty<'tcx>) -> bool {
adjustments.iter().any(|a| {
let ty = mem::replace(&mut ty, a.target);
matches!(a.kind, Adjust::Deref(Some(op)) if op.mutbl == Mutability::Mut) && is_manually_drop(ty)
})
}
+72 -16
View File
@@ -1,11 +1,11 @@
//@aux-build:proc_macros.rs
#![allow(clippy::return_self_not_must_use, clippy::useless_vec)]
#![allow(
dangerous_implicit_autorefs,
clippy::explicit_auto_deref,
clippy::return_self_not_must_use,
clippy::useless_vec
)]
#![warn(clippy::deref_addrof)]
extern crate proc_macros;
use proc_macros::inline_macros;
fn get_number() -> usize {
10
}
@@ -56,19 +56,75 @@ fn main() {
//~^ deref_addrof
// do NOT lint for array as semantic differences with/out `*&`.
let _arr = *&[0, 1, 2, 3, 4];
// Do not lint when text comes from macro
macro_rules! mac {
(dr) => {
*&0
};
(dr $e:expr) => {
*&$e
};
(r $e:expr) => {
&$e
};
}
let b = mac!(dr);
let b = mac!(dr a);
let b = *mac!(r a);
}
#[derive(Copy, Clone)]
pub struct S;
#[inline_macros]
impl S {
pub fn f(&self) -> &Self {
inline!($(@expr self))
//~^ deref_addrof
fn issue14386() {
use std::mem::ManuallyDrop;
#[derive(Copy, Clone)]
struct Data {
num: u64,
}
#[allow(unused_mut)] // mut will be unused, once the macro is fixed
pub fn f_mut(mut self) -> Self {
inline!($(@expr self))
#[derive(Clone, Copy)]
struct M {
md: ManuallyDrop<[u8; 4]>,
}
union DataWithPadding<'lt> {
data: ManuallyDrop<Data>,
prim: ManuallyDrop<u64>,
padding: [u8; size_of::<Data>()],
tup: (ManuallyDrop<Data>, ()),
indirect: M,
indirect_arr: [M; 2],
indirect_ref: &'lt mut M,
}
let mut a = DataWithPadding {
padding: [0; size_of::<DataWithPadding>()],
};
unsafe {
a.padding = [1; size_of::<DataWithPadding>()];
//~^ deref_addrof
a.tup.1 = ();
//~^ deref_addrof
*a.prim = 0;
//~^ deref_addrof
(*a.data).num = 42;
//~^ deref_addrof
(*a.indirect.md)[3] = 1;
//~^ deref_addrof
(*a.indirect_arr[1].md)[3] = 1;
//~^ deref_addrof
(*a.indirect_ref.md)[3] = 1;
//~^ deref_addrof
// Check that raw pointers are properly considered as well
*a.prim = 0;
//~^ deref_addrof
(*a.data).num = 42;
//~^ deref_addrof
// Do not lint, as the dereference happens later, we cannot
// just remove `&mut`
(*&mut a.tup).0.num = 42;
}
}
+72 -16
View File
@@ -1,11 +1,11 @@
//@aux-build:proc_macros.rs
#![allow(clippy::return_self_not_must_use, clippy::useless_vec)]
#![allow(
dangerous_implicit_autorefs,
clippy::explicit_auto_deref,
clippy::return_self_not_must_use,
clippy::useless_vec
)]
#![warn(clippy::deref_addrof)]
extern crate proc_macros;
use proc_macros::inline_macros;
fn get_number() -> usize {
10
}
@@ -56,19 +56,75 @@ fn main() {
//~^ deref_addrof
// do NOT lint for array as semantic differences with/out `*&`.
let _arr = *&[0, 1, 2, 3, 4];
// Do not lint when text comes from macro
macro_rules! mac {
(dr) => {
*&0
};
(dr $e:expr) => {
*&$e
};
(r $e:expr) => {
&$e
};
}
let b = mac!(dr);
let b = mac!(dr a);
let b = *mac!(r a);
}
#[derive(Copy, Clone)]
pub struct S;
#[inline_macros]
impl S {
pub fn f(&self) -> &Self {
inline!(*& $(@expr self))
//~^ deref_addrof
fn issue14386() {
use std::mem::ManuallyDrop;
#[derive(Copy, Clone)]
struct Data {
num: u64,
}
#[allow(unused_mut)] // mut will be unused, once the macro is fixed
pub fn f_mut(mut self) -> Self {
inline!(*&mut $(@expr self))
#[derive(Clone, Copy)]
struct M {
md: ManuallyDrop<[u8; 4]>,
}
union DataWithPadding<'lt> {
data: ManuallyDrop<Data>,
prim: ManuallyDrop<u64>,
padding: [u8; size_of::<Data>()],
tup: (ManuallyDrop<Data>, ()),
indirect: M,
indirect_arr: [M; 2],
indirect_ref: &'lt mut M,
}
let mut a = DataWithPadding {
padding: [0; size_of::<DataWithPadding>()],
};
unsafe {
(*&mut a.padding) = [1; size_of::<DataWithPadding>()];
//~^ deref_addrof
(*&mut a.tup).1 = ();
//~^ deref_addrof
**&mut a.prim = 0;
//~^ deref_addrof
(*&mut a.data).num = 42;
//~^ deref_addrof
(*&mut a.indirect.md)[3] = 1;
//~^ deref_addrof
(*&mut a.indirect_arr[1].md)[3] = 1;
//~^ deref_addrof
(*&mut a.indirect_ref.md)[3] = 1;
//~^ deref_addrof
// Check that raw pointers are properly considered as well
**&raw mut a.prim = 0;
//~^ deref_addrof
(*&raw mut a.data).num = 42;
//~^ deref_addrof
// Do not lint, as the dereference happens later, we cannot
// just remove `&mut`
(*&mut a.tup).0.num = 42;
}
}
+48 -10
View File
@@ -56,20 +56,58 @@ LL | let _repeat = *&[0; 64];
| ^^^^^^^^^ help: try: `[0; 64]`
error: immediately dereferencing a reference
--> tests/ui/deref_addrof.rs:66:17
--> tests/ui/deref_addrof.rs:104:9
|
LL | inline!(*& $(@expr self))
| ^^^^^^^^^^^^^^^^ help: try: `$(@expr self)`
|
= note: this error originates in the macro `__inline_mac_impl` (in Nightly builds, run with -Z macro-backtrace for more info)
LL | (*&mut a.padding) = [1; size_of::<DataWithPadding>()];
| ^^^^^^^^^^^^^^^^^ help: try: `a.padding`
error: immediately dereferencing a reference
--> tests/ui/deref_addrof.rs:71:17
--> tests/ui/deref_addrof.rs:106:9
|
LL | inline!(*&mut $(@expr self))
| ^^^^^^^^^^^^^^^^^^^ help: try: `$(@expr self)`
LL | (*&mut a.tup).1 = ();
| ^^^^^^^^^^^^^ help: try: `a.tup`
error: immediately dereferencing a reference
--> tests/ui/deref_addrof.rs:108:10
|
= note: this error originates in the macro `__inline_mac_impl` (in Nightly builds, run with -Z macro-backtrace for more info)
LL | **&mut a.prim = 0;
| ^^^^^^^^^^^^ help: try: `a.prim`
error: aborting due to 11 previous errors
error: immediately dereferencing a reference
--> tests/ui/deref_addrof.rs:111:9
|
LL | (*&mut a.data).num = 42;
| ^^^^^^^^^^^^^^ help: try: `(*a.data)`
error: immediately dereferencing a reference
--> tests/ui/deref_addrof.rs:113:9
|
LL | (*&mut a.indirect.md)[3] = 1;
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `(*a.indirect.md)`
error: immediately dereferencing a reference
--> tests/ui/deref_addrof.rs:115:9
|
LL | (*&mut a.indirect_arr[1].md)[3] = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(*a.indirect_arr[1].md)`
error: immediately dereferencing a reference
--> tests/ui/deref_addrof.rs:117:9
|
LL | (*&mut a.indirect_ref.md)[3] = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(*a.indirect_ref.md)`
error: immediately dereferencing a reference
--> tests/ui/deref_addrof.rs:121:10
|
LL | **&raw mut a.prim = 0;
| ^^^^^^^^^^^^^^^^ help: try: `a.prim`
error: immediately dereferencing a reference
--> tests/ui/deref_addrof.rs:123:9
|
LL | (*&raw mut a.data).num = 42;
| ^^^^^^^^^^^^^^^^^^ help: try: `(*a.data)`
error: aborting due to 18 previous errors