Rollup merge of rust-lang#98533 - jyn514:drop-tracking-debugging, r=eholk

Add a `-Zdump-drop-tracking-cfg` debugging flag

This is useful for debugging drop-tracking; previously, you had to recompile
rustc from source and manually add a call to `write_graph_to_file`. This
makes the option more discoverable and configurable at runtime.

I also took the liberty of making the labels for the CFG nodes much easier to read:
previously, they looked like `id(2), local_id: 48`, now they look like
```
expr from_config (hir_id=HirId { owner: DefId(0:10 ~ default_struct_update[79f9]::foo), local_id: 2})
```

r? `@eholk`

Author: matthiaskrgr

compiler/rustc_interface/src/tests.rs

@@ -649,6 +649,7 @@ fn test_debugging_options_tracking_hash() {
     untracked!(dlltool, Some(PathBuf::from("custom_dlltool.exe")));
     untracked!(dont_buffer_diagnostics, true);
     untracked!(dump_dep_graph, true);
+    untracked!(dump_drop_tracking_cfg, Some("cfg.dot".to_string()));
     untracked!(dump_mir, Some(String::from("abc")));
     untracked!(dump_mir_dataflow, true);
     untracked!(dump_mir_dir, String::from("abc"));

compiler/rustc_session/src/options.rs

@@ -1246,6 +1246,8 @@ options! {
     dump_dep_graph: bool = (false, parse_bool, [UNTRACKED],
         "dump the dependency graph to $RUST_DEP_GRAPH (default: /tmp/dep_graph.gv) \
         (default: no)"),
+    dump_drop_tracking_cfg: Option<String> = (None, parse_opt_string, [UNTRACKED],
+        "dump drop-tracking control-flow graph as a `.dot` file (default: no)"),
     dump_mir: Option<String> = (None, parse_opt_string, [UNTRACKED],
         "dump MIR state to file.
         `val` is used to select which passes and functions to dump. For example:

compiler/rustc_typeck/src/check/generator_interior/drop_ranges.rs

@@ -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.
     ///
@@ -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 {
@@ -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,
 }

compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_build.rs

@@ -33,6 +33,9 @@ pub(super) fn build_control_flow_graph<'tcx>(
     intravisit::walk_body(&mut drop_range_visitor, body);
 
     drop_range_visitor.drop_ranges.process_deferred_edges();
+    if let Some(filename) = &tcx.sess.opts.debugging_opts.dump_drop_tracking_cfg {
+        super::cfg_visualize::write_graph_to_file(&drop_range_visitor.drop_ranges, filename, tcx);
+    }
 
     (drop_range_visitor.drop_ranges, drop_range_visitor.places.borrowed_temporaries)
 }
@@ -126,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));
         }
     }

compiler/rustc_typeck/src/check/generator_interior/drop_ranges/cfg_visualize.rs

@@ -2,29 +2,43 @@
 //! flow graph when needed for debugging.
 
 use rustc_graphviz as dot;
+use rustc_middle::ty::TyCtxt;
 
 use super::{DropRangesBuilder, PostOrderId};
 
 /// Writes the CFG for DropRangesBuilder to a .dot file for visualization.
 ///
 /// It is not normally called, but is kept around to easily add debugging
 /// code when needed.
-#[allow(dead_code)]
-pub(super) fn write_graph_to_file(drop_ranges: &DropRangesBuilder, filename: &str) {
-    dot::render(drop_ranges, &mut std::fs::File::create(filename).unwrap()).unwrap();
+pub(super) fn write_graph_to_file(
+    drop_ranges: &DropRangesBuilder,
+    filename: &str,
+    tcx: TyCtxt<'_>,
+) {
+    dot::render(
+        &DropRangesGraph { drop_ranges, tcx },
+        &mut std::fs::File::create(filename).unwrap(),
+    )
+    .unwrap();
 }
 
-impl<'a> dot::GraphWalk<'a> for DropRangesBuilder {
+struct DropRangesGraph<'a, 'tcx> {
+    drop_ranges: &'a DropRangesBuilder,
+    tcx: TyCtxt<'tcx>,
+}
+
+impl<'a> dot::GraphWalk<'a> for DropRangesGraph<'_, '_> {
     type Node = PostOrderId;
 
     type Edge = (PostOrderId, PostOrderId);
 
     fn nodes(&'a self) -> dot::Nodes<'a, Self::Node> {
-        self.nodes.iter_enumerated().map(|(i, _)| i).collect()
+        self.drop_ranges.nodes.iter_enumerated().map(|(i, _)| i).collect()
     }
 
     fn edges(&'a self) -> dot::Edges<'a, Self::Edge> {
-        self.nodes
+        self.drop_ranges
+            .nodes
             .iter_enumerated()
             .flat_map(|(i, node)| {
                 if node.successors.len() == 0 {
@@ -45,7 +59,7 @@ impl<'a> dot::GraphWalk<'a> for DropRangesBuilder {
     }
 }
 
-impl<'a> dot::Labeller<'a> for DropRangesBuilder {
+impl<'a> dot::Labeller<'a> for DropRangesGraph<'_, '_> {
     type Node = PostOrderId;
 
     type Edge = (PostOrderId, PostOrderId);
@@ -61,15 +75,15 @@ impl<'a> dot::Labeller<'a> for DropRangesBuilder {
     fn node_label(&'a self, n: &Self::Node) -> dot::LabelText<'a> {
         dot::LabelText::LabelStr(
             format!(
-                "{:?}, local_id: {}",
-                n,
-                self.post_order_map
+                "{n:?}: {}",
+                self.drop_ranges
+                    .post_order_map
                     .iter()
                     .find(|(_hir_id, &post_order_id)| post_order_id == *n)
-                    .map_or("<unknown>".into(), |(hir_id, _)| format!(
-                        "{}",
-                        hir_id.local_id.index()
-                    ))
+                    .map_or("<unknown>".into(), |(hir_id, _)| self
+                        .tcx
+                        .hir()
+                        .node_to_string(*hir_id))
             )
             .into(),
         )

compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs

@@ -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));
     }
 
@@ -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()