diff --git a/Cargo.lock b/Cargo.lock index 5e9808c806884..29ba079cf9524 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4090,6 +4090,7 @@ dependencies = [ "rustc_hir", "rustc_index", "rustc_infer", + "rustc_lint", "rustc_macros", "rustc_middle", "rustc_mir_build", diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index a11eca0b9c746..ab3dffb534024 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -691,7 +691,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R> TerminatorKind::SwitchInt { discr, targets: _ } => { self.consume_operand(loc, (discr, span), state); } - TerminatorKind::Drop { place, target: _, unwind: _, replace } => { + TerminatorKind::Drop { place, target: _, scope: _, unwind: _, replace } => { debug!( "visit_terminator_drop \ loc: {:?} term: {:?} place: {:?} span: {:?}", diff --git a/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs index afd811a0efb43..35178fed160a5 100644 --- a/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs +++ b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs @@ -103,7 +103,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'a, 'tcx> { TerminatorKind::SwitchInt { discr, targets: _ } => { self.consume_operand(location, discr); } - TerminatorKind::Drop { place: drop_place, target: _, unwind: _, replace } => { + TerminatorKind::Drop { place: drop_place, target: _, scope: _, unwind: _, replace } => { let write_kind = if *replace { WriteKind::Replace } else { WriteKind::StorageDeadOrDrop }; self.access_place( diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 1ce0aacab4998..9d4a71826a4e5 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -549,7 +549,7 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) { | TerminatorKind::CoroutineDrop => { bug!("shouldn't exist at codegen {:?}", bb_data.terminator()); } - TerminatorKind::Drop { place, target, unwind: _, replace: _ } => { + TerminatorKind::Drop { place, target, unwind: _, scope: _, replace: _ } => { let drop_place = codegen_place(fx, *place); crate::abi::codegen_drop(fx, source_info, drop_place, *target); } diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 125d3b908c733..fbd7083686229 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -1322,7 +1322,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { MergingSucc::False } - mir::TerminatorKind::Drop { place, target, unwind, replace: _ } => self + mir::TerminatorKind::Drop { place, target, unwind, scope: _, replace: _ } => self .codegen_drop_terminator( helper, bx, diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 8e2367e0d214c..2175eb84b9ad0 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -528,7 +528,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { } } - Drop { place, target, unwind, replace: _ } => { + Drop { place, target, unwind, scope: _, replace: _ } => { let place = self.eval_place(place)?; let instance = Instance::resolve_drop_in_place(*self.tcx, place.layout.ty); if let ty::InstanceKind::DropGlue(_, None) = instance.def { diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index c2b9cae680bf1..9edbeee605c0a 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -460,6 +460,10 @@ impl ChunkedBitSet { self.chunks.iter().map(|chunk| chunk.count()).sum() } + pub fn is_empty(&self) -> bool { + self.chunks.iter().all(|chunk| matches!(chunk, Chunk::Zeros(..) | Chunk::Ones(0))) + } + /// Returns `true` if `self` contains `elem`. #[inline] pub fn contains(&self, elem: T) -> bool { @@ -672,8 +676,60 @@ impl BitRelations> for ChunkedBitSet { unimplemented!("implement if/when necessary"); } - fn intersect(&mut self, _other: &ChunkedBitSet) -> bool { - unimplemented!("implement if/when necessary"); + fn intersect(&mut self, other: &ChunkedBitSet) -> bool { + assert_eq!(self.domain_size, other.domain_size); + debug_assert_eq!(self.chunks.len(), other.chunks.len()); + + let mut changed = false; + for (mut self_chunk, other_chunk) in self.chunks.iter_mut().zip(other.chunks.iter()) { + match (&mut self_chunk, &other_chunk) { + (Zeros(..), _) | (_, Ones(..)) => {} + ( + Ones(self_chunk_domain_size), + Zeros(other_chunk_domain_size) | Mixed(other_chunk_domain_size, ..), + ) + | (Mixed(self_chunk_domain_size, ..), Zeros(other_chunk_domain_size)) => { + debug_assert_eq!(self_chunk_domain_size, other_chunk_domain_size); + changed = true; + *self_chunk = other_chunk.clone(); + } + ( + Mixed( + self_chunk_domain_size, + ref mut self_chunk_count, + ref mut self_chunk_words, + ), + Mixed(_other_chunk_domain_size, _other_chunk_count, other_chunk_words), + ) => { + // See [`>>::union`] for the explanation + let op = |a, b| a & b; + let num_words = num_words(*self_chunk_domain_size as usize); + if bitwise_changes( + &self_chunk_words[0..num_words], + &other_chunk_words[0..num_words], + op, + ) { + let self_chunk_words = Rc::make_mut(self_chunk_words); + let has_changed = bitwise( + &mut self_chunk_words[0..num_words], + &other_chunk_words[0..num_words], + op, + ); + debug_assert!(has_changed); + *self_chunk_count = self_chunk_words[0..num_words] + .iter() + .map(|w| w.count_ones() as ChunkSize) + .sum(); + if *self_chunk_count == 0 { + *self_chunk = Zeros(*self_chunk_domain_size); + } + changed = true; + } + } + } + } + + changed } } diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 375cfccbe9f71..76abbf355a542 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -778,9 +778,6 @@ 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}` diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index c74cb866f21fa..9b145535959ce 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -82,7 +82,6 @@ mod redundant_semicolon; mod reference_casting; mod shadowed_into_iter; mod static_mut_refs; -mod tail_expr_drop_order; mod traits; mod types; mod unit_bindings; @@ -123,7 +122,6 @@ use rustc_middle::ty::TyCtxt; use shadowed_into_iter::ShadowedIntoIter; pub use shadowed_into_iter::{ARRAY_INTO_ITER, BOXED_SLICE_INTO_ITER}; use static_mut_refs::*; -use tail_expr_drop_order::TailExprDropOrder; use traits::*; use types::*; use unit_bindings::*; @@ -248,7 +246,6 @@ late_lint_methods!( AsyncFnInTrait: AsyncFnInTrait, NonLocalDefinitions: NonLocalDefinitions::default(), ImplTraitOvercaptures: ImplTraitOvercaptures, - TailExprDropOrder: TailExprDropOrder, IfLetRescope: IfLetRescope::default(), StaticMutRefs: StaticMutRefs, UnqualifiedLocalImports: UnqualifiedLocalImports, diff --git a/compiler/rustc_lint/src/tail_expr_drop_order.rs b/compiler/rustc_lint/src/tail_expr_drop_order.rs deleted file mode 100644 index 44a36142ed480..0000000000000 --- a/compiler/rustc_lint/src/tail_expr_drop_order.rs +++ /dev/null @@ -1,306 +0,0 @@ -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::Span; -use rustc_span::edition::Edition; - -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 ", - }; -} - -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<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - // We only record locals that have significant drops - locals: Vec, -} - -struct LocalCollector<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - locals: &'a mut Vec, -} - -impl<'a, 'tcx> Visitor<'tcx> for LocalCollector<'a, 'tcx> { - 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<'a, 'tcx> Visitor<'tcx> for LintVisitor<'a, 'tcx> { - 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<'a, 'tcx> LintVisitor<'a, 'tcx> { - 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<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - is_root_tail_expr: bool, - locals: &'a [Span], -} - -impl<'a, 'tcx> LintTailExpr<'a, 'tcx> { - 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<'a, 'tcx> Visitor<'tcx> for LintTailExpr<'a, 'tcx> { - 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, -} diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 549dc64a562b2..a4f9a4dd3730e 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -99,6 +99,7 @@ declare_lint_pass! { SINGLE_USE_LIFETIMES, SOFT_UNSTABLE, STABLE_FEATURES, + TAIL_EXPR_DROP_ORDER, TEST_UNSTABLE_LINT, TEXT_DIRECTION_CODEPOINT_IN_COMMENT, TRIVIAL_CASTS, @@ -4998,3 +4999,81 @@ declare_lint! { reference: "issue #124535 ", }; } + +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 + /// #![cfg_attr(not(bootstrap), feature(shorter_tail_lifetimes))] // Simplify this in bootstrap bump. + /// #![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 ", + }; +} diff --git a/compiler/rustc_middle/src/arena.rs b/compiler/rustc_middle/src/arena.rs index 7050a06b8dc22..c0461266de27e 100644 --- a/compiler/rustc_middle/src/arena.rs +++ b/compiler/rustc_middle/src/arena.rs @@ -114,6 +114,7 @@ macro_rules! arena_types { [decode] specialization_graph: rustc_middle::traits::specialization_graph::Graph, [] crate_inherent_impls: rustc_middle::ty::CrateInherentImpls, [] hir_owner_nodes: rustc_hir::OwnerNodes<'tcx>, + [] hir_id_set: rustc_hir::HirIdSet, ]); ) } diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index ae75f2d4187ba..e44fbbe19835c 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -5,8 +5,8 @@ use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece, Mutability}; use rustc_data_structures::packed::Pu128; -use rustc_hir::CoroutineKind; use rustc_hir::def_id::DefId; +use rustc_hir::{CoroutineKind, ItemLocalId}; use rustc_index::IndexVec; use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable}; use rustc_span::Span; @@ -705,7 +705,15 @@ pub enum TerminatorKind<'tcx> { /// The `replace` flag indicates whether this terminator was created as part of an assignment. /// This should only be used for diagnostic purposes, and does not have any operational /// meaning. - Drop { place: Place<'tcx>, target: BasicBlock, unwind: UnwindAction, replace: bool }, + Drop { + place: Place<'tcx>, + target: BasicBlock, + unwind: UnwindAction, + /// The HIR region scope that this drop is issued for when the evaluation exits the said scope. + /// It is provided at best effort and may come from foreign sources due to inlining. + scope: Option<(DefId, ItemLocalId)>, + replace: bool, + }, /// Roughly speaking, evaluates the `func` operand and the arguments, and starts execution of /// the referred to function. The operand types must match the argument types of the function. diff --git a/compiler/rustc_middle/src/mir/terminator.rs b/compiler/rustc_middle/src/mir/terminator.rs index 783952fb9cb8a..7711d1d7b8c05 100644 --- a/compiler/rustc_middle/src/mir/terminator.rs +++ b/compiler/rustc_middle/src/mir/terminator.rs @@ -631,7 +631,7 @@ impl<'tcx> TerminatorKind<'tcx> { Goto { target } => TerminatorEdges::Single(target), Assert { target, unwind, expected: _, msg: _, cond: _ } - | Drop { target, unwind, place: _, replace: _ } + | Drop { target, unwind, place: _, scope: _, replace: _ } | FalseUnwind { real_target: target, unwind } => match unwind { UnwindAction::Cleanup(unwind) => TerminatorEdges::Double(target, unwind), UnwindAction::Continue | UnwindAction::Terminate(_) | UnwindAction::Unreachable => { diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 64898a8495e26..7d65d8a0a98cc 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -511,6 +511,7 @@ macro_rules! make_mir_visitor { target: _, unwind: _, replace: _, + scope: _, } => { self.visit_place( place, diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index cd9ff9b60d859..e017b71c7944c 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -264,6 +264,7 @@ TrivialTypeTraversalImpls! { // interners). TrivialTypeTraversalAndLiftImpls! { ::rustc_hir::def_id::DefId, + ::rustc_hir::hir_id::ItemLocalId, ::rustc_hir::Safety, ::rustc_target::spec::abi::Abi, crate::ty::ClosureKind, diff --git a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs index 9e3af89105294..9cd1e3f1574b3 100644 --- a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs +++ b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs @@ -70,6 +70,7 @@ impl<'a, 'tcx> ParseCtxt<'a, 'tcx> { target: self.parse_return_to(args[1])?, unwind: self.parse_unwind_action(args[2])?, replace: false, + scope: None, }) }, @call(mir_call, args) => { diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index fd949a533845e..dd7a3f0ec177b 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -716,11 +716,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); if let Operand::Move(to_drop) = value_operand { let success = this.cfg.start_new_block(); + let scope = scope + .and_then(|scope| scope.hir_id(this.region_scope_tree)) + .map(|scope| (scope.owner.to_def_id(), scope.local_id)); this.cfg.terminate(block, outer_source_info, TerminatorKind::Drop { place: to_drop, target: success, unwind: UnwindAction::Continue, replace: false, + scope, }); this.diverge_from(block); block = success; diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index dfc82f705a81b..755e4845816b5 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -151,6 +151,9 @@ struct DropData { /// Whether this is a value Drop or a StorageDead. kind: DropKind, + + /// Scope for which the data is obliged to drop + scope: Option, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -274,8 +277,12 @@ impl DropTree { // represents the block in the tree that should be jumped to once all // of the required drops have been performed. let fake_source_info = SourceInfo::outermost(DUMMY_SP); - let fake_data = - DropData { source_info: fake_source_info, local: Local::MAX, kind: DropKind::Storage }; + let fake_data = DropData { + source_info: fake_source_info, + local: Local::MAX, + kind: DropKind::Storage, + scope: None, + }; let drops = IndexVec::from_raw(vec![DropNode { data: fake_data, next: DropIdx::MAX }]); Self { drops, entry_points: Vec::new(), existing_drops_map: FxHashMap::default() } } @@ -311,12 +318,13 @@ impl DropTree { &mut self, cfg: &mut CFG<'tcx>, blocks: &mut IndexVec>, + scope_tree: ®ion::ScopeTree, ) { debug!("DropTree::build_mir(drops = {:#?})", self); assert_eq!(blocks.len(), self.drops.len()); self.assign_blocks::(cfg, blocks); - self.link_blocks(cfg, blocks) + self.link_blocks(cfg, blocks, scope_tree) } /// Assign blocks for all of the drops in the drop tree that need them. @@ -390,17 +398,24 @@ impl DropTree { &self, cfg: &mut CFG<'tcx>, blocks: &IndexSlice>, + scope_tree: ®ion::ScopeTree, ) { for (drop_idx, drop_node) in self.drops.iter_enumerated().rev() { let Some(block) = blocks[drop_idx] else { continue }; match drop_node.data.kind { DropKind::Value => { + let scope = drop_node + .data + .scope + .and_then(|scope| scope.hir_id(scope_tree)) + .map(|scope| (scope.owner.to_def_id(), scope.local_id)); let terminator = TerminatorKind::Drop { target: blocks[drop_node.next].unwrap(), // The caller will handle this if needed. unwind: UnwindAction::Terminate(UnwindTerminateReason::InCleanup), place: drop_node.data.local.into(), replace: false, + scope, }; cfg.terminate(block, drop_node.data.source_info, terminator); } @@ -763,7 +778,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let local = place.as_local().unwrap_or_else(|| bug!("projection in tail call args")); - Some(DropData { source_info, local, kind: DropKind::Value }) + Some(DropData { source_info, local, kind: DropKind::Value, scope: None }) } Operand::Constant(_) => None, }) @@ -802,11 +817,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { unwind_drops.add_entry_point(block, unwind_entry_point); let next = self.cfg.start_new_block(); + let scope = scope + .region_scope + .hir_id(self.region_scope_tree) + .map(|scope| (scope.owner.to_def_id(), scope.local_id)); self.cfg.terminate(block, source_info, TerminatorKind::Drop { place: local.into(), target: next, unwind: UnwindAction::Continue, replace: false, + scope, }); block = next; } @@ -841,6 +861,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { unwind_to, is_coroutine && needs_cleanup, self.arg_count, + self.region_scope_tree, ) .into_block() } @@ -1089,6 +1110,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { source_info: SourceInfo { span: scope_end, scope: scope.source_scope }, local, kind: drop_kind, + scope: Some(region_scope), }); return; @@ -1280,6 +1302,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { target: assign, unwind: UnwindAction::Cleanup(assign_unwind), replace: true, + scope: None, }); self.diverge_from(block); @@ -1335,6 +1358,7 @@ fn build_scope_drops<'tcx>( mut unwind_to: DropIdx, storage_dead_on_unwind: bool, arg_count: usize, + scope_tree: ®ion::ScopeTree, ) -> BlockAnd<()> { debug!("build_scope_drops({:?} -> {:?})", block, scope); @@ -1381,11 +1405,16 @@ fn build_scope_drops<'tcx>( unwind_drops.add_entry_point(block, unwind_to); let next = cfg.start_new_block(); + let scope = drop_data + .scope + .and_then(|scope| scope.hir_id(scope_tree)) + .map(|scope| (scope.owner.to_def_id(), scope.local_id)); cfg.terminate(block, source_info, TerminatorKind::Drop { place: local.into(), target: next, unwind: UnwindAction::Continue, replace: false, + scope, }); block = next; } @@ -1419,7 +1448,7 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { let mut blocks = IndexVec::from_elem(None, &drops.drops); blocks[ROOT_NODE] = continue_block; - drops.build_mir::(&mut self.cfg, &mut blocks); + drops.build_mir::(&mut self.cfg, &mut blocks, self.region_scope_tree); let is_coroutine = self.coroutine.is_some(); // Link the exit drop tree to unwind drop tree. @@ -1466,6 +1495,7 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { &mut self.scopes.unwind_drops, self.fn_span, &mut None, + self.region_scope_tree, ); } } @@ -1476,7 +1506,7 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { let cfg = &mut self.cfg; let fn_span = self.fn_span; let mut blocks = IndexVec::from_elem(None, &drops.drops); - drops.build_mir::(cfg, &mut blocks); + drops.build_mir::(cfg, &mut blocks, self.region_scope_tree); if let Some(root_block) = blocks[ROOT_NODE] { cfg.terminate( root_block, @@ -1488,7 +1518,7 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { // Build the drop tree for unwinding in the normal control flow paths. let resume_block = &mut None; let unwind_drops = &mut self.scopes.unwind_drops; - Self::build_unwind_tree(cfg, unwind_drops, fn_span, resume_block); + Self::build_unwind_tree(cfg, unwind_drops, fn_span, resume_block, self.region_scope_tree); // Build the drop tree for unwinding when dropping a suspended // coroutine. @@ -1503,7 +1533,7 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { drops.entry_points.push((drop_node.next, blocks[drop_idx].unwrap())); } } - Self::build_unwind_tree(cfg, drops, fn_span, resume_block); + Self::build_unwind_tree(cfg, drops, fn_span, resume_block, self.region_scope_tree); } fn build_unwind_tree( @@ -1511,10 +1541,11 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { drops: &mut DropTree, fn_span: Span, resume_block: &mut Option, + scope_tree: ®ion::ScopeTree, ) { let mut blocks = IndexVec::from_elem(None, &drops.drops); blocks[ROOT_NODE] = *resume_block; - drops.build_mir::(cfg, &mut blocks); + drops.build_mir::(cfg, &mut blocks, scope_tree); if let (None, Some(resume)) = (*resume_block, blocks[ROOT_NODE]) { cfg.terminate(resume, SourceInfo::outermost(fn_span), TerminatorKind::UnwindResume); diff --git a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs index f2541c37167f0..3e15d91dedca8 100644 --- a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs +++ b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs @@ -1,5 +1,7 @@ use std::{fmt, iter}; +use rustc_hir::ItemLocalId; +use rustc_hir::def_id::DefId; use rustc_hir::lang_items::LangItem; use rustc_index::Idx; use rustc_middle::mir::patch::MirPatch; @@ -167,6 +169,10 @@ where path: D::Path, succ: BasicBlock, unwind: Unwind, + + /// Scope from which the drop is obliged + /// which is the HIR node for the "scope" + scope: Option<(DefId, ItemLocalId)>, } /// "Elaborates" a drop of `place`/`path` and patches `bb`'s terminator to execute it. @@ -185,11 +191,12 @@ pub fn elaborate_drop<'b, 'tcx, D>( succ: BasicBlock, unwind: Unwind, bb: BasicBlock, + scope: Option<(DefId, ItemLocalId)>, ) where D: DropElaborator<'b, 'tcx>, 'tcx: 'b, { - DropCtxt { elaborator, source_info, place, path, succ, unwind }.elaborate_drop(bb) + DropCtxt { elaborator, source_info, place, scope, path, succ, unwind }.elaborate_drop(bb) } impl<'a, 'b, 'tcx, D> DropCtxt<'a, 'b, 'tcx, D> @@ -238,6 +245,7 @@ where target: self.succ, unwind: self.unwind.into_action(), replace: false, + scope: self.scope, }); } DropStyle::Conditional => { @@ -299,6 +307,7 @@ where place, succ, unwind, + scope: self.scope, } .elaborated_drop_block() } else { @@ -313,6 +322,7 @@ where // Using `self.path` here to condition the drop on // our own drop flag. path: self.path, + scope: self.scope, } .complete_drop(succ, unwind) } @@ -734,6 +744,7 @@ where target: loop_block, unwind: unwind.into_action(), replace: false, + scope: self.scope, }); loop_block @@ -914,6 +925,7 @@ where target, unwind: unwind.into_action(), replace: false, + scope: self.scope, }; self.new_block(unwind, block) } diff --git a/compiler/rustc_mir_dataflow/src/impls/initialized.rs b/compiler/rustc_mir_dataflow/src/impls/initialized.rs index 25e8726cf6162..6e1d5bff30331 100644 --- a/compiler/rustc_mir_dataflow/src/impls/initialized.rs +++ b/compiler/rustc_mir_dataflow/src/impls/initialized.rs @@ -368,7 +368,8 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { ) -> TerminatorEdges<'mir, 'tcx> { let mut edges = terminator.edges(); if self.skip_unreachable_unwind - && let mir::TerminatorKind::Drop { target, unwind, place, replace: _ } = terminator.kind + && let mir::TerminatorKind::Drop { target, unwind, place, scope: _, replace: _ } = + terminator.kind && matches!(unwind, mir::UnwindAction::Cleanup(_)) && self.is_unwind_dead(place, state) { diff --git a/compiler/rustc_mir_transform/Cargo.toml b/compiler/rustc_mir_transform/Cargo.toml index 07ca51a67aefb..f911d738810cb 100644 --- a/compiler/rustc_mir_transform/Cargo.toml +++ b/compiler/rustc_mir_transform/Cargo.toml @@ -17,6 +17,7 @@ rustc_fluent_macro = { path = "../rustc_fluent_macro" } rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } rustc_infer = { path = "../rustc_infer" } +rustc_lint = { path = "../rustc_lint" } rustc_macros = { path = "../rustc_macros" } rustc_middle = { path = "../rustc_middle" } rustc_mir_build = { path = "../rustc_mir_build" } diff --git a/compiler/rustc_mir_transform/messages.ftl b/compiler/rustc_mir_transform/messages.ftl index f9b79d72b0504..78e8062275d90 100644 --- a/compiler/rustc_mir_transform/messages.ftl +++ b/compiler/rustc_mir_transform/messages.ftl @@ -23,6 +23,9 @@ mir_transform_must_not_suspend = {$pre}`{$def_path}`{$post} held across a suspen .help = consider using a block (`{"{ ... }"}`) to shrink the value's scope, ending before the suspend point mir_transform_operation_will_panic = this operation will panic at runtime +mir_transform_tail_expr_drop_order = this value of type `{$ty}` has significant drop implementation that will have a different drop order from that of Edition 2021 + .label = these local bindings with significant drop implementation may observe changes in drop order under Edition 2024 + mir_transform_unaligned_packed_ref = reference to packed field is unaligned .note = packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses .note_ub = creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) diff --git a/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs b/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs index 559df222a5040..fe87b807d6bb6 100644 --- a/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs +++ b/compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs @@ -79,7 +79,8 @@ fn add_move_for_packed_drop<'tcx>( is_cleanup: bool, ) { debug!("add_move_for_packed_drop({:?} @ {:?})", terminator, loc); - let TerminatorKind::Drop { ref place, target, unwind, replace } = terminator.kind else { + let TerminatorKind::Drop { ref place, target, unwind, scope: _, replace } = terminator.kind + else { unreachable!(); }; @@ -100,5 +101,6 @@ fn add_move_for_packed_drop<'tcx>( target: storage_dead_block, unwind, replace, + scope: None, }); } diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index 8f032728f6be0..3696147395cdd 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -1083,15 +1083,15 @@ fn elaborate_coroutine_drops<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let mut elaborator = DropShimElaborator { body, patch: MirPatch::new(body), tcx, param_env }; for (block, block_data) in body.basic_blocks.iter_enumerated() { - let (target, unwind, source_info) = match block_data.terminator() { + let (target, unwind, source_info, &scope) = match block_data.terminator() { Terminator { source_info, - kind: TerminatorKind::Drop { place, target, unwind, replace: _ }, + kind: TerminatorKind::Drop { place, target, unwind, scope, replace: _ }, } => { if let Some(local) = place.as_local() && local == SELF_ARG { - (target, unwind, source_info) + (target, unwind, source_info, scope) } else { continue; } @@ -1116,6 +1116,7 @@ fn elaborate_coroutine_drops<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { *target, unwind, block, + scope, ); } elaborator.patch.apply(body); @@ -1374,6 +1375,7 @@ fn insert_clean_drop(body: &mut Body<'_>) -> BasicBlock { target: return_block, unwind: UnwindAction::Continue, replace: false, + scope: None, }; let source_info = SourceInfo::outermost(body.span); diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index c35aec42408ca..ce55c93c4701d 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -330,7 +330,8 @@ impl<'a, 'tcx> ElaborateDropsCtxt<'a, 'tcx> { // This function should mirror what `collect_drop_flags` does. for (bb, data) in self.body.basic_blocks.iter_enumerated() { let terminator = data.terminator(); - let TerminatorKind::Drop { place, target, unwind, replace } = terminator.kind else { + let TerminatorKind::Drop { place, target, unwind, scope, replace } = terminator.kind + else { continue; }; @@ -366,7 +367,16 @@ impl<'a, 'tcx> ElaborateDropsCtxt<'a, 'tcx> { } }; self.init_data.seek_before(self.body.terminator_loc(bb)); - elaborate_drop(self, terminator.source_info, place, path, target, unwind, bb) + elaborate_drop( + self, + terminator.source_info, + place, + path, + target, + unwind, + bb, + scope, + ) } LookupResult::Parent(None) => {} LookupResult::Parent(Some(_)) => { diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index c9f24764cc2a1..a9718fabbf868 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -535,7 +535,9 @@ impl<'tcx> Inliner<'tcx> { checker.visit_basic_block_data(bb, blk); let term = blk.terminator(); - if let TerminatorKind::Drop { ref place, target, unwind, replace: _ } = term.kind { + if let TerminatorKind::Drop { ref place, target, unwind, scope: _, replace: _ } = + term.kind + { work_list.push(target); // If the place doesn't actually need dropping, treat it like a regular goto. diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 4c090665992bd..5572ce3af2f2a 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -103,6 +103,7 @@ mod sanity_check; mod shim; mod ssa; // This pass is public to allow external drivers to perform MIR cleanup +mod lint_tail_expr_drop_order; pub mod simplify; mod simplify_branches; mod simplify_comparison_integral; @@ -356,6 +357,7 @@ fn mir_promoted( &[&promote_pass, &simplify::SimplifyCfg::PromoteConsts, &coverage::InstrumentCoverage], Some(MirPhase::Analysis(AnalysisPhase::Initial)), ); + lint_tail_expr_drop_order::run_lint(tcx, def, &body); let promoted = promote_pass.promoted_fragments.into_inner(); (tcx.alloc_steal_mir(body), tcx.alloc_steal_promoted(promoted)) diff --git a/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs b/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs new file mode 100644 index 0000000000000..bd4dfbe70b38f --- /dev/null +++ b/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs @@ -0,0 +1,279 @@ +use rustc_hir::def_id::LocalDefId; +use rustc_hir::{ExprKind, HirId, HirIdSet, OwnerId}; +use rustc_index::IndexVec; +use rustc_index::bit_set::{BitSet, ChunkedBitSet}; +use rustc_lint::{self as lint, Level}; +use rustc_macros::LintDiagnostic; +use rustc_middle::mir::{BasicBlock, Body, Local, Location, TerminatorKind, dump_mir}; +use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_mir_dataflow::impls::MaybeInitializedPlaces; +use rustc_mir_dataflow::move_paths::MoveData; +use rustc_mir_dataflow::{Analysis, MaybeReachable}; +use rustc_span::Span; +use tracing::debug; + +fn blocks_reachable_from_tail_expr_rev(body: &Body<'_>, blocks: &mut BitSet) { + let mut new_blocks = blocks.clone(); + let predecessors = body.basic_blocks.predecessors(); + while !new_blocks.is_empty() { + let mut next_front = BitSet::new_empty(new_blocks.domain_size()); + for bb in new_blocks.iter() { + for &pred in &predecessors[bb] { + if body.basic_blocks[pred].is_cleanup { + continue; + } + if blocks.insert(pred) { + next_front.insert(pred); + } + } + } + new_blocks = next_front; + } +} + +fn blocks_reachable_from_tail_expr_fwd(body: &Body<'_>, blocks: &mut BitSet) { + let mut new_blocks = blocks.clone(); + while !new_blocks.is_empty() { + let mut next_front = BitSet::new_empty(new_blocks.domain_size()); + for bb in new_blocks.iter() { + for succ in body.basic_blocks[bb].terminator.iter().flat_map(|term| term.successors()) { + if body.basic_blocks[succ].is_cleanup { + continue; + } + if blocks.insert(succ) { + next_front.insert(succ); + } + } + } + new_blocks = next_front; + } +} + +fn is_descendent_of_hir_id( + tcx: TyCtxt<'_>, + id: HirId, + root: HirId, + descendants: &mut HirIdSet, + filter: &mut HirIdSet, +) -> bool { + let mut path = vec![]; + for id in [id].into_iter().chain(tcx.hir().parent_id_iter(id)) { + if filter.contains(&id) { + filter.extend(path); + return false; + } else if root == id || descendants.contains(&id) { + descendants.extend(path); + return true; + } + path.push(id); + } + filter.extend(path); + false +} + +pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body<'tcx>) { + if matches!(tcx.def_kind(def_id), rustc_hir::def::DefKind::SyntheticCoroutineBody) { + // A synthetic coroutine has no HIR body and it is enough to just analyse the original body + return; + } + let (tail_expr_hir_id, tail_expr_span) = { + let expr = tcx.hir().body_owned_by(def_id).value; + match expr.kind { + ExprKind::Block(block, _) => { + if let Some(expr) = block.expr { + (expr.hir_id, expr.span) + } else { + // There is no tail expression + return; + } + } + _ => (expr.hir_id, expr.span), + } + }; + if tail_expr_span.edition().at_least_rust_2024() { + // We should stop linting since Edition 2024 + return; + } + if let (Level::Allow, _) = + tcx.lint_level_at_node(lint::builtin::TAIL_EXPR_DROP_ORDER, tail_expr_hir_id) + { + // Analysis is expensive, so let's skip if the lint is suppressed anyway + return; + } + debug!(?def_id, ?tail_expr_span); + dump_mir(tcx, false, "lint_drop_order", &0 as _, body, |pass_where, writer| match pass_where { + rustc_middle::mir::PassWhere::AfterLocation(loc) => { + if loc.statement_index < body.basic_blocks[loc.block].statements.len() { + writeln!( + writer, + "@ {:?}", + body.basic_blocks[loc.block].statements[loc.statement_index].source_info.span + ) + } else { + Ok(()) + } + } + rustc_middle::mir::PassWhere::AfterTerminator(term) => { + writeln!(writer, "@ {:?}", body.basic_blocks[term].terminator().source_info.span) + } + _ => Ok(()), + }); + let mut non_tail_expr_parent: HirIdSet = tcx.hir().parent_id_iter(tail_expr_hir_id).collect(); + let mut tail_expr_descendants: HirIdSet = [tail_expr_hir_id].into_iter().collect(); + + let param_env = tcx.param_env(def_id); + let is_closure_like = tcx.is_closure_like(def_id.to_def_id()); + + let nr_blocks = body.basic_blocks.len(); + let mut tail_expr_blocks = BitSet::new_empty(nr_blocks); + let mut blocks_term_in_tail_expr = BitSet::new_empty(nr_blocks); + let mut tail_expr_stmts = IndexVec::from_elem_n(0, nr_blocks); + for (bb, data) in body.basic_blocks.iter_enumerated() { + if data.is_cleanup { + continue; + } + if let Some(terminator) = &data.terminator + && let TerminatorKind::Drop { scope: Some((def_id, local_id)), .. } = terminator.kind + && let Some(def_id) = def_id.as_local() + { + let hir_id = HirId { owner: OwnerId { def_id }, local_id }; + if is_descendent_of_hir_id( + tcx, + hir_id, + tail_expr_hir_id, + &mut tail_expr_descendants, + &mut non_tail_expr_parent, + ) { + tail_expr_blocks.insert(bb); + blocks_term_in_tail_expr.insert(bb); + continue; + } + } + for (idx, stmt) in data.statements.iter().enumerate().rev() { + if tail_expr_span.contains(stmt.source_info.span) { + tail_expr_blocks.insert(bb); + tail_expr_stmts[bb] = idx; + break; + } + } + } + let mut exit_tail_expr_blocks = tail_expr_blocks.clone(); + debug!(?tail_expr_blocks, "before reachability"); + blocks_reachable_from_tail_expr_rev(body, &mut tail_expr_blocks); + debug!(?tail_expr_blocks, "reachable bb to tail expr"); + blocks_reachable_from_tail_expr_fwd(body, &mut exit_tail_expr_blocks); + debug!(?exit_tail_expr_blocks, "exit reachability"); + exit_tail_expr_blocks.subtract(&tail_expr_blocks); + debug!(?exit_tail_expr_blocks, "exit - rev"); + + let mut tail_expr_exit_boundary_blocks = BitSet::new_empty(nr_blocks); + let mut boundary_lies_on_statement = BitSet::new_empty(nr_blocks); + 'boundary_block: for (bb, block) in + tail_expr_blocks.iter().map(|bb| (bb, &body.basic_blocks[bb])) + { + for succ in block.terminator.iter().flat_map(|term| term.successors()) { + if exit_tail_expr_blocks.contains(succ) { + if blocks_term_in_tail_expr.contains(bb) { + tail_expr_exit_boundary_blocks.insert(succ); + } else { + boundary_lies_on_statement.insert(bb); + tail_expr_exit_boundary_blocks.insert(bb); + continue 'boundary_block; + } + } + } + } + debug!(?tail_expr_exit_boundary_blocks); + + let move_data = MoveData::gather_moves(body, tcx, param_env, |_| true); + let mut droppy_paths = ChunkedBitSet::new_empty(move_data.move_paths.len()); + let mut droppy_paths_not_in_tail = droppy_paths.clone(); + for (idx, path) in move_data.move_paths.iter_enumerated() { + if path.place.ty(&body.local_decls, tcx).ty.has_significant_drop(tcx, param_env) { + droppy_paths.insert(idx); + if !tail_expr_span.contains(body.local_decls[path.place.local].source_info.span) { + droppy_paths_not_in_tail.insert(idx); + } + } + } + let maybe_init = MaybeInitializedPlaces::new(tcx, body, &move_data); + let mut maybe_init = + maybe_init.into_engine(tcx, body).iterate_to_fixpoint().into_results_cursor(body); + let mut observables = ChunkedBitSet::new_empty(move_data.move_paths.len()); + for block in tail_expr_exit_boundary_blocks.iter() { + debug!(?observables); + if boundary_lies_on_statement.contains(block) { + let target = Location { block, statement_index: tail_expr_stmts[block] }; + debug!(?target, "in block"); + maybe_init.seek_after_primary_effect(target); + } else { + maybe_init.seek_to_block_start(block); + } + if let MaybeReachable::Reachable(alive) = maybe_init.get() { + debug!(?block, ?alive); + let mut mask = droppy_paths.clone(); + mask.intersect(alive); + observables.union(&mask); + } + } + debug!(?observables); + // A linted local has a span contained in the tail expression span + let Some(linted_move_path) = observables + .iter() + .map(|path| &move_data.move_paths[path]) + .filter(|move_path| { + if move_path.place.local == Local::ZERO { + return false; + } + if is_closure_like && matches!(move_path.place.local, ty::CAPTURE_STRUCT_LOCAL) { + return false; + } + tail_expr_span.contains(body.local_decls[move_path.place.local].source_info.span) + }) + .nth(0) + else { + debug!("nothing to lint"); + return; + }; + debug!(?linted_move_path); + let body_observables: Vec<_> = observables + .iter() + .filter_map(|idx| { + // We want to lint on place/local in body, not another tail expression temporary. + // Also, closures always drops their upvars last, so we don't have to lint about them. + let base_local = move_data.base_local(idx); + if base_local == linted_move_path.place.local || base_local == Local::ZERO { + debug!(?base_local, "skip"); + return None; + } + if is_closure_like && matches!(base_local, ty::CAPTURE_STRUCT_LOCAL) { + debug!(?base_local, "skip in closure"); + return None; + } + droppy_paths_not_in_tail + .contains(idx) + .then_some(body.local_decls[base_local].source_info.span) + }) + .collect(); + if body_observables.is_empty() { + debug!("nothing observable from body"); + return; + } + debug!(?body_observables); + let linted_local_decl = &body.local_decls[linted_move_path.place.local]; + let decorator = TailExprDropOrderLint { spans: body_observables, ty: linted_local_decl.ty }; + tcx.emit_node_span_lint( + lint::builtin::TAIL_EXPR_DROP_ORDER, + tail_expr_hir_id, + linted_local_decl.source_info.span, + decorator, + ); +} + +#[derive(LintDiagnostic)] +#[diag(mir_transform_tail_expr_drop_order)] +struct TailExprDropOrderLint<'a> { + #[label] + pub spans: Vec, + pub ty: Ty<'a>, +} diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index e872878a751ed..88bbeabf8a4a9 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -288,6 +288,7 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option>) return_block, elaborate_drops::Unwind::To(resume_block), START_BLOCK, + None, ); elaborator.patch }; @@ -616,6 +617,7 @@ impl<'tcx> CloneShimBuilder<'tcx> { target: unwind, unwind: UnwindAction::Terminate(UnwindTerminateReason::InCleanup), replace: false, + scope: None, }, /* is_cleanup */ true, ); @@ -878,6 +880,7 @@ fn build_call_shim<'tcx>( target: BasicBlock::new(2), unwind: UnwindAction::Continue, replace: false, + scope: None, }, false, ); @@ -894,6 +897,7 @@ fn build_call_shim<'tcx>( target: BasicBlock::new(4), unwind: UnwindAction::Terminate(UnwindTerminateReason::InCleanup), replace: false, + scope: None, }, /* is_cleanup */ true, ); diff --git a/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs b/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs index 71723f040b3b6..79a773c192cc1 100644 --- a/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs +++ b/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs @@ -430,6 +430,7 @@ impl<'tcx> AsyncDestructorCtorShimBuilder<'tcx> { UnwindTerminateReason::InCleanup, ), replace: false, + scope: None, } } else { TerminatorKind::Goto { target: *top_cleanup_bb } diff --git a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs index 0dbbc338e738b..fe0a701107ecf 100644 --- a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs +++ b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs @@ -622,7 +622,7 @@ impl<'tcx> Stable<'tcx> for mir::TerminatorKind<'tcx> { mir::TerminatorKind::UnwindTerminate(_) => TerminatorKind::Abort, mir::TerminatorKind::Return => TerminatorKind::Return, mir::TerminatorKind::Unreachable => TerminatorKind::Unreachable, - mir::TerminatorKind::Drop { place, target, unwind, replace: _ } => { + mir::TerminatorKind::Drop { place, target, unwind, scope: _, replace: _ } => { TerminatorKind::Drop { place: place.stable(tables), target: target.as_usize(), diff --git a/tests/ui/drop/lint-tail-expr-drop-order-gated.rs b/tests/ui/drop/lint-tail-expr-drop-order-gated.rs index b22e72bcfad22..493ff11b6648d 100644 --- a/tests/ui/drop/lint-tail-expr-drop-order-gated.rs +++ b/tests/ui/drop/lint-tail-expr-drop-order-gated.rs @@ -1,15 +1,13 @@ -// 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. +// This test ensures that `tail_expr_drop_order` does not activate in case Edition 2024 is used +// because this is a migration lint. +// Only `cargo fix --edition 2024` shall activate this lint. -//@ revisions: neither no_feature_gate edition_less_than_2024 +//@ revisions: neither no_feature_gate edition_at_least_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 +//@ compile-flags: -Z unstable-options +//@ edition: 2024 #![deny(tail_expr_drop_order)] -#![cfg_attr(edition_less_than_2024, feature(shorter_tail_lifetimes))] struct LoudDropper; impl Drop for LoudDropper { diff --git a/tests/ui/drop/lint-tail-expr-drop-order.rs b/tests/ui/drop/lint-tail-expr-drop-order.rs index 0aa0ef026101a..2478b5c1f099c 100644 --- a/tests/ui/drop/lint-tail-expr-drop-order.rs +++ b/tests/ui/drop/lint-tail-expr-drop-order.rs @@ -1,6 +1,3 @@ -//@ 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 @@ -27,14 +24,31 @@ 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 + //~^ ERROR: this value of type `LoudDropper` has 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 { +fn should_not_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 + move || { + // Should not lint + x.get() + LoudDropper.get() + } +} + +fn should_lint_in_nested_items() { + fn should_lint_me() -> i32 { + let x = LoudDropper; + // Should lint + x.get() + LoudDropper.get() + //~^ ERROR: this value of type `LoudDropper` has 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_params(x: LoudDropper) -> i32 { + x.get() + LoudDropper.get() + //~^ ERROR: this value of type `LoudDropper` has significant drop implementation that will have a different drop order from that of Edition 2021 //~| WARN: this changes meaning in Rust 2024 } @@ -44,10 +58,11 @@ fn should_not_lint() -> i32 { x.get() } -fn should_not_lint_in_nested_block() -> i32 { +fn should_lint_in_nested_block() -> i32 { let x = LoudDropper; - // Should not lint because Edition 2021 drops temporaries in blocks earlier already { LoudDropper.get() } + //~^ ERROR: this value of type `LoudDropper` has 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_in_match_arm() -> i32 { @@ -58,14 +73,28 @@ fn should_not_lint_in_match_arm() -> i32 { } } -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 should_not_lint_when_consumed() -> (LoudDropper, i32) { + let x = LoudDropper; + // Should not lint + (LoudDropper, x.get()) +} + +struct MyAdt { + a: LoudDropper, + b: LoudDropper, +} + +fn should_not_lint_when_consumed_in_ctor() -> MyAdt { + let a = LoudDropper; + // Should not lint + MyAdt { a, b: LoudDropper } +} + +fn should_not_lint_when_moved() -> i32 { + let x = LoudDropper; + drop(x); + // Should not lint + LoudDropper.get() } fn main() {} diff --git a/tests/ui/drop/lint-tail-expr-drop-order.stderr b/tests/ui/drop/lint-tail-expr-drop-order.stderr index 630f0a80f092b..01286e372d147 100644 --- a/tests/ui/drop/lint-tail-expr-drop-order.stderr +++ b/tests/ui/drop/lint-tail-expr-drop-order.stderr @@ -1,8 +1,8 @@ -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 +error: this value of type `LoudDropper` has significant drop implementation that will have a different drop order from that of Edition 2021 + --> $DIR/lint-tail-expr-drop-order.rs:26:15 | LL | let x = LoudDropper; - | - these values have significant drop implementation and will observe changes in drop order under Edition 2024 + | - these local bindings with significant drop implementation may observe changes in drop order under Edition 2024 LL | // Should lint LL | x.get() + LoudDropper.get() | ^^^^^^^^^^^ @@ -10,33 +10,44 @@ LL | x.get() + LoudDropper.get() = warning: this changes meaning in Rust 2024 = note: for more information, see issue #123739 note: the lint level is defined here - --> $DIR/lint-tail-expr-drop-order.rs:8:9 + --> $DIR/lint-tail-expr-drop-order.rs:5: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 +error: this value of type `LoudDropper` has significant drop implementation that will have a different drop order from that of Edition 2021 + --> $DIR/lint-tail-expr-drop-order.rs:43:19 | -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() - | ^^^^^^^^^^^ +LL | let x = LoudDropper; + | - these local bindings with significant drop implementation may 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 -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 +error: this value of type `LoudDropper` has significant drop implementation that will have a different drop order from that of Edition 2021 + --> $DIR/lint-tail-expr-drop-order.rs:50: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() - | ^^^^^^^^^^^ +LL | fn should_lint_params(x: LoudDropper) -> i32 { + | - these local bindings with significant drop implementation may observe changes in drop order under Edition 2024 +LL | x.get() + LoudDropper.get() + | ^^^^^^^^^^^ + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see issue #123739 + +error: this value of type `LoudDropper` has significant drop implementation that will have a different drop order from that of Edition 2021 + --> $DIR/lint-tail-expr-drop-order.rs:63:7 + | +LL | let x = LoudDropper; + | - these local bindings with significant drop implementation may observe changes in drop order under Edition 2024 +LL | { LoudDropper.get() } + | ^^^^^^^^^^^ | = warning: this changes meaning in Rust 2024 = note: for more information, see issue #123739 -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors