Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[drop tracking] Visit break expressions #106699

Merged
merged 2 commits into from
Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,8 @@ impl<'a, 'tcx> Visitor<'tcx> for DropRangeVisitor<'a, 'tcx> {
let mut reinit = None;
match expr.kind {
ExprKind::Assign(lhs, rhs, _) => {
self.visit_expr(lhs);
self.visit_expr(rhs);
self.visit_expr(lhs);
eholk marked this conversation as resolved.
Show resolved Hide resolved

reinit = Some(lhs);
}
Expand Down Expand Up @@ -433,7 +433,7 @@ impl<'a, 'tcx> Visitor<'tcx> for DropRangeVisitor<'a, 'tcx> {
self.drop_ranges.add_control_edge(self.expr_index, *target)
}),

ExprKind::Break(destination, ..) => {
ExprKind::Break(destination, value) => {
// destination either points to an expression or to a block. We use
// find_target_expression_from_destination to use the last expression of the block
// if destination points to a block.
Expand All @@ -443,7 +443,11 @@ impl<'a, 'tcx> Visitor<'tcx> for DropRangeVisitor<'a, 'tcx> {
// will refer to the end of the block due to the post order traversal.
self.find_target_expression_from_destination(destination).map_or((), |target| {
self.drop_ranges.add_control_edge_hir_id(self.expr_index, target)
})
});

if let Some(value) = value {
self.visit_expr(value);
}
}

ExprKind::Call(f, args) => {
Expand All @@ -465,6 +469,12 @@ impl<'a, 'tcx> Visitor<'tcx> for DropRangeVisitor<'a, 'tcx> {

ExprKind::AddrOf(..)
| ExprKind::Array(..)
// FIXME(eholk): We probably need special handling for AssignOps. The ScopeTree builder
// in region.rs runs both lhs then rhs and rhs then lhs and then sets all yields to be
// the latest they show up in either traversal. With the older scope-based
// approximation, this was fine, but it's probably not right now. What we probably want
// to do instead is still run both orders, but consider anything that showed up as a
// yield in either order.
| ExprKind::AssignOp(..)
| ExprKind::Binary(..)
| ExprKind::Block(..)
Expand Down Expand Up @@ -502,6 +512,9 @@ impl<'a, 'tcx> Visitor<'tcx> for DropRangeVisitor<'a, 'tcx> {

// Increment expr_count here to match what InteriorVisitor expects.
self.expr_index = self.expr_index + 1;

// Save a node mapping to get better CFG visualization
self.drop_ranges.add_node_mapping(pat.hir_id, self.expr_index);
}
}

Expand All @@ -521,7 +534,7 @@ impl DropRangesBuilder {
}
});
}
debug!("hir_id_map: {:?}", tracked_value_map);
debug!("hir_id_map: {:#?}", tracked_value_map);
let num_values = tracked_value_map.len();
Self {
tracked_value_map,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//! flow graph when needed for debugging.

use rustc_graphviz as dot;
use rustc_hir::{Expr, ExprKind, Node};
use rustc_middle::ty::TyCtxt;

use super::{DropRangesBuilder, PostOrderId};
Expand Down Expand Up @@ -80,10 +81,14 @@ impl<'a> dot::Labeller<'a> for DropRangesGraph<'_, '_> {
.post_order_map
.iter()
.find(|(_hir_id, &post_order_id)| post_order_id == *n)
.map_or("<unknown>".into(), |(hir_id, _)| self
.tcx
.hir()
.node_to_string(*hir_id))
.map_or("<unknown>".into(), |(hir_id, _)| format!(
"{}{}",
self.tcx.hir().node_to_string(*hir_id),
match self.tcx.hir().find(*hir_id) {
Some(Node::Expr(Expr { kind: ExprKind::Yield(..), .. })) => " (yield)",
_ => "",
}
))
)
.into(),
)
Expand Down
18 changes: 14 additions & 4 deletions compiler/rustc_hir_typeck/src/generator_interior/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,8 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
yield_data.expr_and_pat_count, self.expr_count, source_span
);

if self.fcx.sess().opts.unstable_opts.drop_tracking
&& self
.drop_ranges
.is_dropped_at(hir_id, yield_data.expr_and_pat_count)
if self
.is_dropped_at_yield_location(hir_id, yield_data.expr_and_pat_count)
{
debug!("value is dropped at yield point; not recording");
return false;
Expand Down Expand Up @@ -173,6 +171,18 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
}
}
}

/// If drop tracking is enabled, consult drop_ranges to see if a value is
/// known to be dropped at a yield point and therefore can be omitted from
/// the generator witness.
fn is_dropped_at_yield_location(&self, value_hir_id: HirId, yield_location: usize) -> bool {
// short-circuit if drop tracking is not enabled.
if !self.fcx.sess().opts.unstable_opts.drop_tracking {
return false;
}

self.drop_ranges.is_dropped_at(value_hir_id, yield_location)
}
}

pub fn resolve_interior<'a, 'tcx>(
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/async-await/await-sequence.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// edition:2021
// compile-flags: -Z drop-tracking
// build-pass

use std::collections::HashMap;

fn main() {
let _ = real_main();
}

async fn nop() {}

async fn real_main() {
nop().await;
nop().await;
nop().await;
nop().await;

let mut map: HashMap<(), ()> = HashMap::new();
map.insert((), nop().await);
}