mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

lint on tail expr drop order change in Edition 2024

Browse Source
This commit is contained in:
Ding Xiang Fei 2024-08-05 03:50:15 +08:00
parent 4d5b3b1962
commit ef25fbd0b4
No known key found for this signature in database
GPG Key ID: 3CD748647EEF6359
7 changed files with 461 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
compiler/rustc_lint/messages.ftl
View File

@ -758,6 +758,9 @@ lint_suspicious_double_ref_clone =
lint_suspicious_double_ref_deref =
using `.deref()` on a double reference, which returns `{$ty}` instead of dereferencing the inner type
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
.label = these values have significant drop implementation and will observe changes in drop order under Edition 2024
lint_trailing_semi_macro = trailing semicolon in macro used in expression position
.note1 = macro invocations at the end of a block are treated as expressions
.note2 = to ignore the value produced by the macro, add a semicolon after the invocation of `{$name}`

3
compiler/rustc_lint/src/lib.rs
View File

@ -78,6 +78,7 @@
mod redundant_semicolon;
mod reference_casting;
mod shadowed_into_iter;
mod tail_expr_drop_order;
mod traits;
mod types;
mod unit_bindings;
@ -115,6 +116,7 @@
use rustc_middle::ty::TyCtxt;
use shadowed_into_iter::ShadowedIntoIter;
pub use shadowed_into_iter::{ARRAY_INTO_ITER, BOXED_SLICE_INTO_ITER};
use tail_expr_drop_order::TailExprDropOrder;
use traits::*;
use types::*;
use unit_bindings::*;
@ -238,6 +240,7 @@ fn lint_mod(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
AsyncFnInTrait: AsyncFnInTrait,
NonLocalDefinitions: NonLocalDefinitions::default(),
ImplTraitOvercaptures: ImplTraitOvercaptures,
TailExprDropOrder: TailExprDropOrder,
]
]
);

306
compiler/rustc_lint/src/tail_expr_drop_order.rs Normal file
View File

