Skip to content

Commit 2a29369

Browse files
committed
Auto merge of #149435 - sladyn98:fix-panic-drop-ordering-rebased, r=<try>
Fix variable deallocation order in panic unwinding paths
2 parents fd07dbf + d1c93d3 commit 2a29369

79 files changed

Lines changed: 1397 additions & 320 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

compiler/rustc_borrowck/src/lib.rs

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ use rustc_mir_dataflow::move_paths::{
4646
};
4747
use rustc_mir_dataflow::points::DenseLocationMap;
4848
use rustc_mir_dataflow::{Analysis, EntryStates, Results, ResultsVisitor, visit_results};
49-
use rustc_session::lint::builtin::{TAIL_EXPR_DROP_ORDER, UNUSED_MUT};
49+
use rustc_session::lint::builtin::{TAIL_EXPR_DROP_ORDER, UNUSED_MUT, UNWIND_DROP_ORDER};
5050
use rustc_span::{ErrorGuaranteed, Span, Symbol};
5151
use rustc_trait_selection::traits::query::type_op::{QueryTypeOp, TypeOp, TypeOpOutput};
5252
use smallvec::SmallVec;
@@ -853,11 +853,8 @@ impl<'a, 'tcx> ResultsVisitor<'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<'a,
853853
// These do not actually affect borrowck
854854
StatementKind::ConstEvalCounter | StatementKind::StorageLive(..) => {}
855855
// This does not affect borrowck
856-
StatementKind::BackwardIncompatibleDropHint {
857-
place,
858-
reason: BackwardIncompatibleDropReason::Edition2024,
859-
} => {
860-
self.check_backward_incompatible_drop(location, **place, state);
856+
StatementKind::BackwardIncompatibleDropHint { place, reason } => {
857+
self.check_backward_incompatible_drop(location, **place, *reason, state);
861858
}
862859
StatementKind::StorageDead(local) => {
863860
self.access_place(
@@ -1400,6 +1397,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
14001397
&mut self,
14011398
location: Location,
14021399
place: Place<'tcx>,
1400+
reason: BackwardIncompatibleDropReason,
14031401
state: &BorrowckDomain,
14041402
) {
14051403
let tcx = self.infcx.tcx;
@@ -1433,17 +1431,36 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
14331431
borrow,
14341432
Some((WriteKind::StorageDeadOrDrop, place)),
14351433
);
1436-
this.infcx.tcx.emit_node_span_lint(
1437-
TAIL_EXPR_DROP_ORDER,
1438-
CRATE_HIR_ID,
1439-
borrowed,
1440-
session_diagnostics::TailExprDropOrder {
1441-
borrowed,
1442-
callback: |diag| {
1443-
explain.add_explanation_to_diagnostic(&this, diag, "", None, None);
1444-
},
1445-
},
1446-
);
1434+
match reason {
1435+
BackwardIncompatibleDropReason::Edition2024 => {
1436+
this.infcx.tcx.emit_node_span_lint(
1437+
TAIL_EXPR_DROP_ORDER,
1438+
CRATE_HIR_ID,
1439+
borrowed,
1440+
session_diagnostics::TailExprDropOrder {
1441+
borrowed,
1442+
callback: |diag| {
1443+
explain
1444+
.add_explanation_to_diagnostic(&this, diag, "", None, None);
1445+
},
1446+
},
1447+
);
1448+
}
1449+
BackwardIncompatibleDropReason::UnwindStorageDead => {
1450+
this.infcx.tcx.emit_node_span_lint(
1451+
UNWIND_DROP_ORDER,
1452+
CRATE_HIR_ID,
1453+
borrowed,
1454+
session_diagnostics::UnwindDropOrder {
1455+
borrowed,
1456+
callback: |diag| {
1457+
explain
1458+
.add_explanation_to_diagnostic(&this, diag, "", None, None);
1459+
},
1460+
},
1461+
);
1462+
}
1463+
}
14471464
// We may stop at the first case
14481465
ControlFlow::Break(())
14491466
},

compiler/rustc_borrowck/src/polonius/legacy/loan_invalidations.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@ impl<'a, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'a, 'tcx> {
7070
// Doesn't have any language semantics
7171
| StatementKind::Coverage(..)
7272
// Does not actually affect borrowck
73-
| StatementKind::StorageLive(..) => {}
73+
| StatementKind::StorageLive(..)
74+
// Future-compatibility drop hints are lint markers, not loan invalidations.
75+
| StatementKind::BackwardIncompatibleDropHint { .. } => {}
7476
StatementKind::StorageDead(local) => {
7577
self.access_place(
7678
location,
@@ -81,7 +83,6 @@ impl<'a, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'a, 'tcx> {
8183
}
8284
StatementKind::ConstEvalCounter
8385
| StatementKind::Nop
84-
| StatementKind::BackwardIncompatibleDropHint { .. }
8586
| StatementKind::SetDiscriminant { .. } => {
8687
bug!("Statement not allowed in this MIR phase")
8788
}

compiler/rustc_borrowck/src/session_diagnostics.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,3 +633,19 @@ impl<'a, F: FnOnce(&mut Diag<'_, ()>)> Diagnostic<'a, ()> for TailExprDropOrder<
633633
diag
634634
}
635635
}
636+
637+
pub(crate) struct UnwindDropOrder<F: FnOnce(&mut Diag<'_, ()>)> {
638+
pub borrowed: Span,
639+
pub callback: F,
640+
}
641+
642+
impl<'a, F: FnOnce(&mut Diag<'_, ()>)> Diagnostic<'a, ()> for UnwindDropOrder<F> {
643+
fn into_diag(self, dcx: DiagCtxtHandle<'a>, level: Level) -> Diag<'a, ()> {
644+
let Self { borrowed, callback } = self;
645+
let mut diag =
646+
Diag::new(dcx, level, "relative drop order on unwind changing in a future release")
647+
.with_span_label(borrowed, "this value would be considered dead on an unwind path");
648+
callback(&mut diag);
649+
diag
650+
}
651+
}

