Skip to content

Commit 28d804e

Browse files
dingxiangfei2009dingxiangfei2009
committed
lint on tail expr drop order change in Edition 2024
1 parent 4fe1e2b commit 28d804e

File tree

6 files changed

+424
-0
lines changed

6 files changed

+424
-0
lines changed

‎compiler/rustc_lint/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,9 @@ lint_suspicious_double_ref_clone =
758758
lint_suspicious_double_ref_deref =
759759
using `.deref()` on a double reference, which returns `{$ty}` instead of dereferencing the inner type
760760
761+
lint_tail_expr_drop_order = these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
762+
.label = these values have significant drop implementation and will observe changes in drop order under Edition 2024
763+
761764
lint_trailing_semi_macro = trailing semicolon in macro used in expression position
762765
.note1 = macro invocations at the end of a block are treated as expressions
763766
.note2 = to ignore the value produced by the macro, add a semicolon after the invocation of `{$name}`

‎compiler/rustc_lint/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ mod ptr_nulls;
7878
mod redundant_semicolon;
7979
mod reference_casting;
8080
mod shadowed_into_iter;
81+
mod tail_expr_drop_order;
8182
mod traits;
8283
mod types;
8384
mod unit_bindings;
@@ -115,6 +116,7 @@ use rustc_middle::query::Providers;
115116
use rustc_middle::ty::TyCtxt;
116117
use shadowed_into_iter::ShadowedIntoIter;
117118
pub use shadowed_into_iter::{ARRAY_INTO_ITER, BOXED_SLICE_INTO_ITER};
119+
use tail_expr_drop_order::TailExprDropOrder;
118120
use traits::*;
119121
use types::*;
120122
use unit_bindings::*;
@@ -238,6 +240,7 @@ late_lint_methods!(
238240
AsyncFnInTrait: AsyncFnInTrait,
239241
NonLocalDefinitions: NonLocalDefinitions::default(),
240242
ImplTraitOvercaptures: ImplTraitOvercaptures,
243+
TailExprDropOrder: TailExprDropOrder,
241244
]
242245
]
243246
);

‎compiler/rustc_lint/src/tail_expr_drop_order.rs

