Skip to content

Commit df8b2e4

Browse files
committed
Auto merge of #147056 - dianne:fcw-super-let-init-borrow-shortening, r=jackh726
[beta-1.91] Warn on future errors from temporary lifetimes shortening in Rust 1.92 Pursuant to [discussion on Zulip](https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports/topic/.23145838.3A.20beta-nominated/near/540530631), this implements a future-compatibility warning lint `macro_extended_temporary_scopes` for errors in Rust 1.92 caused by #145838: ``` warning: temporary lifetime shortening in Rust 1.92 --> $DIR/macro-extended-temporary-scopes.rs:54:14 | LL | &struct_temp().field | ^^^^^^^^^^^^^ this expression creates a temporary value... ... LL | } else { | - ...which will be dropped at the end of this block in Rust 1.92 | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see <https://doc.rust-lang.org/rustc/lints/listing/warn-by-default.html#macro-extended-temporary-scopes> = note: consider using a `let` binding to create a longer lived value ``` Implementation-wise, this reuses the existing temporary scoping FCW machinery introduced for the `tail_expr_drop_order` edition lint: this adds `BackwardIncompatibleDropHint` statements to the MIR at the end of the shortened scopes for affected temporaries; these are then checked in borrowck to warn if the temporary is used after the future drop hint. There are trade-offs here: on one hand, I believe this gives some assurance over ad-hoc pattern-recognition that there are no false positives[^1]. On the other hand, this fails to lint on future dangling raw pointers and it complicates the potential addition of explanatory diagnostics or suggestions[^2]. I'm hopeful that the limitation around dangling pointers won't be relevant in real code, though; the only real instance we've seen of breakage so far is future errors in formatting macro invocations, which this should be able to catch. Release logistics notes: - This PR targets the beta branch directly, since the breakage it's a FCW for is landing in the next Rust version. - #146098 undoes the breakage this is a FCW for. If that behavior is merged and stabilizes in Rust 1.92, this PR should be reverted (or shouldn't be merged) in order to avoid spurious warnings. cc `@traviscross` `@rustbot` label +T-lang [^1]: In particular, more syntactic approaches are complicated by having to avoid warning on promoted constants; they'd either be full of holes, they'd need a lot of extra logic, or they'd need to hack more MIR-to-HIR mapping into `PromoteTemps`. [^2]: It's definitely possible to add more context and a suggestion, but the ways I've thought of to do so are either too hacky or too complex to feel appropriate for a last-minute direct-to-beta lint.
2 parents d69b1bb + 164620f commit df8b2e4

File tree

26 files changed

+986
-119
lines changed

26 files changed

+986
-119
lines changed

compiler/rustc_borrowck/src/diagnostics/mod.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use rustc_middle::ty::print::Print;
2121
use rustc_middle::ty::{self, Ty, TyCtxt};
2222
use rustc_middle::{bug, span_bug};
2323
use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult, MoveOutIndex};
24+
use rustc_session::lint::builtin::MACRO_EXTENDED_TEMPORARY_SCOPES;
2425
use rustc_span::def_id::LocalDefId;
2526
use rustc_span::source_map::Spanned;
2627
use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, Symbol, sym};
@@ -1580,4 +1581,32 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
15801581
pub(crate) fn local_excluded_from_unused_mut_lint(&self, index: Local) -> bool {
15811582
self.local_name(index).is_none_or(|name| name.as_str().starts_with('_'))
15821583
}
1584+
1585+
/// Report a temporary whose scope will shorten in Rust 1.92 due to #145838.
1586+
pub(crate) fn lint_macro_extended_temporary_scope(
1587+
&self,
1588+
future_drop: Location,
1589+
borrow: &BorrowData<'tcx>,
1590+
) {
1591+
let tcx = self.infcx.tcx;
1592+
let temp_decl = &self.body.local_decls[borrow.borrowed_place.local];
1593+
let temp_span = temp_decl.source_info.span;
1594+
let lint_root = self.body.source_scopes[temp_decl.source_info.scope]
1595+
.local_data
1596+
.as_ref()
1597+
.unwrap_crate_local()
1598+
.lint_root;
1599+
1600+
let mut labels = MultiSpan::from_span(temp_span);
1601+
labels.push_span_label(temp_span, "this expression creates a temporary value...");
1602+
labels.push_span_label(
1603+
self.body.source_info(future_drop).span,
1604+
"...which will be dropped at the end of this block in Rust 1.92",
1605+
);
1606+
1607+
tcx.node_span_lint(MACRO_EXTENDED_TEMPORARY_SCOPES, lint_root, labels, |diag| {
1608+
diag.primary_message("temporary lifetime will be shortened in Rust 1.92");
1609+
diag.note("consider using a `let` binding to create a longer lived value");
1610+
});
1611+
}
15831612
}

