From b9d3f654128ce6ba29e1e9ff31fcc44f2f04b59f Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Tue, 30 Aug 2022 15:17:45 -0700 Subject: [PATCH 1/2] [drop tracking] Use parent expression for scope Previously we were just using the parent node as the scope for a temporary value, but it turns out this is too narrow. For example, in an expression like Foo { b: &42, a: async { 0 }.await, } the scope for the &42 was set to the ExprField node for `b: &42`, when we actually want to use the Foo struct expression. We fix this by recursively searching through parent nodes until we find a Node::Expr. It may be that we don't find one, and if so that's okay, we will just fall back on the enclosing temporary scope which is always sufficient. --- .../src/check/generator_interior.rs | 17 +++++++++++++++-- .../drop_ranges/record_consumed_borrow.rs | 4 ++-- src/test/ui/async-await/issue-73137.rs | 3 +++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_typeck/src/check/generator_interior.rs b/compiler/rustc_typeck/src/check/generator_interior.rs index 85a0d4e449906..f73d498aabc23 100644 --- a/compiler/rustc_typeck/src/check/generator_interior.rs +++ b/compiler/rustc_typeck/src/check/generator_interior.rs @@ -387,6 +387,18 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { ty.needs_drop(self.fcx.tcx, self.fcx.param_env) }; + let find_parent_expr = |mut hir_id| { + let hir = self.fcx.tcx.hir(); + hir_id = hir.find_parent_node(hir_id)?; + loop { + if let hir::Node::Expr(_) = self.fcx.tcx.hir().find(hir_id)? { + return Some(hir_id); + } else { + hir_id = hir.find_parent_node(hir_id)?; + } + } + }; + // Typically, the value produced by an expression is consumed by its parent in some way, // so we only have to check if the parent contains a yield (note that the parent may, for // example, store the value into a local variable, but then we already consider local @@ -409,8 +421,9 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { }) { self.rvalue_scopes.temporary_scope(self.region_scope_tree, expr.hir_id.local_id) } else { - debug!("parent_node: {:?}", self.fcx.tcx.hir().find_parent_node(expr.hir_id)); - match self.fcx.tcx.hir().find_parent_node(expr.hir_id) { + let parent_expr = find_parent_expr(expr.hir_id); + debug!("parent_expr: {:?}", parent_expr); + match parent_expr { Some(parent) => Some(Scope { id: parent.local_id, data: ScopeData::Node }), None => { self.rvalue_scopes.temporary_scope(self.region_scope_tree, expr.hir_id.local_id) diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs index ded0888c33e15..e22675e9d5f4e 100644 --- a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs @@ -159,8 +159,8 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> { bk: rustc_middle::ty::BorrowKind, ) { debug!( - "borrow: place_with_id = {place_with_id:?}, diag_expr_id={diag_expr_id:?}, \ - borrow_kind={bk:?}" + "borrow: place_with_id = {place_with_id:#?}, diag_expr_id={diag_expr_id:#?}, \ + borrow_kind={bk:#?}" ); self.borrow_place(place_with_id); diff --git a/src/test/ui/async-await/issue-73137.rs b/src/test/ui/async-await/issue-73137.rs index c43ce2cadba04..dcbe7765a9e1c 100644 --- a/src/test/ui/async-await/issue-73137.rs +++ b/src/test/ui/async-await/issue-73137.rs @@ -2,6 +2,9 @@ // run-pass // edition:2018 +// revisions: normal drop-tracking +// [normal]compile-flags: -Zdrop-tracking=no +// [drop-tracking]compile-flags: -Zdrop-tracking #![allow(dead_code)] use std::future::Future; From f921f5626d1dcc08f2707ed2c22e22fd1ad678b7 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Wed, 31 Aug 2022 11:10:19 -0700 Subject: [PATCH 2/2] Use parent_iter instead of a find_parent_node loop --- compiler/rustc_middle/src/hir/map/mod.rs | 3 +++ .../src/check/generator_interior.rs | 20 +++++++------------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 8f7877392483f..6217bffb8f76c 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -291,6 +291,9 @@ impl<'hir> Map<'hir> { Some(def_kind) } + /// Finds the id of the parent node to this one. + /// + /// If calling repeatedly and iterating over parents, prefer [`Map::parent_iter`]. pub fn find_parent_node(self, id: HirId) -> Option { if id.local_id == ItemLocalId::from_u32(0) { Some(self.tcx.hir_owner_parent(id.owner)) diff --git a/compiler/rustc_typeck/src/check/generator_interior.rs b/compiler/rustc_typeck/src/check/generator_interior.rs index f73d498aabc23..2710606f91436 100644 --- a/compiler/rustc_typeck/src/check/generator_interior.rs +++ b/compiler/rustc_typeck/src/check/generator_interior.rs @@ -387,18 +387,6 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { ty.needs_drop(self.fcx.tcx, self.fcx.param_env) }; - let find_parent_expr = |mut hir_id| { - let hir = self.fcx.tcx.hir(); - hir_id = hir.find_parent_node(hir_id)?; - loop { - if let hir::Node::Expr(_) = self.fcx.tcx.hir().find(hir_id)? { - return Some(hir_id); - } else { - hir_id = hir.find_parent_node(hir_id)?; - } - } - }; - // Typically, the value produced by an expression is consumed by its parent in some way, // so we only have to check if the parent contains a yield (note that the parent may, for // example, store the value into a local variable, but then we already consider local @@ -421,7 +409,13 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { }) { self.rvalue_scopes.temporary_scope(self.region_scope_tree, expr.hir_id.local_id) } else { - let parent_expr = find_parent_expr(expr.hir_id); + let parent_expr = self + .fcx + .tcx + .hir() + .parent_iter(expr.hir_id) + .find(|(_, node)| matches!(node, hir::Node::Expr(_))) + .map(|(id, _)| id); debug!("parent_expr: {:?}", parent_expr); match parent_expr { Some(parent) => Some(Scope { id: parent.local_id, data: ScopeData::Node }),