Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 56 additions & 12 deletions clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@ use clippy_utils::ty::{
adjust_derefs_manually_drop, get_adt_inherent_method, implements_trait, is_manually_drop, peel_and_count_ty_refs,
};
use clippy_utils::{
DefinedTy, ExprUseNode, get_expr_use_site, get_parent_expr, is_block_like, is_from_proc_macro, is_lint_allowed, sym,
DefinedTy, ExprUseNode, expr_use_sites, get_expr_use_site, get_parent_expr, is_block_like, is_from_proc_macro,
is_lint_allowed, sym,
};
use rustc_ast::util::parser::ExprPrecedence;
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::Applicability;
use rustc_hir::attrs::{AttributeKind, HasAttrs as _};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{InferKind, Visitor, VisitorExt as _, walk_ty};
use rustc_hir::{
self as hir, AmbigArg, BindingMode, Body, BodyId, BorrowKind, Expr, ExprKind, HirId, Item, MatchSource, Mutability,
Node, OwnerId, Pat, PatKind, Path, QPath, TyKind, UnOp,
self as hir, AmbigArg, Attribute, BindingMode, Body, BodyId, BorrowKind, Expr, ExprKind, HirId, Item, MatchSource,
Mutability, Node, OwnerId, Pat, PatKind, Path, QPath, TyKind, UnOp,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
Expand Down Expand Up @@ -275,6 +277,10 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
return;
}

if is_ref_from_no_implicit_raw_pointer(cx, typeck, expr, sub_expr) {
return;
}

Comment on lines +280 to +283

@Gri-ffin Gri-ffin Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you put this guard here instead of where the old guard was? This would now skip handling other diagnostics for the expression not just needless_borrow.

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gri-ffin
My reasons :

  1. need_explicit_ref returns true only when the explicit ref is from a raw pointer deref (maybe index/field operation in the middle), so it will not skip other diagnostics, all tests passed confirms this.
  2. Returning early avoids running all the subsequent checks and matches in dereference.rs when we already know the lint should be suppressed. The old guard was placed later after many calculations.

Let me know if you have other ideas !!!

match (self.state.take(), kind) {
(None, kind) => {
let expr_ty = typeck.expr_ty(expr);
Expand Down Expand Up @@ -732,6 +738,53 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
}
}

fn is_ref_from_no_implicit_raw_pointer<'tcx>(
cx: &LateContext<'tcx>,
typeck: &'tcx TypeckResults<'tcx>,
expr: &'tcx Expr<'tcx>,
sub_expr: &Expr<'tcx>,
) -> bool {
let mut temp_e = sub_expr;

typeck
.expr_ty(loop {
match temp_e.kind {
ExprKind::Index(e, _, _) | ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) => temp_e = e,
ExprKind::Unary(UnOp::Deref, e) => break e,
_ => break temp_e,
}
})
.is_raw_ptr()
&& find_no_auto_method(cx, typeck, expr).is_some_and(|fn_id| {
fn_id
.get_attrs(&cx.tcx) // can not use private micro find_attr!()
.iter()
.any(|attr| matches!(attr, Attribute::Parsed(AttributeKind::RustcNoImplicitAutorefs)))
})
}

fn find_no_auto_method<'tcx>(
cx: &LateContext<'tcx>,
typeck: &'tcx TypeckResults<'tcx>,
mut expr: &'tcx Expr<'tcx>,
) -> Option<DefId> {
while let Some(use_site) = expr_use_sites(cx.tcx, typeck, SyntaxContext::root(), expr).next()
&& let node = use_site.node
{
match node {
Comment thread
skiefucker marked this conversation as resolved.
Outdated
Node::Expr(use_expr) => match use_expr.kind {
ExprKind::Field(_, _) | ExprKind::Index(_, _, _) => {
expr = use_expr;
},
ExprKind::MethodCall(_, _, _, _) => return typeck.type_dependent_def_id(use_expr.hir_id),
_ => return None,
},
_ => return None,
}
}
None
}

fn is_deref_or_derefmut_impl(cx: &LateContext<'_>, item: &Item<'_>) -> bool {
if let hir::ItemKind::Impl(impl_) = item.kind
&& let Some(of_trait) = impl_.of_trait
Expand Down Expand Up @@ -1153,15 +1206,6 @@ impl<'tcx> Dereferencing<'tcx> {
);
},
State::DerefedBorrow(state) => {
// Do not suggest removing a non-mandatory `&` in `&*rawptr` in an `unsafe` context,
// as this may make rustc trigger its `dangerous_implicit_autorefs` lint.
if let ExprKind::AddrOf(BorrowKind::Ref, _, subexpr) = data.first_expr.kind
&& let ExprKind::Unary(UnOp::Deref, subsubexpr) = subexpr.kind
&& cx.typeck_results().expr_ty_adjusted(subsubexpr).is_raw_ptr()
{
return;
}

let mut app = Applicability::MachineApplicable;
let (snip, snip_is_macro) =
snippet_with_context(cx, expr.span, data.first_expr.span.ctxt(), "..", &mut app);
Expand Down
73 changes: 73 additions & 0 deletions tests/ui/needless_borrow.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
clippy::unnecessary_mut_passed
)]

use std::ops::Index;