@ -0,0 +1,306 @@
use std::mem::swap;
use rustc_ast::UnOp;
use rustc_hir::def::Res;
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{self as hir, Block, Expr, ExprKind, LetStmt, Pat, PatKind, QPath, StmtKind};
use rustc_macros::LintDiagnostic;
use rustc_middle::ty;
use rustc_session::lint::FutureIncompatibilityReason;
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::edition::Edition;
use rustc_span::Span;
use crate::{LateContext, LateLintPass};
declare_lint! {
/// The `tail_expr_drop_order` lint looks for those values generated at the tail expression location, that of type
/// with a significant `Drop` implementation, such as locks.
/// In case there are also local variables of type with significant `Drop` implementation as well,
/// this lint warns you of a potential transposition in the drop order.
/// Your discretion on the new drop order introduced by Edition 2024 is required.
///
/// ### Example
/// ```rust,edition2024
/// #![feature(shorter_tail_lifetimes)]
/// #![warn(tail_expr_drop_order)]
/// struct Droppy(i32);
/// impl Droppy {
/// fn get(&self) -> i32 {
/// self.0
/// }
/// }
/// impl Drop for Droppy {
/// fn drop(&mut self) {
/// // This is a custom destructor and it induces side-effects that is observable
/// // especially when the drop order at a tail expression changes.
/// println!("loud drop {}", self.0);
/// }
/// }
/// fn edition_2024() -> i32 {
/// let another_droppy = Droppy(0);
/// Droppy(1).get()
/// }
/// fn main() {
/// edition_2024();
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// In tail expression of blocks or function bodies,
/// values of type with significant `Drop` implementation has an ill-specified drop order
/// before Edition 2024 so that they are dropped only after dropping local variables.
/// Edition 2024 introduces a new rule with drop orders for them,
/// so that they are dropped first before dropping local variables.
///
/// A significant `Drop::drop` destructor here refers to an explicit, arbitrary
/// implementation of the `Drop` trait on the type, with exceptions including `Vec`,
/// `Box`, `Rc`, `BTreeMap` and `HashMap` that are marked by the compiler otherwise
/// so long that the generic types have no significant destructor recursively.
/// In other words, a type has a significant drop destructor when it has a `Drop` implementation
/// or its destructor invokes a significant destructor on a type.
/// Since we cannot completely reason about the change by just inspecting the existence of
/// a significant destructor, this lint remains only a suggestion and is set to `allow` by default.
///
/// This lint only points out the issue with `Droppy`, which will be dropped before `another_droppy`
/// does in Edition 2024.
/// No fix will be proposed by this lint.
/// However, the most probable fix is to hoist `Droppy` into its own local variable binding.
/// ```rust
/// struct Droppy(i32);
/// impl Droppy {
/// fn get(&self) -> i32 {
/// self.0
/// }
/// }
/// fn edition_2024() -> i32 {
/// let value = Droppy(0);
/// let another_droppy = Droppy(1);
/// value.get()
/// }
/// ```
pub TAIL_EXPR_DROP_ORDER,
Allow,
"Detect and warn on significant change in drop order in tail expression location",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024),
reference: "issue #123739 <https://github.com/rust-lang/rust/issues/123739>",
};
}
declare_lint_pass!(TailExprDropOrder => [TAIL_EXPR_DROP_ORDER]);
impl TailExprDropOrder {
fn check_fn_or_closure<'tcx>(
cx: &LateContext<'tcx>,
fn_kind: hir::intravisit::FnKind<'tcx>,
body: &'tcx hir::Body<'tcx>,
def_id: rustc_span::def_id::LocalDefId,
) {
let mut locals = vec![];
if matches!(fn_kind, hir::intravisit::FnKind::Closure) {
for &capture in cx.tcx.closure_captures(def_id) {
if matches!(capture.info.capture_kind, ty::UpvarCapture::ByValue)
&& capture.place.ty().has_significant_drop(cx.tcx, cx.param_env)
{
locals.push(capture.var_ident.span);
}
}
}
for param in body.params {
if cx
.typeck_results()
.node_type(param.hir_id)
.has_significant_drop(cx.tcx, cx.param_env)
{
locals.push(param.span);
}
}
if let hir::ExprKind::Block(block, _) = body.value.kind {
LintVisitor { cx, locals }.check_block_inner(block);
} else {
LintTailExpr { cx, locals: &locals, is_root_tail_expr: true }.visit_expr(body.value);
}
}
}
impl<'tcx> LateLintPass<'tcx> for TailExprDropOrder {
fn check_fn(
&mut self,
cx: &LateContext<'tcx>,
fn_kind: hir::intravisit::FnKind<'tcx>,
_: &'tcx hir::FnDecl<'tcx>,
body: &'tcx hir::Body<'tcx>,
_: Span,
def_id: rustc_span::def_id::LocalDefId,
) {
if cx.tcx.sess.at_least_rust_2024() && cx.tcx.features().shorter_tail_lifetimes {
Self::check_fn_or_closure(cx, fn_kind, body, def_id);
}
}
}
struct LintVisitor<'tcx, 'a> {
cx: &'a LateContext<'tcx>,
// We only record locals that have significant drops
locals: Vec<Span>,
}
struct LocalCollector<'tcx, 'a> {
cx: &'a LateContext<'tcx>,
locals: &'a mut Vec<Span>,
}
impl<'tcx, 'a> Visitor<'tcx> for LocalCollector<'tcx, 'a> {
type Result = ();
fn visit_pat(&mut self, pat: &'tcx Pat<'tcx>) {
if let PatKind::Binding(_binding_mode, id, ident, pat) = pat.kind {
let ty = self.cx.typeck_results().node_type(id);
if ty.has_significant_drop(self.cx.tcx, self.cx.param_env) {
self.locals.push(ident.span);
}
if let Some(pat) = pat {
self.visit_pat(pat);
}
} else {
intravisit::walk_pat(self, pat);
}
}
}
impl<'tcx, 'a> Visitor<'tcx> for LintVisitor<'tcx, 'a> {
fn visit_block(&mut self, block: &'tcx Block<'tcx>) {
let mut locals = <_>::default();
swap(&mut locals, &mut self.locals);
self.check_block_inner(block);
swap(&mut locals, &mut self.locals);
}
fn visit_local(&mut self, local: &'tcx LetStmt<'tcx>) {
LocalCollector { cx: self.cx, locals: &mut self.locals }.visit_local(local);
}
}
impl<'tcx, 'a> LintVisitor<'tcx, 'a> {
fn check_block_inner(&mut self, block: &Block<'tcx>) {
if !block.span.at_least_rust_2024() {
// We only lint for Edition 2024 onwards
return;
}
let Some(tail_expr) = block.expr else { return };
for stmt in block.stmts {
match stmt.kind {
StmtKind::Let(let_stmt) => self.visit_local(let_stmt),
StmtKind::Item(_) => {}
StmtKind::Expr(e) | StmtKind::Semi(e) => self.visit_expr(e),
}
}
if self.locals.is_empty() {
return;
}
LintTailExpr { cx: self.cx, locals: &self.locals, is_root_tail_expr: true }
.visit_expr(tail_expr);
}
}
struct LintTailExpr<'tcx, 'a> {
cx: &'a LateContext<'tcx>,
is_root_tail_expr: bool,
locals: &'a [Span],
}
impl<'tcx, 'a> LintTailExpr<'tcx, 'a> {
fn expr_eventually_point_into_local(mut expr: &Expr<'tcx>) -> bool {
loop {
match expr.kind {
ExprKind::Index(access, _, _) | ExprKind::Field(access, _) => expr = access,
ExprKind::AddrOf(_, _, referee) | ExprKind::Unary(UnOp::Deref, referee) => {
expr = referee
}
ExprKind::Path(_)
if let ExprKind::Path(QPath::Resolved(_, path)) = expr.kind
&& let [local, ..] = path.segments
&& let Res::Local(_) = local.res =>
{
return true;
}
_ => return false,
}
}
}
fn expr_generates_nonlocal_droppy_value(&self, expr: &Expr<'tcx>) -> bool {
if Self::expr_eventually_point_into_local(expr) {
return false;
}
self.cx.typeck_results().expr_ty(expr).has_significant_drop(self.cx.tcx, self.cx.param_env)
}
}
impl<'tcx, 'a> Visitor<'tcx> for LintTailExpr<'tcx, 'a> {
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if self.is_root_tail_expr {
self.is_root_tail_expr = false;
} else if self.expr_generates_nonlocal_droppy_value(expr) {
self.cx.tcx.emit_node_span_lint(
TAIL_EXPR_DROP_ORDER,
expr.hir_id,
expr.span,
TailExprDropOrderLint { spans: self.locals.to_vec() },
);
return;
}
match expr.kind {
ExprKind::Match(scrutinee, _, _) => self.visit_expr(scrutinee),
ExprKind::ConstBlock(_)
| ExprKind::Array(_)
| ExprKind::Break(_, _)
| ExprKind::Continue(_)
| ExprKind::Ret(_)
| ExprKind::Become(_)
| ExprKind::Yield(_, _)
| ExprKind::InlineAsm(_)
| ExprKind::If(_, _, _)
| ExprKind::Loop(_, _, _, _)
| ExprKind::Closure(_)
| ExprKind::DropTemps(_)
| ExprKind::OffsetOf(_, _)
| ExprKind::Assign(_, _, _)
| ExprKind::AssignOp(_, _, _)
| ExprKind::Lit(_)
| ExprKind::Err(_) => {}
ExprKind::MethodCall(_, _, _, _)
| ExprKind::Call(_, _)
| ExprKind::Type(_, _)
| ExprKind::Tup(_)
| ExprKind::Binary(_, _, _)
| ExprKind::Unary(_, _)
| ExprKind::Path(_)
| ExprKind::Let(_)
| ExprKind::Cast(_, _)
| ExprKind::Field(_, _)
| ExprKind::Index(_, _, _)
| ExprKind::AddrOf(_, _, _)
| ExprKind::Struct(_, _, _)
| ExprKind::Repeat(_, _) => intravisit::walk_expr(self, expr),
ExprKind::Block(_, _) => {
// We do not lint further because the drop order stays the same inside the block
}
}
}
fn visit_block(&mut self, block: &'tcx Block<'tcx>) {
LintVisitor { cx: self.cx, locals: <_>::default() }.check_block_inner(block);
}
}
#[derive(LintDiagnostic)]
#[diag(lint_tail_expr_drop_order)]
struct TailExprDropOrderLint {
#[label]
pub spans: Vec<Span>,
}