+304
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,304 @@
1+
use std::mem::swap;
2+
3+
use rustc_ast::UnOp;
4+
use rustc_hir::def::Res;
5+
use rustc_hir::intravisit::{self, Visitor};
6+
use rustc_hir::{self as hir, Block, Expr, ExprKind, LetStmt, Pat, PatKind, QPath, StmtKind};
7+
use rustc_macros::LintDiagnostic;
8+
use rustc_middle::ty;
9+
use rustc_session::lint::FutureIncompatibilityReason;
10+
use rustc_session::{declare_lint, declare_lint_pass};
11+
use rustc_span::edition::Edition;
12+
use rustc_span::Span;
13+
14+
use crate::{LateContext, LateLintPass};
15+
16+
declare_lint! {
17+
/// The `tail_expr_drop_order` lint looks for those values generated at the tail expression location, that of type
18+
/// with a significant `Drop` implementation, such as locks.
19+
/// In case there are also local variables of type with significant `Drop` implementation as well,
20+
/// this lint warns you of a potential transposition in the drop order.
21+
/// Your discretion on the new drop order introduced by Edition 2024 is required.
22+
///
23+
/// ### Example
24+
/// ```rust,edition2024
25+
/// #![feature(shorter_tail_lifetimes)]
26+
/// #![warn(tail_expr_drop_order)]
27+
/// struct Droppy(i32);
28+
/// impl Droppy {
29+
/// fn get(&self) -> i32 {
30+
/// self.0
31+
/// }
32+
/// }
33+
/// impl Drop for Droppy {
34+
/// fn drop(&mut self) {
35+
/// // This is a custom destructor and it induces side-effects that is observable
36+
/// // especially when the drop order at a tail expression changes.
37+
/// println!("loud drop {}", self.0);
38+
/// }
39+
/// }
40+
/// fn edition_2024() -> i32 {
41+
/// let another_droppy = Droppy(0);
42+
/// Droppy(1).get()
43+
/// }
44+
/// fn main() {
45+
/// edition_2024();
46+
/// }
47+
/// ```
48+
///
49+
/// {{produces}}
50+
///
51+
/// ### Explanation
52+
///
53+
/// In tail expression of blocks or function bodies,
54+
/// values of type with significant `Drop` implementation has an ill-specified drop order
55+
/// before Edition 2024 so that they are dropped only after dropping local variables.
56+
/// Edition 2024 introduces a new rule with drop orders for them,
57+
/// so that they are dropped first before dropping local variables.
58+
///
59+
/// A significant `Drop::drop` destructor here refers to an explicit, arbitrary
60+
/// implementation of the `Drop` trait on the type, with exceptions including `Vec`,
61+
/// `Box`, `Rc`, `BTreeMap` and `HashMap` that are marked by the compiler otherwise
62+
/// so long that the generic types have no significant destructor recursively.
63+
/// In other words, a type has a significant drop destructor when it has a `Drop` implementation
64+
/// or its destructor invokes a significant destructor on a type.
65+
/// Since we cannot completely reason about the change by just inspecting the existence of
66+
/// a significant destructor, this lint remains only a suggestion and is set to `allow` by default.
67+
///
68+
/// This lint only points out the issue with `Droppy`, which will be dropped before `another_droppy`
69+
/// does in Edition 2024.
70+
/// No fix will be proposed by this lint.
71+
/// However, the most probable fix is to hoist `Droppy` into its own local variable binding.
72+
/// ```rust
73+
/// struct Droppy(i32);
74+
/// impl Droppy {
75+
/// fn get(&self) -> i32 {
76+
/// self.0
77+
/// }
78+
/// }
79+
/// fn edition_2024() -> i32 {
80+
/// let value = Droppy(0);
81+
/// let another_droppy = Droppy(1);
82+
/// value.get()
83+
/// }
84+
/// ```
85+
pub TAIL_EXPR_DROP_ORDER,
86+
Allow,
87+
"Detect and warn on significant change in drop order in tail expression location",
88+
@future_incompatible = FutureIncompatibleInfo {
89+
reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024),
90+
reference: "issue #123739 <https://github.com/rust-lang/rust/issues/123739>",
91+
};
92+
}
93+
94+
declare_lint_pass!(TailExprDropOrder => [TAIL_EXPR_DROP_ORDER]);
95+
96+
impl TailExprDropOrder {
97+
fn check_fn_or_closure<'tcx>(
98+
cx: &LateContext<'tcx>,
99+
fn_kind: hir::intravisit::FnKind<'tcx>,
100+
body: &'tcx hir::Body<'tcx>,
101+
def_id: rustc_span::def_id::LocalDefId,
102+
) {
103+
let mut locals = vec![];
104+
if matches!(fn_kind, hir::intravisit::FnKind::Closure) {
105+
for &capture in cx.tcx.closure_captures(def_id) {
106+
if matches!(capture.info.capture_kind, ty::UpvarCapture::ByValue)
107+
&& capture.place.ty().has_significant_drop(cx.tcx, cx.param_env)
108+
{
109+
locals.push(capture.var_ident.span);
110+
}
111+
}
112+
}
113+
for param in body.params {
114+
if cx
115+
.typeck_results()
116+
.node_type(param.hir_id)
117+
.has_significant_drop(cx.tcx, cx.param_env)
118+
{
119+
locals.push(param.span);
120+
}
121+
}
122+
if let hir::ExprKind::Block(block, _) = body.value.kind {
123+
LintVisitor { cx, locals }.check_block_inner(block);
124+
} else {
125+
LintTailExpr { cx, locals: &locals, is_root_tail_expr: true }.visit_expr(body.value);
126+
}
127+
}
128+
}
129+
130+
impl<'tcx> LateLintPass<'tcx> for TailExprDropOrder {
131+
fn check_fn(
132+
&mut self,
133+
cx: &LateContext<'tcx>,
134+
fn_kind: hir::intravisit::FnKind<'tcx>,
135+
_: &'tcx hir::FnDecl<'tcx>,
136+
body: &'tcx hir::Body<'tcx>,
137+
_: Span,
138+
def_id: rustc_span::def_id::LocalDefId,
139+
) {
140+
Self::check_fn_or_closure(cx, fn_kind, body, def_id);
141+
}
142+
}
143+
144+
struct LintVisitor<'tcx, 'a> {
145+
cx: &'a LateContext<'tcx>,
146+
// We only record locals that have significant drops
147+
locals: Vec<Span>,
148+
}
149+
150+
struct LocalCollector<'tcx, 'a> {
151+
cx: &'a LateContext<'tcx>,
152+
locals: &'a mut Vec<Span>,
153+
}
154+
155+
impl<'tcx, 'a> Visitor<'tcx> for LocalCollector<'tcx, 'a> {
156+
type Result = ();
157+
fn visit_pat(&mut self, pat: &'tcx Pat<'tcx>) {
158+
if let PatKind::Binding(_binding_mode, id, ident, pat) = pat.kind {
159+
let ty = self.cx.typeck_results().node_type(id);
160+
if ty.has_significant_drop(self.cx.tcx, self.cx.param_env) {
161+
self.locals.push(ident.span);
162+
}
163+
if let Some(pat) = pat {
164+
self.visit_pat(pat);
165+
}
166+
} else {
167+
intravisit::walk_pat(self, pat);
168+
}
169+
}
170+
}
171+
172+
impl<'tcx, 'a> Visitor<'tcx> for LintVisitor<'tcx, 'a> {
173+
fn visit_block(&mut self, block: &'tcx Block<'tcx>) {
174+
let mut locals = <_>::default();
175+
swap(&mut locals, &mut self.locals);
176+
self.check_block_inner(block);
177+
swap(&mut locals, &mut self.locals);
178+
}
179+
fn visit_local(&mut self, local: &'tcx LetStmt<'tcx>) {
180+
LocalCollector { cx: self.cx, locals: &mut self.locals }.visit_local(local);
181+
}
182+
}
183+
184+
impl<'tcx, 'a> LintVisitor<'tcx, 'a> {
185+
fn check_block_inner(&mut self, block: &Block<'tcx>) {
186+
if !block.span.at_least_rust_2024() {
187+
// We only lint for Edition 2024 onwards
188+
return;
189+
}
190+
let Some(tail_expr) = block.expr else { return };
191+
for stmt in block.stmts {
192+
match stmt.kind {
193+
StmtKind::Let(let_stmt) => self.visit_local(let_stmt),
194+
StmtKind::Item(_) => {}
195+
StmtKind::Expr(e) | StmtKind::Semi(e) => self.visit_expr(e),
196+
}
197+
}
198+
if self.locals.is_empty() {
199+
return;
200+
}
201+
LintTailExpr { cx: self.cx, locals: &self.locals, is_root_tail_expr: true }
202+
.visit_expr(tail_expr);
203+
}
204+
}
205+
206+
struct LintTailExpr<'tcx, 'a> {
207+
cx: &'a LateContext<'tcx>,
208+
is_root_tail_expr: bool,
209+
locals: &'a [Span],
210+
}
211+
212+
impl<'tcx, 'a> LintTailExpr<'tcx, 'a> {
213+
fn expr_eventually_point_into_local(mut expr: &Expr<'tcx>) -> bool {
214+
loop {
215+
match expr.kind {
216+
ExprKind::Index(access, _, _) | ExprKind::Field(access, _) => expr = access,
217+
ExprKind::AddrOf(_, _, referee) | ExprKind::Unary(UnOp::Deref, referee) => {
218+
expr = referee
219+
}
220+
ExprKind::Path(_)
221+
if let ExprKind::Path(QPath::Resolved(_, path)) = expr.kind
222+
&& let [local, ..] = path.segments
223+
&& let Res::Local(_) = local.res =>
224+
{
225+
return true;
226+
}
227+
_ => return false,
228+
}
229+
}
230+
}
231+
232+
fn expr_generates_nonlocal_droppy_value(&self, expr: &Expr<'tcx>) -> bool {
233+
if Self::expr_eventually_point_into_local(expr) {
234+
return false;
235+
}
236+
self.cx.typeck_results().expr_ty(expr).has_significant_drop(self.cx.tcx, self.cx.param_env)
237+
}
238+
}
239+
240+
impl<'tcx, 'a> Visitor<'tcx> for LintTailExpr<'tcx, 'a> {
241+
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
242+
if self.is_root_tail_expr {
243+
self.is_root_tail_expr = false;
244+
} else if self.expr_generates_nonlocal_droppy_value(expr) {
245+
self.cx.tcx.emit_node_span_lint(
246+
TAIL_EXPR_DROP_ORDER,
247+
expr.hir_id,
248+
expr.span,
249+
TailExprDropOrderLint { spans: self.locals.to_vec() },
250+
);
251+
return;
252+
}
253+
match expr.kind {
254+
ExprKind::Match(scrutinee, _, _) => self.visit_expr(scrutinee),
255+
256+
ExprKind::ConstBlock(_)
257+
| ExprKind::Array(_)
258+
| ExprKind::Break(_, _)
259+
| ExprKind::Continue(_)
260+
| ExprKind::Ret(_)
261+
| ExprKind::Become(_)
262+
| ExprKind::Yield(_, _)
263+
| ExprKind::InlineAsm(_)
264+
| ExprKind::If(_, _, _)
265+
| ExprKind::Loop(_, _, _, _)
266+
| ExprKind::Closure(_)
267+
| ExprKind::DropTemps(_)
268+
| ExprKind::OffsetOf(_, _)
269+
| ExprKind::Assign(_, _, _)
270+
| ExprKind::AssignOp(_, _, _)
271+
| ExprKind::Lit(_)
272+
| ExprKind::Err(_) => {}
273+
274+
ExprKind::MethodCall(_, _, _, _)
275+
| ExprKind::Call(_, _)
276+
| ExprKind::Type(_, _)
277+
| ExprKind::Tup(_)
278+
| ExprKind::Binary(_, _, _)
279+
| ExprKind::Unary(_, _)
280+
| ExprKind::Path(_)
281+
| ExprKind::Let(_)
282+
| ExprKind::Cast(_, _)
283+
| ExprKind::Field(_, _)
284+
| ExprKind::Index(_, _, _)
285+
| ExprKind::AddrOf(_, _, _)
286+
| ExprKind::Struct(_, _, _)
287+
| ExprKind::Repeat(_, _) => intravisit::walk_expr(self, expr),
288+
289+
ExprKind::Block(_, _) => {
290+
// We do not lint further because the drop order stays the same inside the block
291+
}
292+
}
293+
}
294+
fn visit_block(&mut self, block: &'tcx Block<'tcx>) {
295+
LintVisitor { cx: self.cx, locals: <_>::default() }.check_block_inner(block);
296+
}
297+
}
298+
299+
#[derive(LintDiagnostic)]
300+
#[diag(lint_tail_expr_drop_order)]
301+
struct TailExprDropOrderLint {
302+
#[label]
303+
pub spans: Vec<Span>,
304+
}

‎src/tools/lint-docs/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,7 @@ impl<'a> LintExtractor<'a> {
444444
let mut cmd = Command::new(self.rustc_path);
445445
if options.contains(&"edition2024") {
446446
cmd.arg("--edition=2024");
447+
cmd.arg("-Zunstable-options");
447448
} else if options.contains(&"edition2021") {
448449
cmd.arg("--edition=2021");
449450
} else if options.contains(&"edition2018") {

0 commit comments

Comments
 (0)