diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index d1b25a0f1b53..99212b45ed52 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -184,6 +184,7 @@ impl Dereferencing { } } +#[derive(Debug)] struct StateData { /// Span of the top level expression span: Span, @@ -191,12 +192,14 @@ struct StateData { position: Position, } +#[derive(Debug)] struct DerefedBorrow { count: usize, msg: &'static str, snip_expr: Option, } +#[derive(Debug)] enum State { // Any number of deref method calls. DerefMethod { @@ -276,10 +279,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { (None, kind) => { let expr_ty = typeck.expr_ty(expr); let (position, adjustments) = walk_parents(cx, expr, self.msrv); - match kind { RefOp::Deref => { - if let Position::FieldAccess(name) = position + if let Position::FieldAccess { + name, + of_union: false, + } = position && !ty_contains_field(typeck.expr_ty(sub_expr), name) { self.state = Some(( @@ -451,7 +456,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { (Some((State::DerefedBorrow(state), data)), RefOp::Deref) => { let position = data.position; report(cx, expr, State::DerefedBorrow(state), data); - if let Position::FieldAccess(name) = position + if let Position::FieldAccess{name, ..} = position && !ty_contains_field(typeck.expr_ty(sub_expr), name) { self.state = Some(( @@ -616,14 +621,17 @@ fn deref_method_same_type<'tcx>(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool { } /// The position of an expression relative to it's parent. -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] enum Position { MethodReceiver, /// The method is defined on a reference type. e.g. `impl Foo for &T` MethodReceiverRefImpl, Callee, ImplArg(HirId), - FieldAccess(Symbol), + FieldAccess { + name: Symbol, + of_union: bool, + }, // union fields cannot be auto borrowed Postfix, Deref, /// Any other location which will trigger auto-deref to a specific time. @@ -645,7 +653,10 @@ impl Position { } fn can_auto_borrow(self) -> bool { - matches!(self, Self::MethodReceiver | Self::FieldAccess(_) | Self::Callee) + matches!( + self, + Self::MethodReceiver | Self::FieldAccess { of_union: false, .. } | Self::Callee + ) } fn lint_explicit_deref(self) -> bool { @@ -657,7 +668,7 @@ impl Position { Self::MethodReceiver | Self::MethodReceiverRefImpl | Self::Callee - | Self::FieldAccess(_) + | Self::FieldAccess { .. } | Self::Postfix => PREC_POSTFIX, Self::ImplArg(_) | Self::Deref => PREC_PREFIX, Self::DerefStable(p, _) | Self::ReborrowStable(p) | Self::Other(p) => p, @@ -844,7 +855,10 @@ fn walk_parents<'tcx>( } }) }, - ExprKind::Field(child, name) if child.hir_id == e.hir_id => Some(Position::FieldAccess(name.name)), + ExprKind::Field(child, name) if child.hir_id == e.hir_id => Some(Position::FieldAccess { + name: name.name, + of_union: is_union(cx.typeck_results(), child), + }), ExprKind::Unary(UnOp::Deref, child) if child.hir_id == e.hir_id => Some(Position::Deref), ExprKind::Match(child, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) | ExprKind::Index(child, _) @@ -865,6 +879,13 @@ fn walk_parents<'tcx>( (position, adjustments) } +fn is_union<'tcx>(typeck: &'tcx TypeckResults<'_>, path_expr: &'tcx Expr<'_>) -> bool { + typeck + .expr_ty_adjusted(path_expr) + .ty_adt_def() + .map_or(false, rustc_middle::ty::AdtDef::is_union) +} + fn closure_result_position<'tcx>( cx: &LateContext<'tcx>, closure: &'tcx Closure<'_>, diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index 8cf93bd24817..6914f9183c7c 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -298,3 +298,32 @@ mod meets_msrv { let _ = std::process::Command::new("ls").args(["-a", "-l"]).status().unwrap(); } } + +#[allow(unused)] +fn issue9383() { + // Should not lint because unions need explicit deref when accessing field + use std::mem::ManuallyDrop; + + union Coral { + crab: ManuallyDrop>, + } + + union Ocean { + coral: ManuallyDrop, + } + + let mut ocean = Ocean { + coral: ManuallyDrop::new(Coral { + crab: ManuallyDrop::new(vec![1, 2, 3]), + }), + }; + + unsafe { + ManuallyDrop::drop(&mut (&mut ocean.coral).crab); + + (*ocean.coral).crab = ManuallyDrop::new(vec![4, 5, 6]); + ManuallyDrop::drop(&mut (*ocean.coral).crab); + + ManuallyDrop::drop(&mut ocean.coral); + } +} diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index fd9b2a11df96..e2a609fa5d27 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -298,3 +298,32 @@ mod meets_msrv { let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap(); } } + +#[allow(unused)] +fn issue9383() { + // Should not lint because unions need explicit deref when accessing field + use std::mem::ManuallyDrop; + + union Coral { + crab: ManuallyDrop>, + } + + union Ocean { + coral: ManuallyDrop, + } + + let mut ocean = Ocean { + coral: ManuallyDrop::new(Coral { + crab: ManuallyDrop::new(vec![1, 2, 3]), + }), + }; + + unsafe { + ManuallyDrop::drop(&mut (&mut ocean.coral).crab); + + (*ocean.coral).crab = ManuallyDrop::new(vec![4, 5, 6]); + ManuallyDrop::drop(&mut (*ocean.coral).crab); + + ManuallyDrop::drop(&mut ocean.coral); + } +}