From 64303189f0b3a111c3b1e59d77cdaad91ca90efe Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 17 Mar 2019 10:36:10 +0000 Subject: [PATCH] Use a valid name for graphviz graphs --- src/librustc_driver/pretty.rs | 12 +++++++++- src/librustc_mir/borrow_check/mod.rs | 8 +++---- src/librustc_mir/dataflow/graphviz.rs | 17 +++++++------- src/librustc_mir/dataflow/mod.rs | 12 +++++----- src/librustc_mir/transform/elaborate_drops.rs | 12 +++++----- src/librustc_mir/transform/generator.rs | 6 ++--- src/librustc_mir/transform/rustc_peek.rs | 19 ++++++++------- src/librustc_mir/util/graphviz.rs | 14 ++++++++++- src/librustc_mir/util/mod.rs | 2 +- src/test/mir-opt/graphviz.rs | 23 +++++++++++++++++++ 10 files changed, 85 insertions(+), 40 deletions(-) create mode 100644 src/test/mir-opt/graphviz.rs diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs index dde88a212408d..ace5198deaf2e 100644 --- a/src/librustc_driver/pretty.rs +++ b/src/librustc_driver/pretty.rs @@ -633,10 +633,20 @@ fn print_flowgraph<'a, 'tcx, W: Write>(variants: Vec, let body = tcx.hir().body(body_id); let cfg = cfg::CFG::new(tcx, &body); let labelled_edges = mode != PpFlowGraphMode::UnlabelledEdges; + let hir_id = code.id(); + // We have to disassemble the hir_id because name must be ASCII + // alphanumeric. This does not appear in the rendered graph, so it does not + // have to be user friendly. + let name = format!( + "hir_id_{}_{}_{}", + hir_id.owner.address_space().index(), + hir_id.owner.as_array_index(), + hir_id.local_id.index(), + ); let lcfg = LabelledCFG { tcx, cfg: &cfg, - name: format!("node_{}", code.id()), + name, labelled_edges, }; diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index c4e371d5afedb..551f18b95fe52 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -156,7 +156,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let mut flow_inits = FlowAtLocation::new(do_dataflow( tcx, mir, - id, + def_id, &attributes, &dead_unwinds, MaybeInitializedPlaces::new(tcx, mir, &mdpe), @@ -191,7 +191,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let flow_borrows = FlowAtLocation::new(do_dataflow( tcx, mir, - id, + def_id, &attributes, &dead_unwinds, Borrows::new(tcx, mir, regioncx.clone(), &borrow_set), @@ -200,7 +200,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let flow_uninits = FlowAtLocation::new(do_dataflow( tcx, mir, - id, + def_id, &attributes, &dead_unwinds, MaybeUninitializedPlaces::new(tcx, mir, &mdpe), @@ -209,7 +209,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let flow_ever_inits = FlowAtLocation::new(do_dataflow( tcx, mir, - id, + def_id, &attributes, &dead_unwinds, EverInitializedPlaces::new(tcx, mir, &mdpe), diff --git a/src/librustc_mir/dataflow/graphviz.rs b/src/librustc_mir/dataflow/graphviz.rs index da9cc118f5521..d68377681f1ca 100644 --- a/src/librustc_mir/dataflow/graphviz.rs +++ b/src/librustc_mir/dataflow/graphviz.rs @@ -1,6 +1,6 @@ //! Hook into libgraphviz for rendering dataflow graphs for MIR. -use rustc::hir::HirId; +use rustc::hir::def_id::DefId; use rustc::mir::{BasicBlock, Mir}; use std::fs; @@ -8,13 +8,15 @@ use std::io; use std::marker::PhantomData; use std::path::Path; +use crate::util::graphviz_safe_def_name; + use super::{BitDenotation, DataflowState}; use super::DataflowBuilder; use super::DebugFormatted; pub trait MirWithFlowState<'tcx> { type BD: BitDenotation<'tcx>; - fn hir_id(&self) -> HirId; + fn def_id(&self) -> DefId; fn mir(&self) -> &Mir<'tcx>; fn flow_state(&self) -> &DataflowState<'tcx, Self::BD>; } @@ -23,7 +25,7 @@ impl<'a, 'tcx, BD> MirWithFlowState<'tcx> for DataflowBuilder<'a, 'tcx, BD> where BD: BitDenotation<'tcx> { type BD = BD; - fn hir_id(&self) -> HirId { self.hir_id } + fn def_id(&self) -> DefId { self.def_id } fn mir(&self) -> &Mir<'tcx> { self.flow_state.mir() } fn flow_state(&self) -> &DataflowState<'tcx, Self::BD> { &self.flow_state.flow_state } } @@ -47,8 +49,8 @@ pub(crate) fn print_borrowck_graph_to<'a, 'tcx, BD, P>( let g = Graph { mbcx, phantom: PhantomData, render_idx }; let mut v = Vec::new(); dot::render(&g, &mut v)?; - debug!("print_borrowck_graph_to path: {} hir_id: {}", - path.display(), mbcx.hir_id); + debug!("print_borrowck_graph_to path: {} def_id: {:?}", + path.display(), mbcx.def_id); fs::write(path, v) } @@ -69,9 +71,8 @@ impl<'a, 'tcx, MWF, P> dot::Labeller<'a> for Graph<'a, 'tcx, MWF, P> type Node = Node; type Edge = Edge; fn graph_id(&self) -> dot::Id<'_> { - dot::Id::new(format!("graph_for_node_{}", - self.mbcx.hir_id())) - .unwrap() + let name = graphviz_safe_def_name(self.mbcx.def_id()); + dot::Id::new(format!("graph_for_def_id_{}", name)).unwrap() } fn node_id(&self, n: &Node) -> dot::Id<'_> { diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index 03f8ac6743617..d0035bf7d3bc2 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -4,7 +4,7 @@ use rustc_data_structures::bit_set::{BitSet, BitSetOperator, HybridBitSet}; use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::work_queue::WorkQueue; -use rustc::hir::HirId; +use rustc::hir::def_id::DefId; use rustc::ty::{self, TyCtxt}; use rustc::mir::{self, Mir, BasicBlock, BasicBlockData, Location, Statement, Terminator}; use rustc::mir::traversal; @@ -39,7 +39,7 @@ pub(crate) struct DataflowBuilder<'a, 'tcx: 'a, BD> where BD: BitDenotation<'tcx> { - hir_id: HirId, + def_id: DefId, flow_state: DataflowAnalysis<'a, 'tcx, BD>, print_preflow_to: Option, print_postflow_to: Option, @@ -117,7 +117,7 @@ pub struct MoveDataParamEnv<'gcx, 'tcx> { pub(crate) fn do_dataflow<'a, 'gcx, 'tcx, BD, P>(tcx: TyCtxt<'a, 'gcx, 'tcx>, mir: &'a Mir<'tcx>, - hir_id: HirId, + def_id: DefId, attributes: &[ast::Attribute], dead_unwinds: &BitSet, bd: BD, @@ -127,14 +127,14 @@ pub(crate) fn do_dataflow<'a, 'gcx, 'tcx, BD, P>(tcx: TyCtxt<'a, 'gcx, 'tcx>, P: Fn(&BD, BD::Idx) -> DebugFormatted { let flow_state = DataflowAnalysis::new(mir, dead_unwinds, bd); - flow_state.run(tcx, hir_id, attributes, p) + flow_state.run(tcx, def_id, attributes, p) } impl<'a, 'gcx: 'tcx, 'tcx: 'a, BD> DataflowAnalysis<'a, 'tcx, BD> where BD: BitDenotation<'tcx> { pub(crate) fn run

(self, tcx: TyCtxt<'a, 'gcx, 'tcx>, - hir_id: HirId, + def_id: DefId, attributes: &[ast::Attribute], p: P) -> DataflowResults<'tcx, BD> where P: Fn(&BD, BD::Idx) -> DebugFormatted @@ -159,7 +159,7 @@ impl<'a, 'gcx: 'tcx, 'tcx: 'a, BD> DataflowAnalysis<'a, 'tcx, BD> where BD: BitD name_found(tcx.sess, attributes, "borrowck_graphviz_postflow"); let mut mbcx = DataflowBuilder { - hir_id, + def_id, print_preflow_to, print_postflow_to, flow_state: self, }; diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index 32c027d90a0ce..3b0db48a94f18 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -28,7 +28,7 @@ impl MirPass for ElaborateDrops { { debug!("elaborate_drops({:?} @ {:?})", src, mir.span); - let id = tcx.hir().as_local_hir_id(src.def_id()).unwrap(); + let def_id = src.def_id(); let param_env = tcx.param_env(src.def_id()).with_reveal_all(); let move_data = match MoveData::gather_moves(mir, tcx) { Ok(move_data) => move_data, @@ -50,13 +50,13 @@ impl MirPass for ElaborateDrops { move_data, param_env, }; - let dead_unwinds = find_dead_unwinds(tcx, mir, id, &env); + let dead_unwinds = find_dead_unwinds(tcx, mir, def_id, &env); let flow_inits = - do_dataflow(tcx, mir, id, &[], &dead_unwinds, + do_dataflow(tcx, mir, def_id, &[], &dead_unwinds, MaybeInitializedPlaces::new(tcx, mir, &env), |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p])); let flow_uninits = - do_dataflow(tcx, mir, id, &[], &dead_unwinds, + do_dataflow(tcx, mir, def_id, &[], &dead_unwinds, MaybeUninitializedPlaces::new(tcx, mir, &env), |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p])); @@ -80,7 +80,7 @@ impl MirPass for ElaborateDrops { fn find_dead_unwinds<'a, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, mir: &Mir<'tcx>, - id: hir::HirId, + def_id: hir::def_id::DefId, env: &MoveDataParamEnv<'tcx, 'tcx>) -> BitSet { @@ -89,7 +89,7 @@ fn find_dead_unwinds<'a, 'tcx>( // reach cleanup blocks, which can't have unwind edges themselves. let mut dead_unwinds = BitSet::new_empty(mir.basic_blocks().len()); let flow_inits = - do_dataflow(tcx, mir, id, &[], &dead_unwinds, + do_dataflow(tcx, mir, def_id, &[], &dead_unwinds, MaybeInitializedPlaces::new(tcx, mir, &env), |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p])); for (bb, bb_data) in mir.basic_blocks().iter_enumerated() { diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 1f59802f8c6c0..98dcc3a16f209 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -383,13 +383,13 @@ fn locals_live_across_suspend_points( FxHashMap, ) { let dead_unwinds = BitSet::new_empty(mir.basic_blocks().len()); - let hir_id = tcx.hir().as_local_hir_id(source.def_id()).unwrap(); + let def_id = source.def_id(); // Calculate when MIR locals have live storage. This gives us an upper bound of their // lifetimes. let storage_live_analysis = MaybeStorageLive::new(mir); let storage_live = - do_dataflow(tcx, mir, hir_id, &[], &dead_unwinds, storage_live_analysis, + do_dataflow(tcx, mir, def_id, &[], &dead_unwinds, storage_live_analysis, |bd, p| DebugFormatted::new(&bd.mir().local_decls[p])); // Find the MIR locals which do not use StorageLive/StorageDead statements. @@ -403,7 +403,7 @@ fn locals_live_across_suspend_points( let borrowed_locals = if !movable { let analysis = HaveBeenBorrowedLocals::new(mir); let result = - do_dataflow(tcx, mir, hir_id, &[], &dead_unwinds, analysis, + do_dataflow(tcx, mir, def_id, &[], &dead_unwinds, analysis, |bd, p| DebugFormatted::new(&bd.mir().local_decls[p])); Some((analysis, result)) } else { diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index f9f8abbe6c065..246f876235d71 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -3,7 +3,7 @@ use syntax::ast; use syntax_pos::Span; use rustc::ty::{self, TyCtxt}; -use rustc::hir; +use rustc::hir::def_id::DefId; use rustc::mir::{self, Mir, Location}; use rustc_data_structures::bit_set::BitSet; use crate::transform::{MirPass, MirSource}; @@ -27,7 +27,6 @@ impl MirPass for SanityCheck { fn run_pass<'a, 'tcx>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource<'tcx>, mir: &mut Mir<'tcx>) { let def_id = src.def_id(); - let id = tcx.hir().as_local_hir_id(def_id).unwrap(); if !tcx.has_attr(def_id, "rustc_mir") { debug!("skipping rustc_peek::SanityCheck on {}", tcx.def_path_str(def_id)); return; @@ -41,26 +40,26 @@ impl MirPass for SanityCheck { let mdpe = MoveDataParamEnv { move_data: move_data, param_env: param_env }; let dead_unwinds = BitSet::new_empty(mir.basic_blocks().len()); let flow_inits = - do_dataflow(tcx, mir, id, &attributes, &dead_unwinds, + do_dataflow(tcx, mir, def_id, &attributes, &dead_unwinds, MaybeInitializedPlaces::new(tcx, mir, &mdpe), |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i])); let flow_uninits = - do_dataflow(tcx, mir, id, &attributes, &dead_unwinds, + do_dataflow(tcx, mir, def_id, &attributes, &dead_unwinds, MaybeUninitializedPlaces::new(tcx, mir, &mdpe), |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i])); let flow_def_inits = - do_dataflow(tcx, mir, id, &attributes, &dead_unwinds, + do_dataflow(tcx, mir, def_id, &attributes, &dead_unwinds, DefinitelyInitializedPlaces::new(tcx, mir, &mdpe), |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i])); if has_rustc_mir_with(&attributes, "rustc_peek_maybe_init").is_some() { - sanity_check_via_rustc_peek(tcx, mir, id, &attributes, &flow_inits); + sanity_check_via_rustc_peek(tcx, mir, def_id, &attributes, &flow_inits); } if has_rustc_mir_with(&attributes, "rustc_peek_maybe_uninit").is_some() { - sanity_check_via_rustc_peek(tcx, mir, id, &attributes, &flow_uninits); + sanity_check_via_rustc_peek(tcx, mir, def_id, &attributes, &flow_uninits); } if has_rustc_mir_with(&attributes, "rustc_peek_definite_init").is_some() { - sanity_check_via_rustc_peek(tcx, mir, id, &attributes, &flow_def_inits); + sanity_check_via_rustc_peek(tcx, mir, def_id, &attributes, &flow_def_inits); } if has_rustc_mir_with(&attributes, "stop_after_dataflow").is_some() { tcx.sess.fatal("stop_after_dataflow ended compilation"); @@ -86,12 +85,12 @@ impl MirPass for SanityCheck { /// errors are not intended to be used for unit tests.) pub fn sanity_check_via_rustc_peek<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>, mir: &Mir<'tcx>, - id: hir::HirId, + def_id: DefId, _attributes: &[ast::Attribute], results: &DataflowResults<'tcx, O>) where O: BitDenotation<'tcx, Idx=MovePathIndex> + HasMoveData<'tcx> { - debug!("sanity_check_via_rustc_peek id: {:?}", id); + debug!("sanity_check_via_rustc_peek def_id: {:?}", def_id); // FIXME: this is not DRY. Figure out way to abstract this and // `dataflow::build_sets`. (But note it is doing non-standard // stuff, so such generalization may not be realistic.) diff --git a/src/librustc_mir/util/graphviz.rs b/src/librustc_mir/util/graphviz.rs index 69a2adcfce026..f87714b58c442 100644 --- a/src/librustc_mir/util/graphviz.rs +++ b/src/librustc_mir/util/graphviz.rs @@ -1,6 +1,7 @@ use rustc::hir::def_id::DefId; use rustc::mir::*; use rustc::ty::TyCtxt; +use rustc_data_structures::indexed_vec::Idx; use std::fmt::Debug; use std::io::{self, Write}; @@ -20,6 +21,17 @@ pub fn write_mir_graphviz<'tcx, W>(tcx: TyCtxt<'_, '_, 'tcx>, Ok(()) } +// Must match `[0-9A-Za-z_]*`. This does not appear in the rendered graph, so +// it does not have to be user friendly. +pub fn graphviz_safe_def_name(def_id: DefId) -> String { + format!( + "{}_{}_{}", + def_id.krate.index(), + def_id.index.address_space().index(), + def_id.index.as_array_index(), + ) +} + /// Write a graphviz DOT graph of the MIR. pub fn write_mir_fn_graphviz<'tcx, W>(tcx: TyCtxt<'_, '_, 'tcx>, def_id: DefId, @@ -27,7 +39,7 @@ pub fn write_mir_fn_graphviz<'tcx, W>(tcx: TyCtxt<'_, '_, 'tcx>, w: &mut W) -> io::Result<()> where W: Write { - writeln!(w, "digraph Mir_{} {{", tcx.hir().as_local_hir_id(def_id).unwrap())?; + writeln!(w, "digraph Mir_{} {{", graphviz_safe_def_name(def_id))?; // Global graph properties writeln!(w, r#" graph [fontname="monospace"];"#)?; diff --git a/src/librustc_mir/util/mod.rs b/src/librustc_mir/util/mod.rs index 29614a33f8e27..1a5a2a92247dd 100644 --- a/src/librustc_mir/util/mod.rs +++ b/src/librustc_mir/util/mod.rs @@ -15,7 +15,7 @@ pub mod collect_writes; pub use self::alignment::is_disaligned; pub use self::pretty::{dump_enabled, dump_mir, write_mir_pretty, PassWhere}; -pub use self::graphviz::{write_mir_graphviz}; +pub use self::graphviz::{graphviz_safe_def_name, write_mir_graphviz}; pub use self::graphviz::write_node_label as write_graphviz_node_label; /// If possible, suggest replacing `ref` with `ref mut`. diff --git a/src/test/mir-opt/graphviz.rs b/src/test/mir-opt/graphviz.rs new file mode 100644 index 0000000000000..660576996e5d4 --- /dev/null +++ b/src/test/mir-opt/graphviz.rs @@ -0,0 +1,23 @@ +// Test graphviz output +// compile-flags: -Z dump-mir-graphviz + +// ignore-tidy-linelength + +fn main() {} + +// END RUST SOURCE +// START rustc.main.mir_map.0.dot +// digraph Mir_0_0_3 { // The name here MUST be an ASCII identifier. +// graph [fontname="monospace"]; +// node [fontname="monospace"]; +// edge [fontname="monospace"]; +// label=>; +// bb0 [shape="none", label=<
0
_0 = ()
goto
+// >]; +// bb1 [shape="none", label=<
1
resume
+// >]; +// bb2 [shape="none", label=<
2
return
+// >]; +// bb0 -> bb2 [label=""]; +// } +// END rustc.main.mir_map.0.dot