Skip to content

Commit

Permalink
Make logging for drop-tracking easier to read.
Browse files Browse the repository at this point in the history
Some of these are a little questionable because the output is so much longer, but I would really love to keep the bit that adds the pretty-printed expression to the generated CFG .dot file.

Before:
```
DEBUG rustc_typeck::check::generator_interior::drop_ranges::record_consumed_borrow consume PlaceWithHirId { hir_id: HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 15 }, place: Place { base_ty: impl std::future::Future<Output = ()>, base: Rvalue, projections: [] } }; diag_expr_id=HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 15 }, using parent expr HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 49 }
```

After:
```
DEBUG rustc_typeck::check::generator_interior::drop_ranges::record_consumed_borrow consume PlaceWithHirId { hir_id: HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 15 }, place: Place { base_ty: impl std::future::Future<Output = ()>, base: Rvalue, projections: [] } }; diag_expr_id=expr from_config(Config {
        nickname: None,
        ..Default::default()
    }) (hir_id=HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 15 }), using parent expr
    .await (hir_id=HirId { owner: DefId(0:7 ~ default_struct_update[79f9]::foo), local_id: 49 })
```
  • Loading branch information
jyn514 committed Jul 1, 2022
1 parent 483ee1f commit 3164c2a
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 12 deletions.
19 changes: 17 additions & 2 deletions compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ rustc_index::newtype_index! {
}

/// Identifies a value whose drop state we need to track.
#[derive(PartialEq, Eq, Hash, Debug, Clone, Copy)]
#[derive(PartialEq, Eq, Hash, Clone, Copy)]
enum TrackedValue {
/// Represents a named variable, such as a let binding, parameter, or upvar.
///
Expand All @@ -121,6 +121,21 @@ enum TrackedValue {
Temporary(HirId),
}

impl Debug for TrackedValue {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
ty::tls::with_opt(|opt_tcx| {
if let Some(tcx) = opt_tcx {
write!(f, "{}", tcx.hir().node_to_string(self.hir_id()))
} else {
match self {
Self::Variable(hir_id) => write!(f, "Variable({:?})", hir_id),
Self::Temporary(hir_id) => write!(f, "Temporary({:?})", hir_id),
}
}
})
}
}

impl TrackedValue {
fn hir_id(&self) -> HirId {
match self {
Expand Down Expand Up @@ -148,7 +163,7 @@ enum TrackedValueConversionError {
/// Place projects are not currently supported.
///
/// The reasoning around these is kind of subtle, so we choose to be more
/// conservative around these for now. There is not reason in theory we
/// conservative around these for now. There is no reason in theory we
/// cannot support these, we just have not implemented it yet.
PlaceProjectionsNotSupported,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,14 @@ impl<'a, 'tcx> DropRangeVisitor<'a, 'tcx> {
/// ExprUseVisitor's consume callback doesn't go deep enough for our purposes in all
/// expressions. This method consumes a little deeper into the expression when needed.
fn consume_expr(&mut self, expr: &hir::Expr<'_>) {
debug!("consuming expr {:?}, count={:?}", expr.hir_id, self.expr_index);
debug!("consuming expr {:?}, count={:?}", expr.kind, self.expr_index);
let places = self
.places
.consumed
.get(&expr.hir_id)
.map_or(vec![], |places| places.iter().cloned().collect());
for place in places {
trace!(?place, "consuming place");
for_each_consumable(self.hir, place, |value| self.record_drop(value));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,18 @@ impl<'a> dot::Labeller<'a> for DropRangesGraph<'_, '_> {

fn node_label(&'a self, n: &Self::Node) -> dot::LabelText<'a> {
dot::LabelText::LabelStr(
self.drop_ranges
.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))
.into(),
format!(
"{n:?}: {}",
self.drop_ranges
.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))
)
.into(),
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ impl<'tcx> ExprUseDelegate<'tcx> {
if !self.places.consumed.contains_key(&consumer) {
self.places.consumed.insert(consumer, <_>::default());
}
debug!(?consumer, ?target, "mark_consumed");
self.places.consumed.get_mut(&consumer).map(|places| places.insert(target));
}

Expand Down Expand Up @@ -136,13 +137,16 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
place_with_id: &expr_use_visitor::PlaceWithHirId<'tcx>,
diag_expr_id: HirId,
) {
let parent = match self.tcx.hir().find_parent_node(place_with_id.hir_id) {
let hir = self.tcx.hir();
let parent = match hir.find_parent_node(place_with_id.hir_id) {
Some(parent) => parent,
None => place_with_id.hir_id,
};
debug!(
"consume {:?}; diag_expr_id={:?}, using parent {:?}",
place_with_id, diag_expr_id, parent
"consume {:?}; diag_expr_id={}, using parent {}",
place_with_id,
hir.node_to_string(diag_expr_id),
hir.node_to_string(parent)
);
place_with_id
.try_into()
Expand Down

0 comments on commit 3164c2a

Please sign in to comment.