compiler/rustc_borrowck/src/lib.rs

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -843,8 +843,8 @@ impl<'a, 'tcx> ResultsVisitor<'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<'a,
843843
| StatementKind::ConstEvalCounter
844844
| StatementKind::StorageLive(..) => {}
845845
// This does not affect borrowck
846-
StatementKind::BackwardIncompatibleDropHint { place, reason: BackwardIncompatibleDropReason::Edition2024 } => {
847-
self.check_backward_incompatible_drop(location, **place, state);
846+
StatementKind::BackwardIncompatibleDropHint { place, reason } => {
847+
self.check_backward_incompatible_drop(location, **place, state, *reason);
848848
}
849849
StatementKind::StorageDead(local) => {
850850
self.access_place(
@@ -1386,6 +1386,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
13861386
location: Location,
13871387
place: Place<'tcx>,
13881388
state: &BorrowckDomain,
1389+
reason: BackwardIncompatibleDropReason,
13891390
) {
13901391
let tcx = self.infcx.tcx;
13911392
// If this type does not need `Drop`, then treat it like a `StorageDead`.
@@ -1412,21 +1413,29 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
14121413
if matches!(borrow.kind, BorrowKind::Fake(_)) {
14131414
return ControlFlow::Continue(());
14141415
}
1415-
let borrowed = this.retrieve_borrow_spans(borrow).var_or_use_path_span();
1416-
let explain = this.explain_why_borrow_contains_point(
1417-
location,
1418-
borrow,
1419-
Some((WriteKind::StorageDeadOrDrop, place)),
1420-
);
1421-
this.infcx.tcx.node_span_lint(
1422-
TAIL_EXPR_DROP_ORDER,
1423-
CRATE_HIR_ID,
1424-
borrowed,
1425-
|diag| {
1426-
session_diagnostics::TailExprDropOrder { borrowed }.decorate_lint(diag);
1427-
explain.add_explanation_to_diagnostic(&this, diag, "", None, None);
1428-
},
1429-
);
1416+
match reason {
1417+
BackwardIncompatibleDropReason::Edition2024 => {
1418+
let borrowed = this.retrieve_borrow_spans(borrow).var_or_use_path_span();
1419+
let explain = this.explain_why_borrow_contains_point(
1420+
location,
1421+
borrow,
1422+
Some((WriteKind::StorageDeadOrDrop, place)),
1423+
);
1424+
this.infcx.tcx.node_span_lint(
1425+
TAIL_EXPR_DROP_ORDER,
1426+
CRATE_HIR_ID,
1427+
borrowed,
1428+
|diag| {
1429+
session_diagnostics::TailExprDropOrder { borrowed }
1430+
.decorate_lint(diag);
1431+
explain.add_explanation_to_diagnostic(&this, diag, "", None, None);
1432+
},
1433+
);
1434+
}
1435+
BackwardIncompatibleDropReason::MacroExtendedScope => {
1436+
this.lint_macro_extended_temporary_scope(location, borrow);
1437+
}
1438+
}
14301439
// We may stop at the first case
14311440
ControlFlow::Break(())
14321441
},

0 commit comments

Comments
 (0)