warn on shortening super let binding lifetimes
This commit is contained in:
parent
9f7ad5ee77
commit
164620feda
8 changed files with 84 additions and 34 deletions
|
|
@ -24,7 +24,7 @@ use tracing::debug;
|
|||
#[derive(Debug, Copy, Clone)]
|
||||
struct Context {
|
||||
/// The scope that contains any new variables declared.
|
||||
var_parent: Option<Scope>,
|
||||
var_parent: (Option<Scope>, ScopeCompatibility),
|
||||
|
||||
/// Region parent of expressions, etc.
|
||||
parent: Option<Scope>,
|
||||
|
|
@ -56,7 +56,8 @@ struct ExtendedTemporaryScope {
|
|||
|
||||
/// Records the lifetime of a local variable as `cx.var_parent`
|
||||
fn record_var_lifetime(visitor: &mut ScopeResolutionVisitor<'_>, var_id: hir::ItemLocalId) {
|
||||
match visitor.cx.var_parent {
|
||||
let (var_parent_scope, var_parent_compat) = visitor.cx.var_parent;
|
||||
match var_parent_scope {
|
||||
None => {
|
||||
// this can happen in extern fn declarations like
|
||||
//
|
||||
|
|
@ -64,6 +65,9 @@ fn record_var_lifetime(visitor: &mut ScopeResolutionVisitor<'_>, var_id: hir::It
|
|||
}
|
||||
Some(parent_scope) => visitor.scope_tree.record_var_scope(var_id, parent_scope),
|
||||
}
|
||||
if let ScopeCompatibility::FutureIncompatible { shortens_to } = var_parent_compat {
|
||||
visitor.scope_tree.record_future_incompatible_var_scope(var_id, shortens_to);
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_block<'tcx>(
|
||||
|
|
@ -101,7 +105,7 @@ fn resolve_block<'tcx>(
|
|||
// itself has returned.
|
||||
|
||||
visitor.enter_node_scope_with_dtor(blk.hir_id.local_id, terminating);
|
||||
visitor.cx.var_parent = visitor.cx.parent;
|
||||
visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible);
|
||||
|
||||
{
|
||||
// This block should be kept approximately in sync with
|
||||
|
|
@ -120,7 +124,8 @@ fn resolve_block<'tcx>(
|
|||
local_id: blk.hir_id.local_id,
|
||||
data: ScopeData::Remainder(FirstStatementIndex::new(i)),
|
||||
});
|
||||
visitor.cx.var_parent = visitor.cx.parent;
|
||||
visitor.cx.var_parent =
|
||||
(visitor.cx.parent, ScopeCompatibility::FutureCompatible);
|
||||
visitor.visit_stmt(statement);
|
||||
// We need to back out temporarily to the last enclosing scope
|
||||
// for the `else` block, so that even the temporaries receiving
|
||||
|
|
@ -144,7 +149,8 @@ fn resolve_block<'tcx>(
|
|||
local_id: blk.hir_id.local_id,
|
||||
data: ScopeData::Remainder(FirstStatementIndex::new(i)),
|
||||
});
|
||||
visitor.cx.var_parent = visitor.cx.parent;
|
||||
visitor.cx.var_parent =
|
||||
(visitor.cx.parent, ScopeCompatibility::FutureCompatible);
|
||||
visitor.visit_stmt(statement)
|
||||
}
|
||||
hir::StmtKind::Item(..) => {
|
||||
|
|
@ -208,7 +214,7 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir:
|
|||
let prev_cx = visitor.cx;
|
||||
|
||||
visitor.enter_node_scope_with_dtor(arm.hir_id.local_id, true);
|
||||
visitor.cx.var_parent = visitor.cx.parent;
|
||||
visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible);
|
||||
|
||||
resolve_pat(visitor, arm.pat);
|
||||
if let Some(guard) = arm.guard {
|
||||
|
|
@ -216,7 +222,7 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir:
|
|||
// ensure they're dropped before the arm's pattern's bindings. This extends to the end of
|
||||
// the arm body and is the scope of its locals as well.
|
||||
visitor.enter_scope(Scope { local_id: arm.hir_id.local_id, data: ScopeData::MatchGuard });
|
||||
visitor.cx.var_parent = visitor.cx.parent;
|
||||
visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible);
|
||||
resolve_cond(visitor, guard);
|
||||
}
|
||||
resolve_expr(visitor, arm.body, false);
|
||||
|
|
@ -373,7 +379,7 @@ fn resolve_expr<'tcx>(
|
|||
ScopeData::IfThen
|
||||
};
|
||||
visitor.enter_scope(Scope { local_id: then.hir_id.local_id, data });
|
||||
visitor.cx.var_parent = visitor.cx.parent;
|
||||
visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible);
|
||||
resolve_cond(visitor, cond);
|
||||
resolve_expr(visitor, then, true);
|
||||
visitor.cx = expr_cx;
|
||||
|
|
@ -388,7 +394,7 @@ fn resolve_expr<'tcx>(
|
|||
ScopeData::IfThen
|
||||
};
|
||||
visitor.enter_scope(Scope { local_id: then.hir_id.local_id, data });
|
||||
visitor.cx.var_parent = visitor.cx.parent;
|
||||
visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible);
|
||||
resolve_cond(visitor, cond);
|
||||
resolve_expr(visitor, then, true);
|
||||
visitor.cx = expr_cx;
|
||||
|
|
@ -498,7 +504,7 @@ fn resolve_local<'tcx>(
|
|||
//
|
||||
// Processing of `let a` will have already decided to extend the lifetime of this
|
||||
// `super let` to its own var_scope. We use that scope.
|
||||
visitor.cx.var_parent = scope.scope;
|
||||
visitor.cx.var_parent = (scope.scope, scope.compat);
|
||||
// Inherit compatibility from the original `let` statement. If the original `let`
|
||||
// was regular, lifetime extension should apply as normal. If the original `let` was
|
||||
// `super`, blocks within the initializer will be affected by #145838.
|
||||
|
|
@ -512,9 +518,11 @@ fn resolve_local<'tcx>(
|
|||
//
|
||||
// Iterate up to the enclosing destruction scope to find the same scope that will also
|
||||
// be used for the result of the block itself.
|
||||
if let Some(inner_scope) = visitor.cx.var_parent {
|
||||
(visitor.cx.var_parent, _) =
|
||||
visitor.scope_tree.default_temporary_scope(inner_scope)
|
||||
if let (Some(inner_scope), _) = visitor.cx.var_parent {
|
||||
// NB(@dianne): This could use the incompatibility reported by
|
||||
// `default_temporary_scope` to make `tail_expr_drop_order` more comprehensive.
|
||||
visitor.cx.var_parent =
|
||||
(visitor.scope_tree.default_temporary_scope(inner_scope).0, ScopeCompatibility::FutureCompatible);
|
||||
}
|
||||
// Blocks within the initializer will be affected by #145838.
|
||||
(LetKind::Super, ScopeCompatibility::FutureCompatible)
|
||||
|
|
@ -524,7 +532,7 @@ fn resolve_local<'tcx>(
|
|||
|
||||
if let Some(expr) = init {
|
||||
let scope = ExtendedTemporaryScope {
|
||||
scope: visitor.cx.var_parent,
|
||||
scope: visitor.cx.var_parent.0,
|
||||
let_kind: source_let_kind,
|
||||
compat,
|
||||
};
|
||||
|
|
@ -536,8 +544,8 @@ fn resolve_local<'tcx>(
|
|||
expr.hir_id,
|
||||
RvalueCandidate {
|
||||
target: expr.hir_id.local_id,
|
||||
lifetime: visitor.cx.var_parent,
|
||||
compat: ScopeCompatibility::FutureCompatible,
|
||||
lifetime: visitor.cx.var_parent.0,
|
||||
compat: visitor.cx.var_parent.1,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
|
@ -818,7 +826,7 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
|
|||
self.enter_body(body.value.hir_id, |this| {
|
||||
if this.tcx.hir_body_owner_kind(owner_id).is_fn_or_closure() {
|
||||
// The arguments and `self` are parented to the fn.
|
||||
this.cx.var_parent = this.cx.parent;
|
||||
this.cx.var_parent = (this.cx.parent, ScopeCompatibility::FutureCompatible);
|
||||
for param in body.params {
|
||||
this.visit_pat(param.pat);
|
||||
}
|
||||
|
|
@ -844,7 +852,7 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
|
|||
// would *not* let the `f()` temporary escape into an outer scope
|
||||
// (i.e., `'static`), which means that after `g` returns, it drops,
|
||||
// and all the associated destruction scope rules apply.
|
||||
this.cx.var_parent = None;
|
||||
this.cx.var_parent = (None, ScopeCompatibility::FutureCompatible);
|
||||
this.enter_scope(Scope {
|
||||
local_id: body.value.hir_id.local_id,
|
||||
data: ScopeData::Destruction,
|
||||
|
|
@ -896,7 +904,7 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree {
|
|||
let mut visitor = ScopeResolutionVisitor {
|
||||
tcx,
|
||||
scope_tree: ScopeTree::default(),
|
||||
cx: Context { parent: None, var_parent: None },
|
||||
cx: Context { parent: None, var_parent: (None, ScopeCompatibility::FutureCompatible) },
|
||||
extended_super_lets: Default::default(),
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -441,8 +441,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
|||
// Check scope of binding.
|
||||
fn is_sub_scope(&self, sub_id: hir::ItemLocalId, super_id: hir::ItemLocalId) -> bool {
|
||||
let scope_tree = self.fcx.tcx.region_scope_tree(self.fcx.body_id);
|
||||
if let Some(sub_var_scope) = scope_tree.var_scope(sub_id)
|
||||
&& let Some(super_var_scope) = scope_tree.var_scope(super_id)
|
||||
if let (Some(sub_var_scope), _) = scope_tree.var_scope(sub_id)
|
||||
&& let (Some(super_var_scope), _) = scope_tree.var_scope(super_id)
|
||||
&& scope_tree.is_subscope_of(sub_var_scope, super_var_scope)
|
||||
{
|
||||
return true;
|
||||
|
|
|
|||
|
|
@ -222,6 +222,10 @@ pub struct ScopeTree {
|
|||
/// variable is declared.
|
||||
var_map: FxIndexMap<hir::ItemLocalId, Scope>,
|
||||
|
||||
/// Maps from bindings to their future scopes after #145838 for the
|
||||
/// `macro_extended_temporary_scopes` lint.
|
||||
var_compatibility_map: FxIndexMap<hir::ItemLocalId, Scope>,
|
||||
|
||||
/// Identifies expressions which, if captured into a temporary, ought to
|
||||
/// have a temporary whose lifetime extends to the end of the enclosing *block*,
|
||||
/// and not the enclosing *statement*. Expressions that are not present in this
|
||||
|
|
@ -274,6 +278,11 @@ impl ScopeTree {
|
|||
self.var_map.insert(var, lifetime);
|
||||
}
|
||||
|
||||
pub fn record_future_incompatible_var_scope(&mut self, var: hir::ItemLocalId, lifetime: Scope) {
|
||||
assert!(var != lifetime.local_id);
|
||||
self.var_compatibility_map.insert(var, lifetime);
|
||||
}
|
||||
|
||||
pub fn record_rvalue_candidate(&mut self, var: HirId, candidate: RvalueCandidate) {
|
||||
debug!("record_rvalue_candidate(var={var:?}, candidate={candidate:?})");
|
||||
if let Some(lifetime) = &candidate.lifetime {
|
||||
|
|
@ -287,9 +296,14 @@ impl ScopeTree {
|
|||
self.parent_map.get(&id).cloned()
|
||||
}
|
||||
|
||||
/// Returns the lifetime of the local variable `var_id`, if any.
|
||||
pub fn var_scope(&self, var_id: hir::ItemLocalId) -> Option<Scope> {
|
||||
self.var_map.get(&var_id).cloned()
|
||||
/// Returns the lifetime of the local variable `var_id`, if any, as well as whether it is
|
||||
/// shortening after #145838.
|
||||
pub fn var_scope(&self, var_id: hir::ItemLocalId) -> (Option<Scope>, ScopeCompatibility) {
|
||||
let compat = match self.var_compatibility_map.get(&var_id) {
|
||||
Some(&shortens_to) => ScopeCompatibility::FutureIncompatible { shortens_to },
|
||||
None => ScopeCompatibility::FutureCompatible,
|
||||
};
|
||||
(self.var_map.get(&var_id).cloned(), compat)
|
||||
}
|
||||
|
||||
/// Returns `true` if `subscope` is equal to or is lexically nested inside `superscope`, and
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ use rustc_data_structures::fx::FxIndexMap;
|
|||
use rustc_data_structures::stack::ensure_sufficient_stack;
|
||||
use rustc_hir::{BindingMode, ByRef, LetStmt, LocalSource, Node};
|
||||
use rustc_middle::bug;
|
||||
use rustc_middle::middle::region;
|
||||
use rustc_middle::middle::region::{self, ScopeCompatibility};
|
||||
use rustc_middle::mir::*;
|
||||
use rustc_middle::thir::{self, *};
|
||||
use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty, ValTree, ValTreeKind};
|
||||
|
|
@ -807,10 +807,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
|||
self.cfg.push(block, Statement::new(source_info, StatementKind::StorageLive(local_id)));
|
||||
// Although there is almost always scope for given variable in corner cases
|
||||
// like #92893 we might get variable with no scope.
|
||||
if let Some(region_scope) = self.region_scope_tree.var_scope(var.0.local_id)
|
||||
&& matches!(schedule_drop, ScheduleDrops::Yes)
|
||||
{
|
||||
self.schedule_drop(span, region_scope, local_id, DropKind::Storage);
|
||||
if matches!(schedule_drop, ScheduleDrops::Yes) {
|
||||
let (var_scope, var_scope_compat) = self.region_scope_tree.var_scope(var.0.local_id);
|
||||
if let Some(region_scope) = var_scope {
|
||||
self.schedule_drop(span, region_scope, local_id, DropKind::Storage);
|
||||
}
|
||||
if let ScopeCompatibility::FutureIncompatible { shortens_to } = var_scope_compat {
|
||||
self.schedule_backwards_incompatible_drop(
|
||||
span,
|
||||
shortens_to,
|
||||
local_id,
|
||||
BackwardIncompatibleDropReason::MacroExtendedScope,
|
||||
);
|
||||
}
|
||||
}
|
||||
Place::from(local_id)
|
||||
}
|
||||
|
|
@ -822,7 +831,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
|||
for_guard: ForGuard,
|
||||
) {
|
||||
let local_id = self.var_local_id(var, for_guard);
|
||||
if let Some(region_scope) = self.region_scope_tree.var_scope(var.0.local_id) {
|
||||
// We can ignore the var scope's future-compatibility information since we've already taken
|
||||
// it into account when scheduling the storage drop in `storage_live_binding`.
|
||||
if let (Some(region_scope), _) = self.region_scope_tree.var_scope(var.0.local_id) {
|
||||
self.schedule_drop(span, region_scope, local_id, DropKind::Value);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -64,7 +64,7 @@ pub(super) fn check<'tcx>(
|
|||
if let Some(indexed_extent) = indexed_extent {
|
||||
let parent_def_id = cx.tcx.hir_get_parent_item(expr.hir_id);
|
||||
let region_scope_tree = cx.tcx.region_scope_tree(parent_def_id);
|
||||
let pat_extent = region_scope_tree.var_scope(pat.hir_id.local_id).unwrap();
|
||||
let pat_extent = region_scope_tree.var_scope(pat.hir_id.local_id).0.unwrap();
|
||||
if region_scope_tree.is_subscope_of(indexed_extent, pat_extent) {
|
||||
return;
|
||||
}
|
||||
|
|
@ -298,6 +298,7 @@ impl<'tcx> VarVisitor<'_, 'tcx> {
|
|||
.tcx
|
||||
.region_scope_tree(parent_def_id)
|
||||
.var_scope(hir_id.local_id)
|
||||
.0
|
||||
.unwrap();
|
||||
if index_used_directly {
|
||||
self.indexed_directly.insert(
|
||||
|
|
|
|||
|
|
@ -180,8 +180,8 @@ impl<'tcx> LateLintPass<'tcx> for Shadow {
|
|||
|
||||
fn is_shadow(cx: &LateContext<'_>, owner: LocalDefId, first: ItemLocalId, second: ItemLocalId) -> bool {
|
||||
let scope_tree = cx.tcx.region_scope_tree(owner.to_def_id());
|
||||
if let Some(first_scope) = scope_tree.var_scope(first)
|
||||
&& let Some(second_scope) = scope_tree.var_scope(second)
|
||||
if let Some(first_scope) = scope_tree.var_scope(first).0
|
||||
&& let Some(second_scope) = scope_tree.var_scope(second).0
|
||||
{
|
||||
return scope_tree.is_subscope_of(second_scope, first_scope);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,5 +7,6 @@ fn main() {
|
|||
// The `()` argument to the inner `format_args!` is promoted, but the lifetimes of the internal
|
||||
// `super let` temporaries in its expansion shorten, making this an error in Rust 1.92.
|
||||
println!("{:?}{}", (), { format_args!("{:?}", ()) });
|
||||
// TODO: warn
|
||||
//~^ WARN temporary lifetime will be shortened in Rust 1.92
|
||||
//~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,15 @@
|
|||
warning: temporary lifetime will be shortened in Rust 1.92
|
||||
--> $DIR/extended-super-let-bindings.rs:9:30
|
||||
|
|
||||
LL | println!("{:?}{}", (), { format_args!("{:?}", ()) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ - ...which will be dropped at the end of this block in Rust 1.92
|
||||
| |
|
||||
| this expression creates a temporary value...
|
||||
|
|
||||
= 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
|
||||
= note: `#[warn(macro_extended_temporary_scopes)]` (part of `#[warn(future_incompatible)]`) on by default
|
||||
|
||||
warning: 1 warning emitted
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue