Skip to content

Commit cf843a3

Browse files
lint on tail expr drop order change in Edition 2024
1 parent f7eefec commit cf843a3

File tree

6 files changed

+295
-0
lines changed

6 files changed

+295
-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
);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
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::{Block, Expr, ExprKind, LetStmt, Pat, PatKind, QPath, StmtKind};
7+
use rustc_macros::LintDiagnostic;
8+
use rustc_session::lint::FutureIncompatibilityReason;
9+
use rustc_session::{declare_lint, declare_lint_pass};
10+
use rustc_span::edition::Edition;
11+
use rustc_span::Span;
12+
13+
use crate::{LateContext, LateLintPass};
14+
15+
declare_lint! {
16+
/// The `tail_expr_drop_order` lint looks for those values generated at the tail expression location, that of type
17+
/// with a significant `Drop` implementation, such as locks.
18+
/// In case there are also local variables of type with significant `Drop` implementation as well,
19+
/// this lint warns you of a potential transposition in the drop order.
20+
/// Your discretion on the new drop order introduced by Edition 2024 is required.
21+
///
22+
/// ### Example
23+
/// ```rust,edition2024
24+
/// #[deny(tail_expr_drop_order)]
25+
/// #![feature(shorter_tail_lifetimes)]
26+
/// struct Droppy;
27+
/// impl Droppy {
28+
/// fn get(&self) -> i32 {
29+
/// 0
30+
/// }
31+
/// }
32+
/// impl Drop for Droppy {
33+
/// fn drop(&mut self) {
34+
/// println!("loud drop");
35+
/// }
36+
/// }
37+
/// fn edition_2024() -> i32 {
38+
/// let another_droppy = Droppy;
39+
/// Droppy.get()
40+
/// }
41+
/// fn main() {
42+
/// edition_2024();
43+
/// }
44+
/// ```
45+
///
46+
/// {{produces}}
47+
///
48+
/// ### Explanation
49+
///
50+
/// In tail expression of blocks or function bodies,
51+
/// values of type with significant `Drop` implementation has an ill-specified drop order before Edition 2024
52+
/// so that they are dropped only after dropping local variables.
53+
/// Edition 2024 introduces a new rule with drop orders for them, so that they are dropped first before dropping local variables.
54+
///
55+
/// This lint only points out the issue with `Droppy`, which will be dropped before `another_droppy` does in Edition 2024.
56+
/// No fix will be proposed.
57+
/// However, the most probable fix is to hoist `Droppy` into its own local variable binding.
58+
/// ```no_run
59+
/// fn edition_2024() -> i32 {
60+
/// let value = Droppy;
61+
/// let another_droppy = Droppy;
62+
/// value.get()
63+
/// }
64+
/// ```
65+
pub TAIL_EXPR_DROP_ORDER,
66+
Allow,
67+
"Detect and warn on significant change in drop order in tail expression location",
68+
@future_incompatible = FutureIncompatibleInfo {
69+
reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024),
70+
reference: "issue #123739 <https://github.com/rust-lang/rust/issues/123739>",
71+
};
72+
}
73+
74+
declare_lint_pass!(TailExprDropOrder => [TAIL_EXPR_DROP_ORDER]);
75+
76+
impl<'tcx> LateLintPass<'tcx> for TailExprDropOrder {
77+
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) {
78+
LintVisitor { cx, locals: <_>::default() }.check_block_inner(block);
79+
}
80+
}
81+
82+
struct LintVisitor<'tcx, 'a> {
83+
cx: &'a LateContext<'tcx>,
84+
locals: Vec<Span>,
85+
}
86+
87+
struct LocalCollector<'tcx, 'a> {
88+
cx: &'a LateContext<'tcx>,
89+
locals: &'a mut Vec<Span>,
90+
}
91+
92+
impl<'tcx, 'a> Visitor<'tcx> for LocalCollector<'tcx, 'a> {
93+
type Result = ();
94+
fn visit_pat(&mut self, pat: &'tcx Pat<'tcx>) {
95+
if let PatKind::Binding(_binding_mode, id, ident, pat) = pat.kind {
96+
let ty = self.cx.typeck_results().node_type(id);
97+
if ty.has_significant_drop(self.cx.tcx, self.cx.param_env) {
98+
self.locals.push(ident.span);
99+
}
100+
if let Some(pat) = pat {
101+
self.visit_pat(pat);
102+
}
103+
} else {
104+
intravisit::walk_pat(self, pat);
105+
}
106+
}
107+
}
108+
109+
impl<'tcx, 'a> Visitor<'tcx> for LintVisitor<'tcx, 'a> {
110+
fn visit_block(&mut self, block: &'tcx Block<'tcx>) {
111+
let mut locals = <_>::default();
112+
swap(&mut locals, &mut self.locals);
113+
self.check_block_inner(block);
114+
swap(&mut locals, &mut self.locals);
115+
}
116+
fn visit_local(&mut self, local: &'tcx LetStmt<'tcx>) {
117+
LocalCollector { cx: self.cx, locals: &mut self.locals }.visit_local(local);
118+
}
119+
}
120+
121+
impl<'tcx, 'a> LintVisitor<'tcx, 'a> {
122+
fn check_block_inner(&mut self, block: &Block<'tcx>) {
123+
if !block.span.at_least_rust_2024() {
124+
// We only lint for Edition 2024 onwards
125+
return;
126+
}
127+
let Some(tail_expr) = block.expr else { return };
128+
for stmt in block.stmts {
129+
match stmt.kind {
130+
StmtKind::Let(let_stmt) => self.visit_local(let_stmt),
131+
StmtKind::Item(_) => {}
132+
StmtKind::Expr(e) | StmtKind::Semi(e) => self.visit_expr(e),
133+
}
134+
}
135+
if self.locals.is_empty() {
136+
return;
137+
}
138+
LintTailExpr { cx: self.cx, locals: &self.locals }.visit_expr(tail_expr);
139+
}
140+
}
141+
142+
struct LintTailExpr<'tcx, 'a> {
143+
cx: &'a LateContext<'tcx>,
144+
locals: &'a [Span],
145+
}
146+
147+
impl<'tcx, 'a> LintTailExpr<'tcx, 'a> {
148+
fn expr_eventually_point_into_local(mut expr: &Expr<'tcx>) -> bool {
149+
loop {
150+
match expr.kind {
151+
ExprKind::Index(access, _, _) | ExprKind::Field(access, _) => expr = access,
152+
ExprKind::AddrOf(_, _, referee) | ExprKind::Unary(UnOp::Deref, referee) => {
153+
expr = referee
154+
}
155+
ExprKind::Path(_)
156+
if let ExprKind::Path(QPath::Resolved(_, path)) = expr.kind
157+
&& let [local, ..] = path.segments
158+
&& let Res::Local(_) = local.res =>
159+
{
160+
return true;
161+
}
162+
_ => return false,
163+
}
164+
}
165+
}
166+
167+
fn expr_generates_nonlocal_droppy_value(&self, expr: &Expr<'tcx>) -> bool {
168+
if Self::expr_eventually_point_into_local(expr) {
169+
return false;
170+
}
171+
self.cx.typeck_results().expr_ty(expr).has_significant_drop(self.cx.tcx, self.cx.param_env)
172+
}
173+
}
174+
175+
impl<'tcx, 'a> Visitor<'tcx> for LintTailExpr<'tcx, 'a> {
176+
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
177+
if self.expr_generates_nonlocal_droppy_value(expr) {
178+
self.cx.tcx.emit_node_span_lint(
179+
TAIL_EXPR_DROP_ORDER,
180+
expr.hir_id,
181+
expr.span,
182+
TailExprDropOrderLint { spans: self.locals.to_vec() },
183+
);
184+
return;
185+
}
186+
match expr.kind {
187+
ExprKind::ConstBlock(_)
188+
| ExprKind::Array(_)
189+
| ExprKind::Break(_, _)
190+
| ExprKind::Continue(_)
191+
| ExprKind::Ret(_)
192+
| ExprKind::Become(_)
193+
| ExprKind::Yield(_, _)
194+
| ExprKind::InlineAsm(_)
195+
| ExprKind::If(_, _, _)
196+
| ExprKind::Loop(_, _, _, _)
197+
| ExprKind::Match(_, _, _)
198+
| ExprKind::Closure(_)
199+
| ExprKind::DropTemps(_)
200+
| ExprKind::OffsetOf(_, _)
201+
| ExprKind::Assign(_, _, _)
202+
| ExprKind::AssignOp(_, _, _)
203+
| ExprKind::Lit(_)
204+
| ExprKind::Err(_) => {}
205+
206+
ExprKind::MethodCall(_, _, _, _)
207+
| ExprKind::Call(_, _)
208+
| ExprKind::Type(_, _)
209+
| ExprKind::Tup(_)
210+
| ExprKind::Binary(_, _, _)
211+
| ExprKind::Unary(_, _)
212+
| ExprKind::Path(_)
213+
| ExprKind::Let(_)
214+
| ExprKind::Cast(_, _)
215+
| ExprKind::Field(_, _)
216+
| ExprKind::Index(_, _, _)
217+
| ExprKind::AddrOf(_, _, _)
218+
| ExprKind::Struct(_, _, _)
219+
| ExprKind::Repeat(_, _) => intravisit::walk_expr(self, expr),
220+
221+
ExprKind::Block(block, _) => {
222+
LintVisitor { cx: self.cx, locals: <_>::default() }.check_block_inner(block)
223+
}
224+
}
225+
}
226+
fn visit_block(&mut self, block: &'tcx Block<'tcx>) {
227+
LintVisitor { cx: self.cx, locals: <_>::default() }.check_block_inner(block);
228+
}
229+
}
230+
231+
#[derive(LintDiagnostic)]
232+
#[diag(lint_tail_expr_drop_order)]
233+
struct TailExprDropOrderLint {
234+
#[label]
235+
pub spans: Vec<Span>,
236+
}

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") {
+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//@compile-flags: -Z unstable-options
2+
//@edition:2024
3+
4+
#![deny(tail_expr_drop_order)]
5+
#![feature(shorter_tail_lifetimes)]
6+
7+
struct LoudDropper;
8+
impl Drop for LoudDropper {
9+
fn drop(&mut self) {
10+
println!("loud drop")
11+
}
12+
}
13+
impl LoudDropper {
14+
fn get(&self) -> i32 {
15+
0
16+
}
17+
}
18+
19+
fn should_lint() -> i32 {
20+
let x = LoudDropper;
21+
// Should lint
22+
x.get() + LoudDropper.get()
23+
//~^ ERROR: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
24+
//~| WARN: this changes meaning in Rust 2024
25+
}
26+
27+
fn should_not_lint() -> i32 {
28+
let x = LoudDropper;
29+
// Should not lint
30+
x.get()
31+
}
32+
33+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
error: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
2+
--> $DIR/lint-tail-expr-drop-order.rs:22:15
3+
|
4+
LL | let x = LoudDropper;
5+
| - these values have significant drop implementation and will observe changes in drop order under Edition 2024
6+
LL | // Should lint
7+
LL | x.get() + LoudDropper.get()
8+
| ^^^^^^^^^^^
9+
|
10+
= warning: this changes meaning in Rust 2024
11+
= note: for more information, see issue #123739 <https://github.com/rust-lang/rust/issues/123739>
12+
note: the lint level is defined here
13+
--> $DIR/lint-tail-expr-drop-order.rs:4:9
14+
|
15+
LL | #![deny(tail_expr_drop_order)]
16+
| ^^^^^^^^^^^^^^^^^^^^
17+
18+
error: aborting due to 1 previous error
19+

0 commit comments

Comments
 (0)