fn main() {
let a = 5;
let ref_a = &a;
Expand Down Expand Up @@ -294,3 +296,74 @@ fn issue_14743<T>(slice: &[T]) {
#[expect(dangerous_implicit_autorefs)]
let _ = unsafe { (*slice).len() };
}

fn issue_17414(
slice: &(i32, (i32, [usize])),
tuple_slice: *const (i32, [usize]),
nested_tuple: *const (i32, (i32, [usize])),
) {
let _ = slice.1.1.len();
//~^ needless_borrow
let _ = slice.1.1.len();
//~^ needless_borrow
let _ = slice.1.1.len();
//~^ needless_borrow

unsafe {
// Rule: `rustc_lint:autoref.rs` require explicit ref to arg from a [inner] derefed raw pointer
// when calling a #[rustc_no_implicit_autorefs] method.

// 1. Issue 17414 case: deref tuple, get slice field, call [].len() method.
let _ = (&(*tuple_slice).1).len(); // necessary borrow, no lint

// 2. simple slice
let a = [0, 1, 2];
let b = &raw const a[0..2]; // same with raw const or raw mut
let _ = (&*b).len(); // necessary
let _ = (*b)[0]; // ok
let _ = (&*b)[0]; // TODO needless, should lint

// 3. nested tuple
let _ = (&*nested_tuple).1.1.len(); // necessary
let _ = (&(*nested_tuple).1).1.len(); // necessary
let _ = (&(*nested_tuple).1.1).len(); // necessary

let _ = (*nested_tuple).1.1.first(); // ok

let _ = (*nested_tuple).1.1.first();
//~^ needless_borrow

let _ = (*nested_tuple).1.1.first();
//~^ needless_borrow

let _ = (*nested_tuple).1.1.first();
//~^ needless_borrow

// 4. array
let a = [1, 2, 3];
let b: *const [i32; 3] = &raw const a;
// let _ = (*b).index(0); // fail
let _ = (&*b).index(0); // necessary

// 5. trait
trait T: Index<usize, Output = i32> {}
struct S {}
impl Index<usize> for S {
type Output = i32;
// no attr, defined in super.index
fn index(&self, _: usize) -> &i32 {
&42
}
}
impl T for S {}

let s = S {};
let t: &dyn T = &s;
let ptr: *const dyn T = t;

// let _ = (*ptr)[0]; // fail
let _ = (&*ptr)[0]; // necessary
// let _ = (*ptr).index(0); // fail
let _ = (&*ptr).index(0); // necessary
}
}
73 changes: 73 additions & 0 deletions tests/ui/needless_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
clippy::unnecessary_mut_passed
)]

use std::ops::Index;

fn main() {
let a = 5;
let ref_a = &a;
Expand Down Expand Up @@ -294,3 +296,74 @@ fn issue_14743<T>(slice: &[T]) {
#[expect(dangerous_implicit_autorefs)]
let _ = unsafe { (*slice).len() };
}

fn issue_17414(
slice: &(i32, (i32, [usize])),
tuple_slice: *const (i32, [usize]),
nested_tuple: *const (i32, (i32, [usize])),
) {
let _ = (&slice).1.1.len();
//~^ needless_borrow
let _ = (&slice.1).1.len();
//~^ needless_borrow
let _ = (&slice.1.1).len();
//~^ needless_borrow

unsafe {
// Rule: `rustc_lint:autoref.rs` require explicit ref to arg from a [inner] derefed raw pointer
// when calling a #[rustc_no_implicit_autorefs] method.

// 1. Issue 17414 case: deref tuple, get slice field, call [].len() method.
let _ = (&(*tuple_slice).1).len(); // necessary borrow, no lint

// 2. simple slice
let a = [0, 1, 2];
let b = &raw const a[0..2]; // same with raw const or raw mut
let _ = (&*b).len(); // necessary
let _ = (*b)[0]; // ok
let _ = (&*b)[0]; // TODO needless, should lint
Comment thread
skiefucker marked this conversation as resolved.
Outdated

// 3. nested tuple
let _ = (&*nested_tuple).1.1.len(); // necessary
let _ = (&(*nested_tuple).1).1.len(); // necessary
let _ = (&(*nested_tuple).1.1).len(); // necessary

let _ = (*nested_tuple).1.1.first(); // ok

let _ = (&*nested_tuple).1.1.first();
//~^ needless_borrow

let _ = (&(*nested_tuple).1).1.first();
//~^ needless_borrow

let _ = (&(*nested_tuple).1.1).first();
//~^ needless_borrow

// 4. array
let a = [1, 2, 3];
let b: *const [i32; 3] = &raw const a;
// let _ = (*b).index(0); // fail
let _ = (&*b).index(0); // necessary

// 5. trait
trait T: Index<usize, Output = i32> {}
struct S {}
impl Index<usize> for S {
type Output = i32;
// no attr, defined in super.index
fn index(&self, _: usize) -> &i32 {
&42
}
}
impl T for S {}

let s = S {};
let t: &dyn T = &s;
let ptr: *const dyn T = t;

// let _ = (*ptr)[0]; // fail
let _ = (&*ptr)[0]; // necessary
// let _ = (*ptr).index(0); // fail
let _ = (&*ptr).index(0); // necessary
}
}
Loading