diff --git a/src/tools/clippy/clippy_lints/src/double_parens.rs b/src/tools/clippy/clippy_lints/src/double_parens.rs index bddf4702fb340..8defbeeaa5f22 100644 --- a/src/tools/clippy/clippy_lints/src/double_parens.rs +++ b/src/tools/clippy/clippy_lints/src/double_parens.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::{HasSession, snippet_with_applicability, snippet_with_context}; +use clippy_utils::source::{HasSession, SpanRangeExt, snippet_with_applicability, snippet_with_context}; use rustc_ast::ast::{Expr, ExprKind, MethodCall}; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; @@ -47,8 +47,12 @@ impl EarlyLintPass for DoubleParens { // ^^^^^^ expr // ^^^^ inner ExprKind::Paren(inner) if matches!(inner.kind, ExprKind::Paren(_) | ExprKind::Tup(_)) => { - // suggest removing the outer parens - if expr.span.eq_ctxt(inner.span) { + if expr.span.eq_ctxt(inner.span) + && !expr.span.in_external_macro(cx.sess().source_map()) + && check_source(cx, inner) + { + // suggest removing the outer parens + let mut applicability = Applicability::MachineApplicable; // We don't need to use `snippet_with_context` here, because: // - if `inner`'s `ctxt` is from macro, we don't lint in the first place (see the check above) @@ -74,8 +78,12 @@ impl EarlyLintPass for DoubleParens { if let [arg] = &**args && let ExprKind::Paren(inner) = &arg.kind => { - // suggest removing the inner parens - if expr.span.eq_ctxt(arg.span) { + if expr.span.eq_ctxt(arg.span) + && !arg.span.in_external_macro(cx.sess().source_map()) + && check_source(cx, arg) + { + // suggest removing the inner parens + let mut applicability = Applicability::MachineApplicable; let sugg = snippet_with_context(cx.sess(), inner.span, arg.span.ctxt(), "_", &mut applicability).0; span_lint_and_sugg( @@ -93,3 +101,22 @@ impl EarlyLintPass for DoubleParens { } } } + +/// Check that the span does indeed look like `( (..) )` +fn check_source(cx: &EarlyContext<'_>, inner: &Expr) -> bool { + if let Some(sfr) = inner.span.get_source_range(cx) + // this is the same as `SourceFileRange::as_str`, but doesn't apply the range right away, because + // we're interested in the source code outside it + && let Some(src) = sfr.sf.src.as_ref().map(|src| src.as_str()) + && let Some((start, outer_after_inner)) = src.split_at_checked(sfr.range.end) + && let Some((outer_before_inner, inner)) = start.split_at_checked(sfr.range.start) + && outer_before_inner.trim_end().ends_with('(') + && inner.starts_with('(') + && inner.ends_with(')') + && outer_after_inner.trim_start().starts_with(')') + { + true + } else { + false + } +} diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index a4ad9424b3eb1..e270d3d9cf12e 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -828,6 +828,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny)); store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)); store.register_late_pass(|_| Box::new(volatile_composites::VolatileComposites)); - store.register_late_pass(|_| Box::new(replace_box::ReplaceBox)); + store.register_late_pass(|_| Box::new(replace_box::ReplaceBox::default())); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/src/tools/clippy/clippy_lints/src/multiple_unsafe_ops_per_block.rs b/src/tools/clippy/clippy_lints/src/multiple_unsafe_ops_per_block.rs index bc5e72270f4e3..80cf081992cc7 100644 --- a/src/tools/clippy/clippy_lints/src/multiple_unsafe_ops_per_block.rs +++ b/src/tools/clippy/clippy_lints/src/multiple_unsafe_ops_per_block.rs @@ -1,13 +1,13 @@ use clippy_utils::desugar_await; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::visitors::{Descend, Visitable, for_each_expr}; -use core::ops::ControlFlow::Continue; use hir::def::{DefKind, Res}; use hir::{BlockCheckMode, ExprKind, QPath, UnOp}; -use rustc_ast::Mutability; +use rustc_ast::{BorrowKind, Mutability}; use rustc_hir as hir; +use rustc_hir::intravisit::{Visitor, walk_body, walk_expr}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty; +use rustc_middle::hir::nested_filter; +use rustc_middle::ty::{self, TyCtxt, TypeckResults}; use rustc_session::declare_lint_pass; use rustc_span::{DesugaringKind, Span}; @@ -55,6 +55,13 @@ declare_clippy_lint! { /// unsafe { char::from_u32_unchecked(int_value) } /// } /// ``` + /// + /// ### Note + /// + /// Taking a raw pointer to a union field is always safe and will + /// not be considered unsafe by this lint, even when linting code written + /// with a specified Rust version of 1.91 or earlier (which required + /// using an `unsafe` block). #[clippy::version = "1.69.0"] pub MULTIPLE_UNSAFE_OPS_PER_BLOCK, restriction, @@ -70,8 +77,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock { { return; } - let mut unsafe_ops = vec![]; - collect_unsafe_exprs(cx, block, &mut unsafe_ops); + let unsafe_ops = UnsafeExprCollector::collect_unsafe_exprs(cx, block); if unsafe_ops.len() > 1 { span_lint_and_then( cx, @@ -91,25 +97,49 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock { } } -fn collect_unsafe_exprs<'tcx>( - cx: &LateContext<'tcx>, - node: impl Visitable<'tcx>, - unsafe_ops: &mut Vec<(&'static str, Span)>, -) { - for_each_expr(cx, node, |expr| { +struct UnsafeExprCollector<'tcx> { + tcx: TyCtxt<'tcx>, + typeck_results: &'tcx TypeckResults<'tcx>, + unsafe_ops: Vec<(&'static str, Span)>, +} + +impl<'tcx> UnsafeExprCollector<'tcx> { + fn collect_unsafe_exprs(cx: &LateContext<'tcx>, block: &'tcx hir::Block<'tcx>) -> Vec<(&'static str, Span)> { + let mut collector = Self { + tcx: cx.tcx, + typeck_results: cx.typeck_results(), + unsafe_ops: vec![], + }; + collector.visit_block(block); + collector.unsafe_ops + } +} + +impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> { + type NestedFilter = nested_filter::OnlyBodies; + + fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) { match expr.kind { // The `await` itself will desugar to two unsafe calls, but we should ignore those. // Instead, check the expression that is `await`ed _ if let Some(e) = desugar_await(expr) => { - collect_unsafe_exprs(cx, e, unsafe_ops); - return Continue(Descend::No); + return self.visit_expr(e); }, - ExprKind::InlineAsm(_) => unsafe_ops.push(("inline assembly used here", expr.span)), + ExprKind::InlineAsm(_) => self.unsafe_ops.push(("inline assembly used here", expr.span)), + + ExprKind::AddrOf(BorrowKind::Raw, _, mut inner) => { + while let ExprKind::Field(prefix, _) = inner.kind + && self.typeck_results.expr_adjustments(prefix).is_empty() + { + inner = prefix; + } + return self.visit_expr(inner); + }, ExprKind::Field(e, _) => { - if cx.typeck_results().expr_ty(e).is_union() { - unsafe_ops.push(("union field access occurs here", expr.span)); + if self.typeck_results.expr_ty(e).is_union() { + self.unsafe_ops.push(("union field access occurs here", expr.span)); } }, @@ -127,32 +157,32 @@ fn collect_unsafe_exprs<'tcx>( .. }, )) => { - unsafe_ops.push(("access of a mutable static occurs here", expr.span)); + self.unsafe_ops + .push(("access of a mutable static occurs here", expr.span)); }, - ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty_adjusted(e).is_raw_ptr() => { - unsafe_ops.push(("raw pointer dereference occurs here", expr.span)); + ExprKind::Unary(UnOp::Deref, e) if self.typeck_results.expr_ty(e).is_raw_ptr() => { + self.unsafe_ops.push(("raw pointer dereference occurs here", expr.span)); }, ExprKind::Call(path_expr, _) => { - let sig = match *cx.typeck_results().expr_ty(path_expr).kind() { - ty::FnDef(id, _) => cx.tcx.fn_sig(id).skip_binder(), - ty::FnPtr(sig_tys, hdr) => sig_tys.with(hdr), - _ => return Continue(Descend::Yes), + let opt_sig = match *self.typeck_results.expr_ty_adjusted(path_expr).kind() { + ty::FnDef(id, _) => Some(self.tcx.fn_sig(id).skip_binder()), + ty::FnPtr(sig_tys, hdr) => Some(sig_tys.with(hdr)), + _ => None, }; - if sig.safety().is_unsafe() { - unsafe_ops.push(("unsafe function call occurs here", expr.span)); + if opt_sig.is_some_and(|sig| sig.safety().is_unsafe()) { + self.unsafe_ops.push(("unsafe function call occurs here", expr.span)); } }, ExprKind::MethodCall(..) => { - if let Some(sig) = cx - .typeck_results() + let opt_sig = self + .typeck_results .type_dependent_def_id(expr.hir_id) - .map(|def_id| cx.tcx.fn_sig(def_id)) - && sig.skip_binder().safety().is_unsafe() - { - unsafe_ops.push(("unsafe method call occurs here", expr.span)); + .map(|def_id| self.tcx.fn_sig(def_id)); + if opt_sig.is_some_and(|sig| sig.skip_binder().safety().is_unsafe()) { + self.unsafe_ops.push(("unsafe method call occurs here", expr.span)); } }, @@ -173,15 +203,26 @@ fn collect_unsafe_exprs<'tcx>( } )) ) { - unsafe_ops.push(("modification of a mutable static occurs here", expr.span)); - collect_unsafe_exprs(cx, rhs, unsafe_ops); - return Continue(Descend::No); + self.unsafe_ops + .push(("modification of a mutable static occurs here", expr.span)); + return self.visit_expr(rhs); } }, _ => {}, } - Continue::<(), _>(Descend::Yes) - }); + walk_expr(self, expr); + } + + fn visit_body(&mut self, body: &hir::Body<'tcx>) { + let saved_typeck_results = self.typeck_results; + self.typeck_results = self.tcx.typeck_body(body.id()); + walk_body(self, body); + self.typeck_results = saved_typeck_results; + } + + fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt { + self.tcx + } } diff --git a/src/tools/clippy/clippy_lints/src/replace_box.rs b/src/tools/clippy/clippy_lints/src/replace_box.rs index 4bbd1803a78d2..638f6dc1532bb 100644 --- a/src/tools/clippy/clippy_lints/src/replace_box.rs +++ b/src/tools/clippy/clippy_lints/src/replace_box.rs @@ -3,11 +3,17 @@ use clippy_utils::res::{MaybeDef, MaybeResPath}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::implements_trait; use clippy_utils::{is_default_equivalent_call, local_is_initialized}; +use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::smallvec::SmallVec; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, LangItem, QPath}; +use rustc_hir::{Body, BodyId, Expr, ExprKind, HirId, LangItem, QPath}; +use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::declare_lint_pass; -use rustc_span::sym; +use rustc_middle::hir::place::ProjectionKind; +use rustc_middle::mir::FakeReadCause; +use rustc_middle::ty; +use rustc_session::impl_lint_pass; +use rustc_span::{Symbol, sym}; declare_clippy_lint! { /// ### What it does @@ -33,17 +39,57 @@ declare_clippy_lint! { perf, "assigning a newly created box to `Box` is inefficient" } -declare_lint_pass!(ReplaceBox => [REPLACE_BOX]); + +#[derive(Default)] +pub struct ReplaceBox { + consumed_locals: FxHashSet, + loaded_bodies: SmallVec<[BodyId; 2]>, +} + +impl ReplaceBox { + fn get_consumed_locals(&mut self, cx: &LateContext<'_>) -> &FxHashSet { + if let Some(body_id) = cx.enclosing_body + && !self.loaded_bodies.contains(&body_id) + { + self.loaded_bodies.push(body_id); + ExprUseVisitor::for_clippy( + cx, + cx.tcx.hir_body_owner_def_id(body_id), + MovedVariablesCtxt { + consumed_locals: &mut self.consumed_locals, + }, + ) + .consume_body(cx.tcx.hir_body(body_id)) + .into_ok(); + } + + &self.consumed_locals + } +} + +impl_lint_pass!(ReplaceBox => [REPLACE_BOX]); impl LateLintPass<'_> for ReplaceBox { + fn check_body_post(&mut self, _: &LateContext<'_>, body: &Body<'_>) { + if self.loaded_bodies.first().is_some_and(|&x| x == body.id()) { + self.consumed_locals.clear(); + self.loaded_bodies.clear(); + } + } + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) { if let ExprKind::Assign(lhs, rhs, _) = &expr.kind && !lhs.span.from_expansion() && !rhs.span.from_expansion() && let lhs_ty = cx.typeck_results().expr_ty(lhs) + && let Some(inner_ty) = lhs_ty.boxed_ty() // No diagnostic for late-initialized locals && lhs.res_local_id().is_none_or(|local| local_is_initialized(cx, local)) - && let Some(inner_ty) = lhs_ty.boxed_ty() + // No diagnostic if this is a local that has been moved, or the field + // of a local that has been moved, or several chained field accesses of a local + && local_base(lhs).is_none_or(|(base_id, _)| { + !self.get_consumed_locals(cx).contains(&base_id) + }) { if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default) && implements_trait(cx, inner_ty, default_trait_id, &[]) @@ -109,3 +155,46 @@ fn get_box_new_payload<'tcx>(cx: &LateContext<'_>, expr: &Expr<'tcx>) -> Option< None } } + +struct MovedVariablesCtxt<'a> { + consumed_locals: &'a mut FxHashSet, +} + +impl<'tcx> Delegate<'tcx> for MovedVariablesCtxt<'_> { + fn consume(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId) { + if let PlaceBase::Local(id) = cmt.place.base + && let mut projections = cmt + .place + .projections + .iter() + .filter(|x| matches!(x.kind, ProjectionKind::Deref)) + // Either no deref or multiple derefs + && (projections.next().is_none() || projections.next().is_some()) + { + self.consumed_locals.insert(id); + } + } + + fn use_cloned(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {} + + fn borrow(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) {} + + fn mutate(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {} + + fn fake_read(&mut self, _: &PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {} +} + +/// A local place followed by optional fields +type IdFields = (HirId, Vec); + +/// If `expr` is a local variable with optional field accesses, return it. +fn local_base(expr: &Expr<'_>) -> Option { + match expr.kind { + ExprKind::Path(qpath) => qpath.res_local_id().map(|id| (id, Vec::new())), + ExprKind::Field(expr, field) => local_base(expr).map(|(id, mut fields)| { + fields.push(field.name); + (id, fields) + }), + _ => None, + } +} diff --git a/src/tools/clippy/tests/ui/auxiliary/macro_rules.rs b/src/tools/clippy/tests/ui/auxiliary/macro_rules.rs index 9efbb39084976..1f1afe4ea3a53 100644 --- a/src/tools/clippy/tests/ui/auxiliary/macro_rules.rs +++ b/src/tools/clippy/tests/ui/auxiliary/macro_rules.rs @@ -57,3 +57,15 @@ macro_rules! bad_transmute { std::mem::transmute($e) }; } + +#[macro_export] +#[rustfmt::skip] +macro_rules! double_parens { + ($a:expr, $b:expr, $c:expr, $d:expr) => {{ + let a = ($a); + let a = (()); + let b = ((5)); + let c = std::convert::identity((5)); + InterruptMask((($a.union($b).union($c).union($d)).into_bits()) as u32) + }}; +} diff --git a/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs b/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs index 5465092287179..629a500ff6f56 100644 --- a/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs +++ b/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs @@ -230,3 +230,14 @@ pub fn allow_lint_same_span_derive(input: TokenStream) -> TokenStream { span_help(Group::new(Delimiter::Brace, TokenStream::new()).into()), ]) } + +#[proc_macro_derive(DoubleParens)] +pub fn derive_double_parens(_: TokenStream) -> TokenStream { + quote! { + fn foo() { + let a = (()); + let b = ((5)); + let c = std::convert::identity((5)); + } + } +} diff --git a/src/tools/clippy/tests/ui/double_parens.fixed b/src/tools/clippy/tests/ui/double_parens.fixed index dedc513438d11..024af68401327 100644 --- a/src/tools/clippy/tests/ui/double_parens.fixed +++ b/src/tools/clippy/tests/ui/double_parens.fixed @@ -1,8 +1,13 @@ +//@aux-build:proc_macros.rs +//@aux-build:proc_macro_derive.rs +//@aux-build:macro_rules.rs #![warn(clippy::double_parens)] #![expect(clippy::eq_op, clippy::no_effect)] #![feature(custom_inner_attributes)] #![rustfmt::skip] +use proc_macros::{external, with_span}; + fn dummy_fn(_: T) {} struct DummyStruct; @@ -96,4 +101,64 @@ fn issue9000(x: DummyStruct) { //~^ double_parens } +fn issue15892() { + use macro_rules::double_parens as double_parens_external; + + macro_rules! double_parens{ + ($a:expr, $b:expr, $c:expr, $d:expr) => {{ + let a = ($a); + let a = (); + //~^ double_parens + let b = (5); + //~^ double_parens + let c = std::convert::identity(5); + //~^ double_parens + InterruptMask((($a.union($b).union($c).union($d)).into_bits()) as u32) + }}; + } + + // Don't lint: external macro + (external!((5))); + external!(((5))); + + #[repr(transparent)] + #[derive(Clone, Copy, PartialEq, Eq)] + pub struct InterruptMask(u32); + + impl InterruptMask { + pub const OE: InterruptMask = InterruptMask(1 << 10); + pub const BE: InterruptMask = InterruptMask(1 << 9); + pub const PE: InterruptMask = InterruptMask(1 << 8); + pub const FE: InterruptMask = InterruptMask(1 << 7); + // Lint: internal macro + pub const E: InterruptMask = double_parens!((Self::OE), Self::BE, Self::PE, Self::FE); + // Don't lint: external macro + pub const F: InterruptMask = double_parens_external!((Self::OE), Self::BE, Self::PE, Self::FE); + #[allow(clippy::unnecessary_cast)] + pub const G: InterruptMask = external!( + InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32) + ); + #[allow(clippy::unnecessary_cast)] + // Don't lint: external proc-macro + pub const H: InterruptMask = with_span!(span + InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32) + ); + pub const fn into_bits(self) -> u32 { + self.0 + } + #[must_use] + pub const fn union(self, rhs: Self) -> Self { + InterruptMask(self.0 | rhs.0) + } + } +} + +fn issue15940() { + use proc_macro_derive::DoubleParens; + + #[derive(DoubleParens)] + // Don't lint: external derive macro + pub struct Person; +} + fn main() {} diff --git a/src/tools/clippy/tests/ui/double_parens.rs b/src/tools/clippy/tests/ui/double_parens.rs index 27f252485b714..8a76f2837f353 100644 --- a/src/tools/clippy/tests/ui/double_parens.rs +++ b/src/tools/clippy/tests/ui/double_parens.rs @@ -1,8 +1,13 @@ +//@aux-build:proc_macros.rs +//@aux-build:proc_macro_derive.rs +//@aux-build:macro_rules.rs #![warn(clippy::double_parens)] #![expect(clippy::eq_op, clippy::no_effect)] #![feature(custom_inner_attributes)] #![rustfmt::skip] +use proc_macros::{external, with_span}; + fn dummy_fn(_: T) {} struct DummyStruct; @@ -96,4 +101,64 @@ fn issue9000(x: DummyStruct) { //~^ double_parens } +fn issue15892() { + use macro_rules::double_parens as double_parens_external; + + macro_rules! double_parens{ + ($a:expr, $b:expr, $c:expr, $d:expr) => {{ + let a = ($a); + let a = (()); + //~^ double_parens + let b = ((5)); + //~^ double_parens + let c = std::convert::identity((5)); + //~^ double_parens + InterruptMask((($a.union($b).union($c).union($d)).into_bits()) as u32) + }}; + } + + // Don't lint: external macro + (external!((5))); + external!(((5))); + + #[repr(transparent)] + #[derive(Clone, Copy, PartialEq, Eq)] + pub struct InterruptMask(u32); + + impl InterruptMask { + pub const OE: InterruptMask = InterruptMask(1 << 10); + pub const BE: InterruptMask = InterruptMask(1 << 9); + pub const PE: InterruptMask = InterruptMask(1 << 8); + pub const FE: InterruptMask = InterruptMask(1 << 7); + // Lint: internal macro + pub const E: InterruptMask = double_parens!((Self::OE), Self::BE, Self::PE, Self::FE); + // Don't lint: external macro + pub const F: InterruptMask = double_parens_external!((Self::OE), Self::BE, Self::PE, Self::FE); + #[allow(clippy::unnecessary_cast)] + pub const G: InterruptMask = external!( + InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32) + ); + #[allow(clippy::unnecessary_cast)] + // Don't lint: external proc-macro + pub const H: InterruptMask = with_span!(span + InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32) + ); + pub const fn into_bits(self) -> u32 { + self.0 + } + #[must_use] + pub const fn union(self, rhs: Self) -> Self { + InterruptMask(self.0 | rhs.0) + } + } +} + +fn issue15940() { + use proc_macro_derive::DoubleParens; + + #[derive(DoubleParens)] + // Don't lint: external derive macro + pub struct Person; +} + fn main() {} diff --git a/src/tools/clippy/tests/ui/double_parens.stderr b/src/tools/clippy/tests/ui/double_parens.stderr index 3a740e44cacf3..51b5c6279b211 100644 --- a/src/tools/clippy/tests/ui/double_parens.stderr +++ b/src/tools/clippy/tests/ui/double_parens.stderr @@ -1,5 +1,5 @@ error: unnecessary parentheses - --> tests/ui/double_parens.rs:15:5 + --> tests/ui/double_parens.rs:20:5 | LL | ((0)) | ^^^^^ help: remove them: `(0)` @@ -8,37 +8,37 @@ LL | ((0)) = help: to override `-D warnings` add `#[allow(clippy::double_parens)]` error: unnecessary parentheses - --> tests/ui/double_parens.rs:20:14 + --> tests/ui/double_parens.rs:25:14 | LL | dummy_fn((0)); | ^^^ help: remove them: `0` error: unnecessary parentheses - --> tests/ui/double_parens.rs:25:20 + --> tests/ui/double_parens.rs:30:20 | LL | x.dummy_method((0)); | ^^^ help: remove them: `0` error: unnecessary parentheses - --> tests/ui/double_parens.rs:30:5 + --> tests/ui/double_parens.rs:35:5 | LL | ((1, 2)) | ^^^^^^^^ help: remove them: `(1, 2)` error: unnecessary parentheses - --> tests/ui/double_parens.rs:36:5 + --> tests/ui/double_parens.rs:41:5 | LL | (()) | ^^^^ help: remove them: `()` error: unnecessary parentheses - --> tests/ui/double_parens.rs:59:16 + --> tests/ui/double_parens.rs:64:16 | LL | assert_eq!(((1, 2)), (1, 2), "Error"); | ^^^^^^^^ help: remove them: `(1, 2)` error: unnecessary parentheses - --> tests/ui/double_parens.rs:84:16 + --> tests/ui/double_parens.rs:89:16 | LL | () => {((100))} | ^^^^^^^ help: remove them: `(100)` @@ -49,22 +49,55 @@ LL | bar!(); = note: this error originates in the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info) error: unnecessary parentheses - --> tests/ui/double_parens.rs:91:5 + --> tests/ui/double_parens.rs:96:5 | LL | ((vec![1, 2])); | ^^^^^^^^^^^^^^ help: remove them: `(vec![1, 2])` error: unnecessary parentheses - --> tests/ui/double_parens.rs:93:14 + --> tests/ui/double_parens.rs:98:14 | LL | dummy_fn((vec![1, 2])); | ^^^^^^^^^^^^ help: remove them: `vec![1, 2]` error: unnecessary parentheses - --> tests/ui/double_parens.rs:95:20 + --> tests/ui/double_parens.rs:100:20 | LL | x.dummy_method((vec![1, 2])); | ^^^^^^^^^^^^ help: remove them: `vec![1, 2]` -error: aborting due to 10 previous errors +error: unnecessary parentheses + --> tests/ui/double_parens.rs:110:21 + | +LL | let a = (()); + | ^^^^ help: remove them: `()` +... +LL | pub const E: InterruptMask = double_parens!((Self::OE), Self::BE, Self::PE, Self::FE); + | -------------------------------------------------------- in this macro invocation + | + = note: this error originates in the macro `double_parens` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: unnecessary parentheses + --> tests/ui/double_parens.rs:112:21 + | +LL | let b = ((5)); + | ^^^^^ help: remove them: `(5)` +... +LL | pub const E: InterruptMask = double_parens!((Self::OE), Self::BE, Self::PE, Self::FE); + | -------------------------------------------------------- in this macro invocation + | + = note: this error originates in the macro `double_parens` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: unnecessary parentheses + --> tests/ui/double_parens.rs:114:44 + | +LL | let c = std::convert::identity((5)); + | ^^^ help: remove them: `5` +... +LL | pub const E: InterruptMask = double_parens!((Self::OE), Self::BE, Self::PE, Self::FE); + | -------------------------------------------------------- in this macro invocation + | + = note: this error originates in the macro `double_parens` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 13 previous errors diff --git a/src/tools/clippy/tests/ui/multiple_unsafe_ops_per_block.rs b/src/tools/clippy/tests/ui/multiple_unsafe_ops_per_block.rs index 132673d5164ab..c1512ba3e2696 100644 --- a/src/tools/clippy/tests/ui/multiple_unsafe_ops_per_block.rs +++ b/src/tools/clippy/tests/ui/multiple_unsafe_ops_per_block.rs @@ -105,6 +105,14 @@ fn correct3() { } } +fn with_adjustment(f: &unsafe fn()) { + unsafe { + //~^ multiple_unsafe_ops_per_block + f(); + f(); + } +} + fn issue10064() { unsafe fn read_char_bad(ptr: *const u8) -> char { unsafe { char::from_u32_unchecked(*ptr.cast::()) } @@ -209,4 +217,99 @@ async fn issue13879() { } } +fn issue16076() { + #[derive(Clone, Copy)] + union U { + i: u32, + f: f32, + } + + let u = U { i: 0 }; + + // Taking a raw pointer to a place is safe since Rust 1.92 + unsafe { + _ = &raw const u.i; + _ = &raw const u.i; + } + + // Taking a reference to a union field is not safe + unsafe { + //~^ multiple_unsafe_ops_per_block + _ = &u.i; + _ = &u.i; + } + + // Check that we still check and lint the prefix of the raw pointer to a field access + #[expect(clippy::deref_addrof)] + unsafe { + //~^ multiple_unsafe_ops_per_block + _ = &raw const (*&raw const u).i; + _ = &raw const (*&raw const u).i; + } + + union V { + u: U, + } + + // Taking a raw pointer to a union field of an union field (etc.) is safe + let v = V { u }; + unsafe { + _ = &raw const v.u.i; + _ = &raw const v.u.i; + } + + // Check that unions in structs work properly as well + struct T { + u: U, + } + let t = T { u }; + unsafe { + _ = &raw const t.u.i; + _ = &raw const t.u.i; + } + + // As well as structs in unions + #[derive(Clone, Copy)] + struct X { + i: i32, + } + union Z { + x: X, + } + let z = Z { x: X { i: 0 } }; + unsafe { + _ = &raw const z.x.i; + _ = &raw const z.x.i; + } + + // If a field needs to be adjusted then it is accessed + struct S { + i: i32, + } + union W<'a> { + s: &'a S, + } + let s = S { i: 0 }; + let w = W { s: &s }; + unsafe { + //~^ multiple_unsafe_ops_per_block + _ = &raw const w.s.i; + _ = &raw const w.s.i; + } +} + +fn check_closures() { + unsafe fn apply(f: impl Fn()) { + todo!() + } + unsafe fn f(_x: i32) { + todo!() + } + + unsafe { + //~^ multiple_unsafe_ops_per_block + apply(|| f(0)); + } +} + fn main() {} diff --git a/src/tools/clippy/tests/ui/multiple_unsafe_ops_per_block.stderr b/src/tools/clippy/tests/ui/multiple_unsafe_ops_per_block.stderr index 922a464c6b6ef..63f7742b734b1 100644 --- a/src/tools/clippy/tests/ui/multiple_unsafe_ops_per_block.stderr +++ b/src/tools/clippy/tests/ui/multiple_unsafe_ops_per_block.stderr @@ -113,24 +113,45 @@ LL | asm!("nop"); | ^^^^^^^^^^^ error: this `unsafe` block contains 2 unsafe operations, expected only one - --> tests/ui/multiple_unsafe_ops_per_block.rs:110:9 + --> tests/ui/multiple_unsafe_ops_per_block.rs:109:5 + | +LL | / unsafe { +LL | | +LL | | f(); +LL | | f(); +LL | | } + | |_____^ + | +note: unsafe function call occurs here + --> tests/ui/multiple_unsafe_ops_per_block.rs:111:9 + | +LL | f(); + | ^^^ +note: unsafe function call occurs here + --> tests/ui/multiple_unsafe_ops_per_block.rs:112:9 + | +LL | f(); + | ^^^ + +error: this `unsafe` block contains 2 unsafe operations, expected only one + --> tests/ui/multiple_unsafe_ops_per_block.rs:118:9 | LL | unsafe { char::from_u32_unchecked(*ptr.cast::()) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:110:18 + --> tests/ui/multiple_unsafe_ops_per_block.rs:118:18 | LL | unsafe { char::from_u32_unchecked(*ptr.cast::()) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: raw pointer dereference occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:110:43 + --> tests/ui/multiple_unsafe_ops_per_block.rs:118:43 | LL | unsafe { char::from_u32_unchecked(*ptr.cast::()) } | ^^^^^^^^^^^^^^^^^^ error: this `unsafe` block contains 2 unsafe operations, expected only one - --> tests/ui/multiple_unsafe_ops_per_block.rs:131:9 + --> tests/ui/multiple_unsafe_ops_per_block.rs:139:9 | LL | / unsafe { LL | | @@ -140,18 +161,18 @@ LL | | } | |_________^ | note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:133:13 + --> tests/ui/multiple_unsafe_ops_per_block.rs:141:13 | LL | x(); | ^^^ note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:134:13 + --> tests/ui/multiple_unsafe_ops_per_block.rs:142:13 | LL | x(); | ^^^ error: this `unsafe` block contains 2 unsafe operations, expected only one - --> tests/ui/multiple_unsafe_ops_per_block.rs:143:13 + --> tests/ui/multiple_unsafe_ops_per_block.rs:151:13 | LL | / unsafe { LL | | @@ -161,18 +182,18 @@ LL | | } | |_____________^ | note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:145:17 + --> tests/ui/multiple_unsafe_ops_per_block.rs:153:17 | LL | T::X(); | ^^^^^^ note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:146:17 + --> tests/ui/multiple_unsafe_ops_per_block.rs:154:17 | LL | T::X(); | ^^^^^^ error: this `unsafe` block contains 2 unsafe operations, expected only one - --> tests/ui/multiple_unsafe_ops_per_block.rs:154:9 + --> tests/ui/multiple_unsafe_ops_per_block.rs:162:9 | LL | / unsafe { LL | | @@ -182,18 +203,18 @@ LL | | } | |_________^ | note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:156:13 + --> tests/ui/multiple_unsafe_ops_per_block.rs:164:13 | LL | x.0(); | ^^^^^ note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:157:13 + --> tests/ui/multiple_unsafe_ops_per_block.rs:165:13 | LL | x.0(); | ^^^^^ error: this `unsafe` block contains 2 unsafe operations, expected only one - --> tests/ui/multiple_unsafe_ops_per_block.rs:184:5 + --> tests/ui/multiple_unsafe_ops_per_block.rs:192:5 | LL | / unsafe { LL | | @@ -204,18 +225,18 @@ LL | | } | |_____^ | note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:186:9 + --> tests/ui/multiple_unsafe_ops_per_block.rs:194:9 | LL | not_very_safe(); | ^^^^^^^^^^^^^^^ note: modification of a mutable static occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:187:9 + --> tests/ui/multiple_unsafe_ops_per_block.rs:195:9 | LL | STATIC += 1; | ^^^^^^^^^^^ error: this `unsafe` block contains 2 unsafe operations, expected only one - --> tests/ui/multiple_unsafe_ops_per_block.rs:199:5 + --> tests/ui/multiple_unsafe_ops_per_block.rs:207:5 | LL | / unsafe { LL | | @@ -225,18 +246,18 @@ LL | | } | |_____^ | note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:201:9 + --> tests/ui/multiple_unsafe_ops_per_block.rs:209:9 | LL | not_very_safe(); | ^^^^^^^^^^^^^^^ note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:202:9 + --> tests/ui/multiple_unsafe_ops_per_block.rs:210:9 | LL | foo_unchecked().await; | ^^^^^^^^^^^^^^^ error: this `unsafe` block contains 2 unsafe operations, expected only one - --> tests/ui/multiple_unsafe_ops_per_block.rs:206:5 + --> tests/ui/multiple_unsafe_ops_per_block.rs:214:5 | LL | / unsafe { LL | | @@ -245,15 +266,98 @@ LL | | } | |_____^ | note: unsafe method call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:208:9 + --> tests/ui/multiple_unsafe_ops_per_block.rs:216:9 | LL | Some(foo_unchecked()).unwrap_unchecked().await; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: unsafe function call occurs here - --> tests/ui/multiple_unsafe_ops_per_block.rs:208:14 + --> tests/ui/multiple_unsafe_ops_per_block.rs:216:14 | LL | Some(foo_unchecked()).unwrap_unchecked().await; | ^^^^^^^^^^^^^^^ -error: aborting due to 11 previous errors +error: this `unsafe` block contains 2 unsafe operations, expected only one + --> tests/ui/multiple_unsafe_ops_per_block.rs:236:5 + | +LL | / unsafe { +LL | | +LL | | _ = &u.i; +LL | | _ = &u.i; +LL | | } + | |_____^ + | +note: union field access occurs here + --> tests/ui/multiple_unsafe_ops_per_block.rs:238:14 + | +LL | _ = &u.i; + | ^^^ +note: union field access occurs here + --> tests/ui/multiple_unsafe_ops_per_block.rs:239:14 + | +LL | _ = &u.i; + | ^^^ + +error: this `unsafe` block contains 2 unsafe operations, expected only one + --> tests/ui/multiple_unsafe_ops_per_block.rs:244:5 + | +LL | / unsafe { +LL | | +LL | | _ = &raw const (*&raw const u).i; +LL | | _ = &raw const (*&raw const u).i; +LL | | } + | |_____^ + | +note: raw pointer dereference occurs here + --> tests/ui/multiple_unsafe_ops_per_block.rs:246:24 + | +LL | _ = &raw const (*&raw const u).i; + | ^^^^^^^^^^^^^^^ +note: raw pointer dereference occurs here + --> tests/ui/multiple_unsafe_ops_per_block.rs:247:24 + | +LL | _ = &raw const (*&raw const u).i; + | ^^^^^^^^^^^^^^^ + +error: this `unsafe` block contains 2 unsafe operations, expected only one + --> tests/ui/multiple_unsafe_ops_per_block.rs:294:5 + | +LL | / unsafe { +LL | | +LL | | _ = &raw const w.s.i; +LL | | _ = &raw const w.s.i; +LL | | } + | |_____^ + | +note: union field access occurs here + --> tests/ui/multiple_unsafe_ops_per_block.rs:296:24 + | +LL | _ = &raw const w.s.i; + | ^^^ +note: union field access occurs here + --> tests/ui/multiple_unsafe_ops_per_block.rs:297:24 + | +LL | _ = &raw const w.s.i; + | ^^^ + +error: this `unsafe` block contains 2 unsafe operations, expected only one + --> tests/ui/multiple_unsafe_ops_per_block.rs:309:5 + | +LL | / unsafe { +LL | | +LL | | apply(|| f(0)); +LL | | } + | |_____^ + | +note: unsafe function call occurs here + --> tests/ui/multiple_unsafe_ops_per_block.rs:311:9 + | +LL | apply(|| f(0)); + | ^^^^^^^^^^^^^^ +note: unsafe function call occurs here + --> tests/ui/multiple_unsafe_ops_per_block.rs:311:18 + | +LL | apply(|| f(0)); + | ^^^^ + +error: aborting due to 16 previous errors diff --git a/src/tools/clippy/tests/ui/replace_box.fixed b/src/tools/clippy/tests/ui/replace_box.fixed index 58c8ed1691d77..e3fc7190a9c9c 100644 --- a/src/tools/clippy/tests/ui/replace_box.fixed +++ b/src/tools/clippy/tests/ui/replace_box.fixed @@ -70,3 +70,74 @@ fn main() { let bb: Box; bb = Default::default(); } + +fn issue15951() { + struct Foo { + inner: String, + } + + fn embedded_body() { + let mut x = Box::new(()); + let y = x; + x = Box::new(()); + + let mut x = Box::new(Foo { inner: String::new() }); + let y = x.inner; + *x = Foo { inner: String::new() }; + //~^ replace_box + } + + let mut x = Box::new(Foo { inner: String::new() }); + let in_closure = || { + *x = Foo { inner: String::new() }; + //~^ replace_box + }; +} + +static R: fn(&mut Box) = |x| **x = String::new(); +//~^ replace_box + +fn field() { + struct T { + content: String, + } + + impl T { + fn new() -> Self { + Self { content: String::new() } + } + } + + struct S { + b: Box, + } + + let mut s = S { b: Box::new(T::new()) }; + let _b = s.b; + s.b = Box::new(T::new()); + + // Interestingly, the lint and fix are valid here as `s.b` is not really moved + let mut s = S { b: Box::new(T::new()) }; + _ = s.b; + *s.b = T::new(); + //~^ replace_box + + let mut s = S { b: Box::new(T::new()) }; + *s.b = T::new(); + //~^ replace_box + + struct Q(Box); + let mut q = Q(Box::new(T::new())); + let _b = q.0; + q.0 = Box::new(T::new()); + + let mut q = Q(Box::new(T::new())); + _ = q.0; + *q.0 = T::new(); + //~^ replace_box + + // This one is a false negative, but it will need MIR analysis to work properly + let mut x = Box::new(String::new()); + x = Box::new(String::new()); + x; +} diff --git a/src/tools/clippy/tests/ui/replace_box.rs b/src/tools/clippy/tests/ui/replace_box.rs index e1fb223e4f211..1d5ca1b24994f 100644 --- a/src/tools/clippy/tests/ui/replace_box.rs +++ b/src/tools/clippy/tests/ui/replace_box.rs @@ -70,3 +70,74 @@ fn main() { let bb: Box; bb = Default::default(); } + +fn issue15951() { + struct Foo { + inner: String, + } + + fn embedded_body() { + let mut x = Box::new(()); + let y = x; + x = Box::new(()); + + let mut x = Box::new(Foo { inner: String::new() }); + let y = x.inner; + x = Box::new(Foo { inner: String::new() }); + //~^ replace_box + } + + let mut x = Box::new(Foo { inner: String::new() }); + let in_closure = || { + x = Box::new(Foo { inner: String::new() }); + //~^ replace_box + }; +} + +static R: fn(&mut Box) = |x| *x = Box::new(String::new()); +//~^ replace_box + +fn field() { + struct T { + content: String, + } + + impl T { + fn new() -> Self { + Self { content: String::new() } + } + } + + struct S { + b: Box, + } + + let mut s = S { b: Box::new(T::new()) }; + let _b = s.b; + s.b = Box::new(T::new()); + + // Interestingly, the lint and fix are valid here as `s.b` is not really moved + let mut s = S { b: Box::new(T::new()) }; + _ = s.b; + s.b = Box::new(T::new()); + //~^ replace_box + + let mut s = S { b: Box::new(T::new()) }; + s.b = Box::new(T::new()); + //~^ replace_box + + struct Q(Box); + let mut q = Q(Box::new(T::new())); + let _b = q.0; + q.0 = Box::new(T::new()); + + let mut q = Q(Box::new(T::new())); + _ = q.0; + q.0 = Box::new(T::new()); + //~^ replace_box + + // This one is a false negative, but it will need MIR analysis to work properly + let mut x = Box::new(String::new()); + x = Box::new(String::new()); + x; +} diff --git a/src/tools/clippy/tests/ui/replace_box.stderr b/src/tools/clippy/tests/ui/replace_box.stderr index 7d9c85da17314..4b7bd4a0eeaeb 100644 --- a/src/tools/clippy/tests/ui/replace_box.stderr +++ b/src/tools/clippy/tests/ui/replace_box.stderr @@ -48,5 +48,53 @@ LL | b = Box::new(mac!(three)); | = note: this creates a needless allocation -error: aborting due to 6 previous errors +error: creating a new box + --> tests/ui/replace_box.rs:86:9 + | +LL | x = Box::new(Foo { inner: String::new() }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `*x = Foo { inner: String::new() }` + | + = note: this creates a needless allocation + +error: creating a new box + --> tests/ui/replace_box.rs:92:9 + | +LL | x = Box::new(Foo { inner: String::new() }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `*x = Foo { inner: String::new() }` + | + = note: this creates a needless allocation + +error: creating a new box + --> tests/ui/replace_box.rs:97:38 + | +LL | static R: fn(&mut Box) = |x| *x = Box::new(String::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `**x = String::new()` + | + = note: this creates a needless allocation + +error: creating a new box + --> tests/ui/replace_box.rs:122:5 + | +LL | s.b = Box::new(T::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `*s.b = T::new()` + | + = note: this creates a needless allocation + +error: creating a new box + --> tests/ui/replace_box.rs:126:5 + | +LL | s.b = Box::new(T::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `*s.b = T::new()` + | + = note: this creates a needless allocation + +error: creating a new box + --> tests/ui/replace_box.rs:136:5 + | +LL | q.0 = Box::new(T::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace existing content with inner value instead: `*q.0 = T::new()` + | + = note: this creates a needless allocation + +error: aborting due to 12 previous errors