1
src/tools/lint-docs/src/lib.rs
View File

@ -444,6 +444,7 @@ fn generate_lint_output(
let mut cmd = Command::new(self.rustc_path);
if options.contains(&"edition2024") {
cmd.arg("--edition=2024");
cmd.arg("-Zunstable-options");
} else if options.contains(&"edition2021") {
cmd.arg("--edition=2021");
} else if options.contains(&"edition2018") {

35
tests/ui/drop/lint-tail-expr-drop-order-gated.rs Normal file
View File

@ -0,0 +1,35 @@
// This test ensures that `tail_expr_drop_order` does not activate in case Edition 2024 is not used
// or the feature gate `shorter_tail_lifetimes` is disabled.
//@ revisions: neither no_feature_gate edition_less_than_2024
//@ check-pass
//@ [neither] edition: 2021
//@ [no_feature_gate] compile-flags: -Z unstable-options
//@ [no_feature_gate] edition: 2024
//@ [edition_less_than_2024] edition: 2021
#![deny(tail_expr_drop_order)]
#![cfg_attr(edition_less_than_2024, feature(shorter_tail_lifetimes))]
struct LoudDropper;
impl Drop for LoudDropper {
fn drop(&mut self) {
// This destructor should be considered significant because it is a custom destructor
// and we will assume that the destructor can generate side effects arbitrarily so that
// a change in drop order is visible.
println!("loud drop");
}
}
impl LoudDropper {
fn get(&self) -> i32 {
0
}
}
fn should_not_lint() -> i32 {
let x = LoudDropper;
x.get() + LoudDropper.get()
// Lint should not action
}
fn main() {}

71
tests/ui/drop/lint-tail-expr-drop-order.rs Normal file
View File

@ -0,0 +1,71 @@
//@ compile-flags: -Z unstable-options
//@ edition: 2024
// Edition 2024 lint for change in drop order at tail expression
// This lint is to capture potential change in program semantics
// due to implementation of RFC 3606 <https://github.com/rust-lang/rfcs/pull/3606>
#![deny(tail_expr_drop_order)]
#![feature(shorter_tail_lifetimes)]
struct LoudDropper;
impl Drop for LoudDropper {
fn drop(&mut self) {
// This destructor should be considered significant because it is a custom destructor
// and we will assume that the destructor can generate side effects arbitrarily so that
// a change in drop order is visible.
println!("loud drop");
}
}
impl LoudDropper {
fn get(&self) -> i32 {
0
}
}
fn should_lint() -> i32 {
let x = LoudDropper;
// Should lint
x.get() + LoudDropper.get()
//~^ ERROR: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
//~| WARN: this changes meaning in Rust 2024
}
fn should_lint_closure() -> impl FnOnce() -> i32 {
let x = LoudDropper;
move || x.get() + LoudDropper.get()
//~^ ERROR: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
//~| WARN: this changes meaning in Rust 2024
}
fn should_not_lint() -> i32 {
let x = LoudDropper;
// Should not lint
x.get()
}
fn should_not_lint_in_nested_block() -> i32 {
let x = LoudDropper;
// Should not lint because Edition 2021 drops temporaries in blocks earlier already
{ LoudDropper.get() }
}
fn should_not_lint_in_match_arm() -> i32 {
let x = LoudDropper;
// Should not lint because Edition 2021 drops temporaries in blocks earlier already
match &x {
_ => LoudDropper.get(),
}
}
fn should_lint_in_nested_items() {
fn should_lint_me() -> i32 {
let x = LoudDropper;
// Should lint
x.get() + LoudDropper.get()
//~^ ERROR: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
//~| WARN: this changes meaning in Rust 2024
}
}
fn main() {}

42
tests/ui/drop/lint-tail-expr-drop-order.stderr Normal file
View File

@ -0,0 +1,42 @@
error: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
--> $DIR/lint-tail-expr-drop-order.rs:29:15
|
LL | let x = LoudDropper;
| - these values have significant drop implementation and will observe changes in drop order under Edition 2024
LL | // Should lint
LL | x.get() + LoudDropper.get()
| ^^^^^^^^^^^
|
= warning: this changes meaning in Rust 2024
= note: for more information, see issue #123739 <https://github.com/rust-lang/rust/issues/123739>
note: the lint level is defined here
--> $DIR/lint-tail-expr-drop-order.rs:8:9
|
LL | #![deny(tail_expr_drop_order)]
| ^^^^^^^^^^^^^^^^^^^^
error: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
--> $DIR/lint-tail-expr-drop-order.rs:36:23
|
LL | let x = LoudDropper;
| - these values have significant drop implementation and will observe changes in drop order under Edition 2024
LL | move || x.get() + LoudDropper.get()
| ^^^^^^^^^^^
|
= warning: this changes meaning in Rust 2024
= note: for more information, see issue #123739 <https://github.com/rust-lang/rust/issues/123739>
error: these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
--> $DIR/lint-tail-expr-drop-order.rs:65:19
|
LL | let x = LoudDropper;
| - these values have significant drop implementation and will observe changes in drop order under Edition 2024
LL | // Should lint
LL | x.get() + LoudDropper.get()
| ^^^^^^^^^^^
|
= warning: this changes meaning in Rust 2024
= note: for more information, see issue #123739 <https://github.com/rust-lang/rust/issues/123739>
error: aborting due to 3 previous errors