Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
37 changes: 32 additions & 5 deletions src/tools/clippy/clippy_lints/src/double_parens.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -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
}
}
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
115 changes: 78 additions & 37 deletions src/tools/clippy/clippy_lints/src/multiple_unsafe_ops_per_block.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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));
}
},

Expand All @@ -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));
}
},

Expand All @@ -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
}
}
99 changes: 94 additions & 5 deletions src/tools/clippy/clippy_lints/src/replace_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -33,17 +39,57 @@ declare_clippy_lint! {
perf,
"assigning a newly created box to `Box<T>` is inefficient"
}
declare_lint_pass!(ReplaceBox => [REPLACE_BOX]);

#[derive(Default)]
pub struct ReplaceBox {
consumed_locals: FxHashSet<HirId>,
loaded_bodies: SmallVec<[BodyId; 2]>,
}

impl ReplaceBox {
fn get_consumed_locals(&mut self, cx: &LateContext<'_>) -> &FxHashSet<HirId> {
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, &[])
Expand Down Expand Up @@ -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<HirId>,
}

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<Symbol>);

/// If `expr` is a local variable with optional field accesses, return it.
fn local_base(expr: &Expr<'_>) -> Option<IdFields> {
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,
}
}
12 changes: 12 additions & 0 deletions src/tools/clippy/tests/ui/auxiliary/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}};
}
Loading
Loading