compiler/rustc_lint_defs/src/builtin.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ pub mod hardwired {
150150
UNUSED_UNSAFE,
151151
UNUSED_VARIABLES,
152152
UNUSED_VISIBILITIES,
153+
UNWIND_DROP_ORDER,
153154
USELESS_DEPRECATED,
154155
VARARGS_WITHOUT_PATTERN,
155156
WARNINGS,
@@ -5233,6 +5234,47 @@ declare_lint! {
52335234
};
52345235
}
52355236

5237+
declare_lint! {
5238+
/// The `unwind_drop_order` lint detects values whose relative drop order on
5239+
/// panic unwind paths will change in a future release.
5240+
///
5241+
/// ### Example
5242+
/// ```rust,no_run
5243+
/// #![warn(unwind_drop_order)]
5244+
///
5245+
/// struct Wrap<T>(T);
5246+
///
5247+
/// impl<T> Drop for Wrap<T> {
5248+
/// fn drop(&mut self) {}
5249+
/// }
5250+
///
5251+
/// fn main() {
5252+
/// let x;
5253+
/// {
5254+
/// let y = 1;
5255+
/// x = Wrap(&y);
5256+
/// panic!();
5257+
/// }
5258+
/// }
5259+
/// ```
5260+
///
5261+
/// {{produces}}
5262+
///
5263+
/// ### Explanation
5264+
///
5265+
/// During panic unwinding, Rust currently does not mark locals as dead at
5266+
/// the same program points as normal control flow. A future release may
5267+
/// make unwind paths use the same storage-dead points as normal paths. This
5268+
/// lint reports code where that change can affect relative drop order
5269+
/// checking.
5270+
pub UNWIND_DROP_ORDER,
5271+
Allow,
5272+
"detects future changes to drop order during panic unwinding",
5273+
@future_incompatible = FutureIncompatibleInfo {
5274+
reason: fcw!(FutureReleaseSemanticsChange #147875),
5275+
};
5276+
}
5277+
52365278
declare_lint! {
52375279
/// The `rust_2024_guarded_string_incompatible_syntax` lint detects `#` tokens
52385280
/// that will be parsed as part of a guarded string literal in Rust 2024.

compiler/rustc_middle/src/mir/pretty.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -879,10 +879,8 @@ impl Debug for StatementKind<'_> {
879879
Intrinsic(ref intrinsic) => write!(fmt, "{intrinsic}"),
880880
ConstEvalCounter => write!(fmt, "ConstEvalCounter"),
881881
Nop => write!(fmt, "nop"),
882-
BackwardIncompatibleDropHint { ref place, reason: _ } => {
883-
// For now, we don't record the reason because there is only one use case,
884-
// which is to report breaking change in drop order by Edition 2024
885-
write!(fmt, "BackwardIncompatibleDropHint({place:?})")
882+
BackwardIncompatibleDropHint { ref place, reason } => {
883+
write!(fmt, "BackwardIncompatibleDropHint({place:?}, {reason:?})")
886884
}
887885
}
888886
}

compiler/rustc_middle/src/mir/syntax.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,8 +445,7 @@ pub enum StatementKind<'tcx> {
445445
/// Marker statement indicating where `place` would be dropped.
446446
/// This is semantically equivalent to `Nop`, so codegen and MIRI should interpret this
447447
/// statement as such.
448-
/// The only use case of this statement is for linting in MIR to detect temporary lifetime
449-
/// changes.
448+
/// The only use case of this statement is for MIR future-compatibility linting.
450449
BackwardIncompatibleDropHint {
451450
/// Place to drop
452451
place: Box<Place<'tcx>>,
@@ -975,6 +974,7 @@ pub enum TerminatorKind<'tcx> {
975974

976975
#[derive(
977976
Clone,
977+
Copy,
978978
Debug,
979979
TyEncodable,
980980
TyDecodable,
@@ -986,6 +986,7 @@ pub enum TerminatorKind<'tcx> {
986986
)]
987987
pub enum BackwardIncompatibleDropReason {
988988
Edition2024,
989+
UnwindStorageDead,
989990
}
990991

991992
#[derive(Debug, Clone, TyEncodable, TyDecodable, Hash, StableHash, PartialEq)]

0 commit comments

Comments
 (0)