Auto merge of #10774 - c410-f3r:lock-1, r=Jarcho
[significant_drop_tightening] Fix #10413 Fix #10413 This is quite a rewrite that unfortunately took a large amount of time. I tried my best to comment what is going on to easy review but feel free to ask any question. The problem basically is that the current algorithm is only taking into consideration single blocks which means that things like the following don't work or show unpredictable results. ```rust let mutex = Mutex::new(1); { let lock = mutex.lock().unwrap(); { let _ = *lock; } } ``` The solve the issue, each path that refers a lock is now being tracked individually. ``` changelog: [`significant_drop_tightening`]: Lift the restriction of only considerate single blocks ```
This commit is contained in:
commit
73f14176e3
@ -1,10 +1,10 @@
|
|||||||
use clippy_utils::{
|
use clippy_utils::{
|
||||||
diagnostics::span_lint_and_then,
|
diagnostics::span_lint_and_then,
|
||||||
get_attr,
|
expr_or_init, get_attr, path_to_local,
|
||||||
source::{indent_of, snippet},
|
source::{indent_of, snippet},
|
||||||
};
|
};
|
||||||
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
|
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
|
||||||
use rustc_errors::{Applicability, Diagnostic};
|
use rustc_errors::Applicability;
|
||||||
use rustc_hir::{
|
use rustc_hir::{
|
||||||
self as hir,
|
self as hir,
|
||||||
intravisit::{walk_expr, Visitor},
|
intravisit::{walk_expr, Visitor},
|
||||||
@ -13,6 +13,7 @@
|
|||||||
use rustc_middle::ty::{subst::GenericArgKind, Ty, TypeAndMut};
|
use rustc_middle::ty::{subst::GenericArgKind, Ty, TypeAndMut};
|
||||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||||
use rustc_span::{symbol::Ident, Span, DUMMY_SP};
|
use rustc_span::{symbol::Ident, Span, DUMMY_SP};
|
||||||
|
use std::borrow::Cow;
|
||||||
|
|
||||||
declare_clippy_lint! {
|
declare_clippy_lint! {
|
||||||
/// ### What it does
|
/// ### What it does
|
||||||
@ -56,108 +57,58 @@
|
|||||||
|
|
||||||
#[derive(Default)]
|
#[derive(Default)]
|
||||||
pub struct SignificantDropTightening<'tcx> {
|
pub struct SignificantDropTightening<'tcx> {
|
||||||
|
apas: FxIndexMap<hir::HirId, AuxParamsAttr>,
|
||||||
/// Auxiliary structure used to avoid having to verify the same type multiple times.
|
/// Auxiliary structure used to avoid having to verify the same type multiple times.
|
||||||
seen_types: FxHashSet<Ty<'tcx>>,
|
seen_types: FxHashSet<Ty<'tcx>>,
|
||||||
type_cache: FxHashMap<Ty<'tcx>, bool>,
|
type_cache: FxHashMap<Ty<'tcx>, bool>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'tcx> SignificantDropTightening<'tcx> {
|
impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> {
|
||||||
/// Unifies the statements of a block with its return expression.
|
fn check_fn(
|
||||||
fn all_block_stmts<'ret, 'rslt, 'stmts>(
|
|
||||||
block_stmts: &'stmts [hir::Stmt<'tcx>],
|
|
||||||
dummy_ret_stmt: Option<&'ret hir::Stmt<'tcx>>,
|
|
||||||
) -> impl Iterator<Item = &'rslt hir::Stmt<'tcx>>
|
|
||||||
where
|
|
||||||
'ret: 'rslt,
|
|
||||||
'stmts: 'rslt,
|
|
||||||
{
|
|
||||||
block_stmts.iter().chain(dummy_ret_stmt)
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Searches for at least one statement that could slow down the release of a significant drop.
|
|
||||||
fn at_least_one_stmt_is_expensive<'stmt>(stmts: impl Iterator<Item = &'stmt hir::Stmt<'tcx>>) -> bool
|
|
||||||
where
|
|
||||||
'tcx: 'stmt,
|
|
||||||
{
|
|
||||||
for stmt in stmts {
|
|
||||||
match stmt.kind {
|
|
||||||
hir::StmtKind::Expr(expr) if let hir::ExprKind::Path(_) = expr.kind => {}
|
|
||||||
hir::StmtKind::Local(local) if let Some(expr) = local.init
|
|
||||||
&& let hir::ExprKind::Path(_) = expr.kind => {},
|
|
||||||
_ => return true
|
|
||||||
};
|
|
||||||
}
|
|
||||||
false
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Verifies if the expression is of type `drop(some_lock_path)` to assert that the temporary
|
|
||||||
/// is already being dropped before the end of its scope.
|
|
||||||
fn has_drop(expr: &'tcx hir::Expr<'_>, init_bind_ident: Ident) -> bool {
|
|
||||||
if let hir::ExprKind::Call(fun, args) = expr.kind
|
|
||||||
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, fun_path)) = &fun.kind
|
|
||||||
&& let [fun_ident, ..] = fun_path.segments
|
|
||||||
&& fun_ident.ident.name == rustc_span::sym::drop
|
|
||||||
&& let [first_arg, ..] = args
|
|
||||||
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, arg_path)) = &first_arg.kind
|
|
||||||
&& let [first_arg_ps, .. ] = arg_path.segments
|
|
||||||
{
|
|
||||||
first_arg_ps.ident == init_bind_ident
|
|
||||||
}
|
|
||||||
else {
|
|
||||||
false
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Tries to find types marked with `#[has_significant_drop]` of an expression `expr` that is
|
|
||||||
/// originated from `stmt` and then performs common logic on `sdap`.
|
|
||||||
fn modify_sdap_if_sig_drop_exists(
|
|
||||||
&mut self,
|
&mut self,
|
||||||
cx: &LateContext<'tcx>,
|
cx: &LateContext<'tcx>,
|
||||||
expr: &'tcx hir::Expr<'_>,
|
_: hir::intravisit::FnKind<'_>,
|
||||||
idx: usize,
|
_: &hir::FnDecl<'_>,
|
||||||
sdap: &mut SigDropAuxParams,
|
body: &'tcx hir::Body<'_>,
|
||||||
stmt: &hir::Stmt<'_>,
|
_: Span,
|
||||||
cb: impl Fn(&mut SigDropAuxParams),
|
_: hir::def_id::LocalDefId,
|
||||||
) {
|
) {
|
||||||
let mut sig_drop_finder = SigDropFinder::new(cx, &mut self.seen_types, &mut self.type_cache);
|
self.apas.clear();
|
||||||
sig_drop_finder.visit_expr(expr);
|
let initial_dummy_stmt = dummy_stmt_expr(body.value);
|
||||||
if sig_drop_finder.has_sig_drop {
|
let mut ap = AuxParams::new(&mut self.apas, &initial_dummy_stmt);
|
||||||
cb(sdap);
|
StmtsChecker::new(&mut ap, cx, &mut self.seen_types, &mut self.type_cache).visit_body(body);
|
||||||
if sdap.number_of_stmts > 0 {
|
for apa in ap.apas.values() {
|
||||||
sdap.last_use_stmt_idx = idx;
|
if apa.counter <= 1 || !apa.has_expensive_expr_after_last_attr {
|
||||||
sdap.last_use_stmt_span = stmt.span;
|
continue;
|
||||||
if let hir::ExprKind::MethodCall(_, _, _, span) = expr.kind {
|
|
||||||
sdap.last_use_method_span = span;
|
|
||||||
}
|
}
|
||||||
}
|
span_lint_and_then(
|
||||||
sdap.number_of_stmts = sdap.number_of_stmts.wrapping_add(1);
|
cx,
|
||||||
}
|
SIGNIFICANT_DROP_TIGHTENING,
|
||||||
}
|
apa.first_bind_ident.span,
|
||||||
|
"temporary with significant `Drop` can be early dropped",
|
||||||
/// Shows generic overall messages as well as specialized messages depending on the usage.
|
|diag| {
|
||||||
fn set_suggestions(cx: &LateContext<'tcx>, block_span: Span, diag: &mut Diagnostic, sdap: &SigDropAuxParams) {
|
match apa.counter {
|
||||||
match sdap.number_of_stmts {
|
|
||||||
0 | 1 => {},
|
0 | 1 => {},
|
||||||
2 => {
|
2 => {
|
||||||
let indent = " ".repeat(indent_of(cx, sdap.last_use_stmt_span).unwrap_or(0));
|
let indent = " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0));
|
||||||
let init_method = snippet(cx, sdap.init_method_span, "..");
|
let init_method = snippet(cx, apa.first_method_span, "..");
|
||||||
let usage_method = snippet(cx, sdap.last_use_method_span, "..");
|
let usage_method = snippet(cx, apa.last_method_span, "..");
|
||||||
let stmt = if let Some(last_use_bind_span) = sdap.last_use_bind_span {
|
let stmt = if apa.last_bind_ident == Ident::empty() {
|
||||||
|
format!("\n{indent}{init_method}.{usage_method};")
|
||||||
|
} else {
|
||||||
format!(
|
format!(
|
||||||
"\n{indent}let {} = {init_method}.{usage_method};",
|
"\n{indent}let {} = {init_method}.{usage_method};",
|
||||||
snippet(cx, last_use_bind_span, ".."),
|
snippet(cx, apa.last_bind_ident.span, ".."),
|
||||||
)
|
)
|
||||||
} else {
|
|
||||||
format!("\n{indent}{init_method}.{usage_method};")
|
|
||||||
};
|
};
|
||||||
diag.span_suggestion_verbose(
|
diag.span_suggestion_verbose(
|
||||||
sdap.init_stmt_span,
|
apa.first_stmt_span,
|
||||||
"merge the temporary construction with its single usage",
|
"merge the temporary construction with its single usage",
|
||||||
stmt,
|
stmt,
|
||||||
Applicability::MaybeIncorrect,
|
Applicability::MaybeIncorrect,
|
||||||
);
|
);
|
||||||
diag.span_suggestion(
|
diag.span_suggestion(
|
||||||
sdap.last_use_stmt_span,
|
apa.last_stmt_span,
|
||||||
"remove separated single usage",
|
"remove separated single usage",
|
||||||
"",
|
"",
|
||||||
Applicability::MaybeIncorrect,
|
Applicability::MaybeIncorrect,
|
||||||
@ -165,12 +116,12 @@ fn set_suggestions(cx: &LateContext<'tcx>, block_span: Span, diag: &mut Diagnost
|
|||||||
},
|
},
|
||||||
_ => {
|
_ => {
|
||||||
diag.span_suggestion(
|
diag.span_suggestion(
|
||||||
sdap.last_use_stmt_span.shrink_to_hi(),
|
apa.last_stmt_span.shrink_to_hi(),
|
||||||
"drop the temporary after the end of its last usage",
|
"drop the temporary after the end of its last usage",
|
||||||
format!(
|
format!(
|
||||||
"\n{}drop({});",
|
"\n{}drop({});",
|
||||||
" ".repeat(indent_of(cx, sdap.last_use_stmt_span).unwrap_or(0)),
|
" ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)),
|
||||||
sdap.init_bind_ident
|
apa.first_bind_ident
|
||||||
),
|
),
|
||||||
Applicability::MaybeIncorrect,
|
Applicability::MaybeIncorrect,
|
||||||
);
|
);
|
||||||
@ -178,133 +129,30 @@ fn set_suggestions(cx: &LateContext<'tcx>, block_span: Span, diag: &mut Diagnost
|
|||||||
}
|
}
|
||||||
diag.note("this might lead to unnecessary resource contention");
|
diag.note("this might lead to unnecessary resource contention");
|
||||||
diag.span_label(
|
diag.span_label(
|
||||||
block_span,
|
apa.first_block_span,
|
||||||
format!(
|
format!(
|
||||||
"temporary `{}` is currently being dropped at the end of its contained scope",
|
"temporary `{}` is currently being dropped at the end of its contained scope",
|
||||||
sdap.init_bind_ident
|
apa.first_bind_ident
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> {
|
|
||||||
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
|
|
||||||
let dummy_ret_stmt = block.expr.map(|expr| hir::Stmt {
|
|
||||||
hir_id: hir::HirId::INVALID,
|
|
||||||
kind: hir::StmtKind::Expr(expr),
|
|
||||||
span: DUMMY_SP,
|
|
||||||
});
|
|
||||||
let mut sdap = SigDropAuxParams::default();
|
|
||||||
for (idx, stmt) in Self::all_block_stmts(block.stmts, dummy_ret_stmt.as_ref()).enumerate() {
|
|
||||||
match stmt.kind {
|
|
||||||
hir::StmtKind::Expr(expr) => self.modify_sdap_if_sig_drop_exists(
|
|
||||||
cx,
|
|
||||||
expr,
|
|
||||||
idx,
|
|
||||||
&mut sdap,
|
|
||||||
stmt,
|
|
||||||
|_| {}
|
|
||||||
),
|
|
||||||
hir::StmtKind::Local(local) if let Some(expr) = local.init => self.modify_sdap_if_sig_drop_exists(
|
|
||||||
cx,
|
|
||||||
expr,
|
|
||||||
idx,
|
|
||||||
&mut sdap,
|
|
||||||
stmt,
|
|
||||||
|local_sdap| {
|
|
||||||
if local_sdap.number_of_stmts == 0 {
|
|
||||||
if let hir::PatKind::Binding(_, _, ident, _) = local.pat.kind {
|
|
||||||
local_sdap.init_bind_ident = ident;
|
|
||||||
}
|
|
||||||
if let hir::ExprKind::MethodCall(_, local_expr, _, span) = expr.kind {
|
|
||||||
local_sdap.init_method_span = local_expr.span.to(span);
|
|
||||||
}
|
|
||||||
local_sdap.init_stmt_span = stmt.span;
|
|
||||||
}
|
|
||||||
else if let hir::PatKind::Binding(_, _, ident, _) = local.pat.kind {
|
|
||||||
local_sdap.last_use_bind_span = Some(ident.span);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
),
|
|
||||||
hir::StmtKind::Semi(expr) => {
|
|
||||||
if Self::has_drop(expr, sdap.init_bind_ident) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
self.modify_sdap_if_sig_drop_exists(cx, expr, idx, &mut sdap, stmt, |_| {});
|
|
||||||
},
|
|
||||||
_ => {}
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
let idx = sdap.last_use_stmt_idx.wrapping_add(1);
|
|
||||||
let stmts_after_last_use = Self::all_block_stmts(block.stmts, dummy_ret_stmt.as_ref()).skip(idx);
|
|
||||||
if sdap.number_of_stmts > 1 && Self::at_least_one_stmt_is_expensive(stmts_after_last_use) {
|
|
||||||
span_lint_and_then(
|
|
||||||
cx,
|
|
||||||
SIGNIFICANT_DROP_TIGHTENING,
|
|
||||||
sdap.init_bind_ident.span,
|
|
||||||
"temporary with significant `Drop` can be early dropped",
|
|
||||||
|diag| {
|
|
||||||
Self::set_suggestions(cx, block.span, diag, &sdap);
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Auxiliary parameters used on each block check.
|
/// Checks the existence of the `#[has_significant_drop]` attribute.
|
||||||
struct SigDropAuxParams {
|
struct AttrChecker<'cx, 'others, 'tcx> {
|
||||||
/// The binding or variable that references the initial construction of the type marked with
|
|
||||||
/// `#[has_significant_drop]`.
|
|
||||||
init_bind_ident: Ident,
|
|
||||||
/// Similar to `init_bind_ident` but encompasses the right-hand method call.
|
|
||||||
init_method_span: Span,
|
|
||||||
/// Similar to `init_bind_ident` but encompasses the whole contained statement.
|
|
||||||
init_stmt_span: Span,
|
|
||||||
|
|
||||||
/// The last visited binding or variable span within a block that had any referenced inner type
|
|
||||||
/// marked with `#[has_significant_drop]`.
|
|
||||||
last_use_bind_span: Option<Span>,
|
|
||||||
/// Index of the last visited statement within a block that had any referenced inner type
|
|
||||||
/// marked with `#[has_significant_drop]`.
|
|
||||||
last_use_stmt_idx: usize,
|
|
||||||
/// Similar to `last_use_bind_span` but encompasses the whole contained statement.
|
|
||||||
last_use_stmt_span: Span,
|
|
||||||
/// Similar to `last_use_bind_span` but encompasses the right-hand method call.
|
|
||||||
last_use_method_span: Span,
|
|
||||||
|
|
||||||
/// Total number of statements within a block that have any referenced inner type marked with
|
|
||||||
/// `#[has_significant_drop]`.
|
|
||||||
number_of_stmts: usize,
|
|
||||||
}
|
|
||||||
|
|
||||||
impl Default for SigDropAuxParams {
|
|
||||||
fn default() -> Self {
|
|
||||||
Self {
|
|
||||||
init_bind_ident: Ident::empty(),
|
|
||||||
init_method_span: DUMMY_SP,
|
|
||||||
init_stmt_span: DUMMY_SP,
|
|
||||||
last_use_bind_span: None,
|
|
||||||
last_use_method_span: DUMMY_SP,
|
|
||||||
last_use_stmt_idx: 0,
|
|
||||||
last_use_stmt_span: DUMMY_SP,
|
|
||||||
number_of_stmts: 0,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Checks the existence of the `#[has_significant_drop]` attribute
|
|
||||||
struct SigDropChecker<'cx, 'sdt, 'tcx> {
|
|
||||||
cx: &'cx LateContext<'tcx>,
|
cx: &'cx LateContext<'tcx>,
|
||||||
seen_types: &'sdt mut FxHashSet<Ty<'tcx>>,
|
seen_types: &'others mut FxHashSet<Ty<'tcx>>,
|
||||||
type_cache: &'sdt mut FxHashMap<Ty<'tcx>, bool>,
|
type_cache: &'others mut FxHashMap<Ty<'tcx>, bool>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'cx, 'sdt, 'tcx> SigDropChecker<'cx, 'sdt, 'tcx> {
|
impl<'cx, 'others, 'tcx> AttrChecker<'cx, 'others, 'tcx> {
|
||||||
pub(crate) fn new(
|
pub(crate) fn new(
|
||||||
cx: &'cx LateContext<'tcx>,
|
cx: &'cx LateContext<'tcx>,
|
||||||
seen_types: &'sdt mut FxHashSet<Ty<'tcx>>,
|
seen_types: &'others mut FxHashSet<Ty<'tcx>>,
|
||||||
type_cache: &'sdt mut FxHashMap<Ty<'tcx>, bool>,
|
type_cache: &'others mut FxHashMap<Ty<'tcx>, bool>,
|
||||||
) -> Self {
|
) -> Self {
|
||||||
seen_types.clear();
|
seen_types.clear();
|
||||||
Self {
|
Self {
|
||||||
@ -314,7 +162,17 @@ pub(crate) fn new(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn has_sig_drop_attr_uncached(&mut self, ty: Ty<'tcx>) -> bool {
|
fn has_sig_drop_attr(&mut self, ty: Ty<'tcx>) -> bool {
|
||||||
|
// The borrow checker prevents us from using something fancier like or_insert_with.
|
||||||
|
if let Some(ty) = self.type_cache.get(&ty) {
|
||||||
|
return *ty;
|
||||||
|
}
|
||||||
|
let value = self.has_sig_drop_attr_uncached(ty);
|
||||||
|
self.type_cache.insert(ty, value);
|
||||||
|
value
|
||||||
|
}
|
||||||
|
|
||||||
|
fn has_sig_drop_attr_uncached(&mut self, ty: Ty<'tcx>) -> bool {
|
||||||
if let Some(adt) = ty.ty_adt_def() {
|
if let Some(adt) = ty.ty_adt_def() {
|
||||||
let mut iter = get_attr(
|
let mut iter = get_attr(
|
||||||
self.cx.sess(),
|
self.cx.sess(),
|
||||||
@ -350,73 +208,244 @@ pub(crate) fn has_sig_drop_attr_uncached(&mut self, ty: Ty<'tcx>) -> bool {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn has_sig_drop_attr(&mut self, ty: Ty<'tcx>) -> bool {
|
|
||||||
// The borrow checker prevents us from using something fancier like or_insert_with.
|
|
||||||
if let Some(ty) = self.type_cache.get(&ty) {
|
|
||||||
return *ty;
|
|
||||||
}
|
|
||||||
let value = self.has_sig_drop_attr_uncached(ty);
|
|
||||||
self.type_cache.insert(ty, value);
|
|
||||||
value
|
|
||||||
}
|
|
||||||
|
|
||||||
fn has_seen_ty(&mut self, ty: Ty<'tcx>) -> bool {
|
fn has_seen_ty(&mut self, ty: Ty<'tcx>) -> bool {
|
||||||
!self.seen_types.insert(ty)
|
!self.seen_types.insert(ty)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Performs recursive calls to find any inner type marked with `#[has_significant_drop]`.
|
struct StmtsChecker<'ap, 'lc, 'others, 'stmt, 'tcx> {
|
||||||
struct SigDropFinder<'cx, 'sdt, 'tcx> {
|
ap: &'ap mut AuxParams<'others, 'stmt, 'tcx>,
|
||||||
cx: &'cx LateContext<'tcx>,
|
cx: &'lc LateContext<'tcx>,
|
||||||
has_sig_drop: bool,
|
seen_types: &'others mut FxHashSet<Ty<'tcx>>,
|
||||||
sig_drop_checker: SigDropChecker<'cx, 'sdt, 'tcx>,
|
type_cache: &'others mut FxHashMap<Ty<'tcx>, bool>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'cx, 'sdt, 'tcx> SigDropFinder<'cx, 'sdt, 'tcx> {
|
impl<'ap, 'lc, 'others, 'stmt, 'tcx> StmtsChecker<'ap, 'lc, 'others, 'stmt, 'tcx> {
|
||||||
fn new(
|
fn new(
|
||||||
cx: &'cx LateContext<'tcx>,
|
ap: &'ap mut AuxParams<'others, 'stmt, 'tcx>,
|
||||||
seen_types: &'sdt mut FxHashSet<Ty<'tcx>>,
|
cx: &'lc LateContext<'tcx>,
|
||||||
type_cache: &'sdt mut FxHashMap<Ty<'tcx>, bool>,
|
seen_types: &'others mut FxHashSet<Ty<'tcx>>,
|
||||||
|
type_cache: &'others mut FxHashMap<Ty<'tcx>, bool>,
|
||||||
) -> Self {
|
) -> Self {
|
||||||
Self {
|
Self {
|
||||||
|
ap,
|
||||||
cx,
|
cx,
|
||||||
has_sig_drop: false,
|
seen_types,
|
||||||
sig_drop_checker: SigDropChecker::new(cx, seen_types, type_cache),
|
type_cache,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn manage_has_expensive_expr_after_last_attr(&mut self) {
|
||||||
|
let has_expensive_stmt = match self.ap.curr_stmt.kind {
|
||||||
|
hir::StmtKind::Expr(expr) if !is_expensive_expr(expr) => false,
|
||||||
|
hir::StmtKind::Local(local) if let Some(expr) = local.init
|
||||||
|
&& let hir::ExprKind::Path(_) = expr.kind => false,
|
||||||
|
_ => true
|
||||||
|
};
|
||||||
|
if has_expensive_stmt {
|
||||||
|
for apa in self.ap.apas.values_mut() {
|
||||||
|
let last_stmt_is_not_dummy = apa.last_stmt_span != DUMMY_SP;
|
||||||
|
let last_stmt_is_not_curr = self.ap.curr_stmt.span != apa.last_stmt_span;
|
||||||
|
let block_equals_curr = self.ap.curr_block_hir_id == apa.first_block_hir_id;
|
||||||
|
let block_is_ancestor = self
|
||||||
|
.cx
|
||||||
|
.tcx
|
||||||
|
.hir()
|
||||||
|
.parent_iter(self.ap.curr_block_hir_id)
|
||||||
|
.any(|(id, _)| id == apa.first_block_hir_id);
|
||||||
|
if last_stmt_is_not_dummy && last_stmt_is_not_curr && (block_equals_curr || block_is_ancestor) {
|
||||||
|
apa.has_expensive_expr_after_last_attr = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'cx, 'sdt, 'tcx> Visitor<'tcx> for SigDropFinder<'cx, 'sdt, 'tcx> {
|
impl<'ap, 'lc, 'others, 'stmt, 'tcx> Visitor<'tcx> for StmtsChecker<'ap, 'lc, 'others, 'stmt, 'tcx> {
|
||||||
fn visit_expr(&mut self, ex: &'tcx hir::Expr<'_>) {
|
fn visit_block(&mut self, block: &'tcx hir::Block<'tcx>) {
|
||||||
if self
|
self.ap.curr_block_hir_id = block.hir_id;
|
||||||
.sig_drop_checker
|
self.ap.curr_block_span = block.span;
|
||||||
.has_sig_drop_attr(self.cx.typeck_results().expr_ty(ex))
|
for stmt in block.stmts {
|
||||||
{
|
self.ap.curr_stmt = Cow::Borrowed(stmt);
|
||||||
self.has_sig_drop = true;
|
self.visit_stmt(stmt);
|
||||||
return;
|
self.ap.curr_block_hir_id = block.hir_id;
|
||||||
|
self.ap.curr_block_span = block.span;
|
||||||
|
self.manage_has_expensive_expr_after_last_attr();
|
||||||
|
}
|
||||||
|
if let Some(expr) = block.expr {
|
||||||
|
self.ap.curr_stmt = Cow::Owned(dummy_stmt_expr(expr));
|
||||||
|
self.visit_expr(expr);
|
||||||
|
self.ap.curr_block_hir_id = block.hir_id;
|
||||||
|
self.ap.curr_block_span = block.span;
|
||||||
|
self.manage_has_expensive_expr_after_last_attr();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
match ex.kind {
|
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
|
||||||
hir::ExprKind::MethodCall(_, expr, ..) => {
|
let modify_apa_params = |apa: &mut AuxParamsAttr| {
|
||||||
self.visit_expr(expr);
|
apa.counter = apa.counter.wrapping_add(1);
|
||||||
|
apa.has_expensive_expr_after_last_attr = false;
|
||||||
|
};
|
||||||
|
let mut ac = AttrChecker::new(self.cx, self.seen_types, self.type_cache);
|
||||||
|
if ac.has_sig_drop_attr(self.cx.typeck_results().expr_ty(expr)) {
|
||||||
|
if let hir::StmtKind::Local(local) = self.ap.curr_stmt.kind
|
||||||
|
&& let hir::PatKind::Binding(_, hir_id, ident, _) = local.pat.kind
|
||||||
|
&& !self.ap.apas.contains_key(&hir_id)
|
||||||
|
&& {
|
||||||
|
if let Some(local_hir_id) = path_to_local(expr) {
|
||||||
|
local_hir_id == hir_id
|
||||||
|
}
|
||||||
|
else {
|
||||||
|
true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
{
|
||||||
|
let mut apa = AuxParamsAttr {
|
||||||
|
first_bind_ident: ident,
|
||||||
|
first_block_hir_id: self.ap.curr_block_hir_id,
|
||||||
|
first_block_span: self.ap.curr_block_span,
|
||||||
|
first_method_span: {
|
||||||
|
let expr_or_init = expr_or_init(self.cx, expr);
|
||||||
|
if let hir::ExprKind::MethodCall(_, local_expr, _, span) = expr_or_init.kind {
|
||||||
|
local_expr.span.to(span)
|
||||||
|
}
|
||||||
|
else {
|
||||||
|
expr_or_init.span
|
||||||
|
}
|
||||||
},
|
},
|
||||||
hir::ExprKind::Array(..)
|
first_stmt_span: self.ap.curr_stmt.span,
|
||||||
| hir::ExprKind::Assign(..)
|
..Default::default()
|
||||||
| hir::ExprKind::AssignOp(..)
|
};
|
||||||
| hir::ExprKind::Binary(..)
|
modify_apa_params(&mut apa);
|
||||||
| hir::ExprKind::Call(..)
|
let _ = self.ap.apas.insert(hir_id, apa);
|
||||||
| hir::ExprKind::Field(..)
|
} else {
|
||||||
| hir::ExprKind::If(..)
|
let Some(hir_id) = path_to_local(expr) else { return; };
|
||||||
| hir::ExprKind::Index(..)
|
let Some(apa) = self.ap.apas.get_mut(&hir_id) else { return; };
|
||||||
| hir::ExprKind::Match(..)
|
match self.ap.curr_stmt.kind {
|
||||||
| hir::ExprKind::Repeat(..)
|
hir::StmtKind::Local(local) => {
|
||||||
| hir::ExprKind::Ret(..)
|
if let hir::PatKind::Binding(_, _, ident, _) = local.pat.kind {
|
||||||
| hir::ExprKind::Tup(..)
|
apa.last_bind_ident = ident;
|
||||||
| hir::ExprKind::Unary(..)
|
}
|
||||||
| hir::ExprKind::Yield(..) => {
|
if let Some(local_init) = local.init
|
||||||
walk_expr(self, ex);
|
&& let hir::ExprKind::MethodCall(_, _, _, span) = local_init.kind
|
||||||
|
{
|
||||||
|
apa.last_method_span = span;
|
||||||
|
}
|
||||||
|
},
|
||||||
|
hir::StmtKind::Semi(expr) => {
|
||||||
|
if has_drop(expr, &apa.first_bind_ident) {
|
||||||
|
apa.has_expensive_expr_after_last_attr = false;
|
||||||
|
apa.last_stmt_span = DUMMY_SP;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if let hir::ExprKind::MethodCall(_, _, _, span) = expr.kind {
|
||||||
|
apa.last_method_span = span;
|
||||||
|
}
|
||||||
},
|
},
|
||||||
_ => {},
|
_ => {},
|
||||||
}
|
}
|
||||||
|
apa.last_stmt_span = self.ap.curr_stmt.span;
|
||||||
|
modify_apa_params(apa);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
walk_expr(self, expr);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Auxiliary parameters used on each block check of an item
|
||||||
|
struct AuxParams<'others, 'stmt, 'tcx> {
|
||||||
|
//// See [AuxParamsAttr].
|
||||||
|
apas: &'others mut FxIndexMap<hir::HirId, AuxParamsAttr>,
|
||||||
|
/// The current block identifier that is being visited.
|
||||||
|
curr_block_hir_id: hir::HirId,
|
||||||
|
/// The current block span that is being visited.
|
||||||
|
curr_block_span: Span,
|
||||||
|
/// The current statement that is being visited.
|
||||||
|
curr_stmt: Cow<'stmt, hir::Stmt<'tcx>>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<'others, 'stmt, 'tcx> AuxParams<'others, 'stmt, 'tcx> {
|
||||||
|
fn new(apas: &'others mut FxIndexMap<hir::HirId, AuxParamsAttr>, curr_stmt: &'stmt hir::Stmt<'tcx>) -> Self {
|
||||||
|
Self {
|
||||||
|
apas,
|
||||||
|
curr_block_hir_id: hir::HirId::INVALID,
|
||||||
|
curr_block_span: DUMMY_SP,
|
||||||
|
curr_stmt: Cow::Borrowed(curr_stmt),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Auxiliary parameters used on expression created with `#[has_significant_drop]`.
|
||||||
|
#[derive(Debug)]
|
||||||
|
struct AuxParamsAttr {
|
||||||
|
/// The number of times `#[has_significant_drop]` was referenced.
|
||||||
|
counter: usize,
|
||||||
|
/// If an expensive expression follows the last use of anything marked with
|
||||||
|
/// `#[has_significant_drop]`.
|
||||||
|
has_expensive_expr_after_last_attr: bool,
|
||||||
|
|
||||||
|
/// The identifier of the block that involves the first `#[has_significant_drop]`.
|
||||||
|
first_block_hir_id: hir::HirId,
|
||||||
|
/// The span of the block that involves the first `#[has_significant_drop]`.
|
||||||
|
first_block_span: Span,
|
||||||
|
/// The binding or variable that references the initial construction of the type marked with
|
||||||
|
/// `#[has_significant_drop]`.
|
||||||
|
first_bind_ident: Ident,
|
||||||
|
/// Similar to `init_bind_ident` but encompasses the right-hand method call.
|
||||||
|
first_method_span: Span,
|
||||||
|
/// Similar to `init_bind_ident` but encompasses the whole contained statement.
|
||||||
|
first_stmt_span: Span,
|
||||||
|
|
||||||
|
/// The last visited binding or variable span within a block that had any referenced inner type
|
||||||
|
/// marked with `#[has_significant_drop]`.
|
||||||
|
last_bind_ident: Ident,
|
||||||
|
/// Similar to `last_bind_span` but encompasses the right-hand method call.
|
||||||
|
last_method_span: Span,
|
||||||
|
/// Similar to `last_bind_span` but encompasses the whole contained statement.
|
||||||
|
last_stmt_span: Span,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Default for AuxParamsAttr {
|
||||||
|
fn default() -> Self {
|
||||||
|
Self {
|
||||||
|
counter: 0,
|
||||||
|
has_expensive_expr_after_last_attr: false,
|
||||||
|
first_block_hir_id: hir::HirId::INVALID,
|
||||||
|
first_bind_ident: Ident::empty(),
|
||||||
|
first_block_span: DUMMY_SP,
|
||||||
|
first_method_span: DUMMY_SP,
|
||||||
|
first_stmt_span: DUMMY_SP,
|
||||||
|
last_bind_ident: Ident::empty(),
|
||||||
|
last_method_span: DUMMY_SP,
|
||||||
|
last_stmt_span: DUMMY_SP,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn dummy_stmt_expr<'any>(expr: &'any hir::Expr<'any>) -> hir::Stmt<'any> {
|
||||||
|
hir::Stmt {
|
||||||
|
hir_id: hir::HirId::INVALID,
|
||||||
|
kind: hir::StmtKind::Expr(expr),
|
||||||
|
span: DUMMY_SP,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn has_drop(expr: &hir::Expr<'_>, first_bind_ident: &Ident) -> bool {
|
||||||
|
if let hir::ExprKind::Call(fun, args) = expr.kind
|
||||||
|
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, fun_path)) = &fun.kind
|
||||||
|
&& let [fun_ident, ..] = fun_path.segments
|
||||||
|
&& fun_ident.ident.name == rustc_span::sym::drop
|
||||||
|
&& let [first_arg, ..] = args
|
||||||
|
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, arg_path)) = &first_arg.kind
|
||||||
|
&& let [first_arg_ps, .. ] = arg_path.segments
|
||||||
|
{
|
||||||
|
&first_arg_ps.ident == first_bind_ident
|
||||||
|
}
|
||||||
|
else {
|
||||||
|
false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_expensive_expr(expr: &hir::Expr<'_>) -> bool {
|
||||||
|
!matches!(expr.kind, hir::ExprKind::Path(_))
|
||||||
|
}
|
||||||
|
@ -16,6 +16,18 @@ pub fn complex_return_triggers_the_lint() -> i32 {
|
|||||||
foo()
|
foo()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn issue_10413() {
|
||||||
|
let mutex = Mutex::new(Some(1));
|
||||||
|
let opt = Some(1);
|
||||||
|
if opt.is_some() {
|
||||||
|
let lock = mutex.lock().unwrap();
|
||||||
|
let _ = *lock;
|
||||||
|
if opt.is_some() {
|
||||||
|
let _ = *lock;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
pub fn path_return_can_be_ignored() -> i32 {
|
pub fn path_return_can_be_ignored() -> i32 {
|
||||||
let mutex = Mutex::new(1);
|
let mutex = Mutex::new(1);
|
||||||
let lock = mutex.lock().unwrap();
|
let lock = mutex.lock().unwrap();
|
||||||
|
@ -15,6 +15,18 @@ fn foo() -> i32 {
|
|||||||
foo()
|
foo()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn issue_10413() {
|
||||||
|
let mutex = Mutex::new(Some(1));
|
||||||
|
let opt = Some(1);
|
||||||
|
if opt.is_some() {
|
||||||
|
let lock = mutex.lock().unwrap();
|
||||||
|
let _ = *lock;
|
||||||
|
if opt.is_some() {
|
||||||
|
let _ = *lock;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
pub fn path_return_can_be_ignored() -> i32 {
|
pub fn path_return_can_be_ignored() -> i32 {
|
||||||
let mutex = Mutex::new(1);
|
let mutex = Mutex::new(1);
|
||||||
let lock = mutex.lock().unwrap();
|
let lock = mutex.lock().unwrap();
|
||||||
|
@ -23,7 +23,7 @@ LL + drop(lock);
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: temporary with significant `Drop` can be early dropped
|
error: temporary with significant `Drop` can be early dropped
|
||||||
--> $DIR/significant_drop_tightening.rs:44:13
|
--> $DIR/significant_drop_tightening.rs:56:13
|
||||||
|
|
|
|
||||||
LL | / {
|
LL | / {
|
||||||
LL | | let mutex = Mutex::new(1i32);
|
LL | | let mutex = Mutex::new(1i32);
|
||||||
@ -43,7 +43,7 @@ LL + drop(lock);
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: temporary with significant `Drop` can be early dropped
|
error: temporary with significant `Drop` can be early dropped
|
||||||
--> $DIR/significant_drop_tightening.rs:65:13
|
--> $DIR/significant_drop_tightening.rs:77:13
|
||||||
|
|
|
|
||||||
LL | / {
|
LL | / {
|
||||||
LL | | let mutex = Mutex::new(1i32);
|
LL | | let mutex = Mutex::new(1i32);
|
||||||
@ -67,7 +67,7 @@ LL +
|
|||||||
|
|
|
|
||||||
|
|
||||||
error: temporary with significant `Drop` can be early dropped
|
error: temporary with significant `Drop` can be early dropped
|
||||||
--> $DIR/significant_drop_tightening.rs:71:17
|
--> $DIR/significant_drop_tightening.rs:83:17
|
||||||
|
|
|
|
||||||
LL | / {
|
LL | / {
|
||||||
LL | | let mutex = Mutex::new(vec![1i32]);
|
LL | | let mutex = Mutex::new(vec![1i32]);
|
||||||
|
Loading…
Reference in New Issue
Block a user