Rollup merge of #69005 - ecstatic-morse:unified-dataflow-graphviz, r=wesleywiser

Small graphviz improvements for the new dataflow framework

Split out from #68241. This prints the correct effect diff for each line (the before-effect and the after-effect) and makes marginal improvements to the graphviz output for the new dataflow framework including using monospaced font and better line breaking. This will only be useful when #68241 is merged and the graphviz output changes from this:

![image](https://user-images.githubusercontent.com/29463364/74107776-5e3c3300-4b28-11ea-9d33-4862228b5e87.png)

to this:

![image](https://user-images.githubusercontent.com/29463364/74107751-20d7a580-4b28-11ea-99ca-24f749386601.png)

r? @wesleywiser

Author: Dylan-DPC

src/libgraphviz/lib.rs

@@ -597,6 +597,8 @@ pub enum RenderOption {
     NoNodeLabels,
     NoEdgeStyles,
     NoNodeStyles,
+
+    Monospace,
 }
 
 /// Returns vec holding all the default render options.
@@ -626,6 +628,14 @@ where
     W: Write,
 {
     writeln!(w, "digraph {} {{", g.graph_id().as_slice())?;
+
+    // Global graph properties
+    if options.contains(&RenderOption::Monospace) {
+        writeln!(w, r#"    graph[fontname="monospace"];"#)?;
+        writeln!(w, r#"    node[fontname="monospace"];"#)?;
+        writeln!(w, r#"    edge[fontname="monospace"];"#)?;
+    }
+
     for n in g.nodes().iter() {
         write!(w, "    ")?;
         let id = g.node_id(n);

src/librustc_mir/dataflow/generic/engine.rs

@@ -331,7 +331,7 @@ where
     let mut buf = Vec::new();
 
     let graphviz = graphviz::Formatter::new(body, def_id, results, &mut *formatter);
-    dot::render(&graphviz, &mut buf)?;
+    dot::render_opts(&graphviz, &mut buf, &[dot::RenderOption::Monospace])?;
     fs::write(&path, buf)?;
     Ok(())
 }

src/librustc_mir/dataflow/generic/graphviz.rs

@@ -171,10 +171,19 @@ where
         //   | | (on successful return)           | +_4        |
         //   +-+----------------------------------+------------+
 
-        write!(
-            w,
-            r#"<table border="1" cellborder="1" cellspacing="0" cellpadding="3" sides="rb">"#,
-        )?;
+        // N.B., Some attributes (`align`, `balign`) are repeated on parent elements and their
+        // children. This is because `xdot` seemed to have a hard time correctly propagating
+        // attributes. Make sure to test the output before trying to remove the redundancy.
+        // Notably, `align` was found to have no effect when applied only to <table>.
+
+        let table_fmt = concat!(
+            " border=\"1\"",
+            " cellborder=\"1\"",
+            " cellspacing=\"0\"",
+            " cellpadding=\"3\"",
+            " sides=\"rb\"",
+        );
+        write!(w, r#"<table{fmt}>"#, fmt = table_fmt)?;
 
         // A + B: Block header
         if self.state_formatter.column_names().is_empty() {
@@ -186,7 +195,7 @@ where
         // C: Entry state
         self.bg = Background::Light;
         self.results.seek_to_block_start(block);
-        self.write_row_with_full_state(w, "", "(on_entry)")?;
+        self.write_row_with_full_state(w, "", "(on entry)")?;
 
         // D: Statement transfer functions
         for (i, statement) in body[block].statements.iter().enumerate() {
@@ -212,7 +221,7 @@ where
             self.write_row(w, "", "(on successful return)", |this, w, fmt| {
                 write!(
                     w,
-                    r#"<td colspan="{colspan}" {fmt} align="left">"#,
+                    r#"<td balign="left" colspan="{colspan}" {fmt} align="left">"#,
                     colspan = num_state_columns,
                     fmt = fmt,
                 )?;
@@ -311,7 +320,9 @@ where
         f: impl FnOnce(&mut Self, &mut W, &str) -> io::Result<()>,
     ) -> io::Result<()> {
         let bg = self.toggle_background();
-        let fmt = format!("sides=\"tl\" {}", bg.attr());
+        let valign = if mir.starts_with("(on ") && mir != "(on entry)" { "bottom" } else { "top" };
+
+        let fmt = format!("valign=\"{}\" sides=\"tl\" {}", valign, bg.attr());
 
         write!(
             w,
@@ -345,7 +356,7 @@ where
                 colspan = this.num_state_columns(),
                 fmt = fmt,
             )?;
-            pretty_print_state_elems(w, analysis, state.iter(), ",", LIMIT_40_ALIGN_1)?;
+            pretty_print_state_elems(w, analysis, state.iter(), ", ", LIMIT_30_ALIGN_1)?;
             write!(w, "}}</td>")
         })
     }
@@ -387,7 +398,6 @@ pub struct SimpleDiff<T: Idx> {
 }
 
 impl<T: Idx> SimpleDiff<T> {
-    #![allow(unused)]
     pub fn new(bits_per_block: usize) -> Self {
         SimpleDiff { prev_state: BitSet::new_empty(bits_per_block), prev_loc: Location::START }
     }
@@ -417,8 +427,8 @@ where
         }
 
         self.prev_loc = location;
-        write!(w, r#"<td {fmt} align="left">"#, fmt = fmt)?;
-        results.seek_before(location);
+        write!(w, r#"<td {fmt} balign="left" align="left">"#, fmt = fmt)?;
+        results.seek_after(location);
         let curr_state = results.get();
         write_diff(&mut w, results.analysis(), &self.prev_state, curr_state)?;
         self.prev_state.overwrite(curr_state);
@@ -434,7 +444,6 @@ pub struct TwoPhaseDiff<T: Idx> {
 }
 
 impl<T: Idx> TwoPhaseDiff<T> {
-    #![allow(unused)]
     pub fn new(bits_per_block: usize) -> Self {
         TwoPhaseDiff { prev_state: BitSet::new_empty(bits_per_block), prev_loc: Location::START }
     }
@@ -445,7 +454,7 @@ where
     A: Analysis<'tcx>,
 {
     fn column_names(&self) -> &[&str] {
-        &["ENTRY", " EXIT"]
+        &["BEFORE", " AFTER"]
     }
 
     fn write_state_for_location(
@@ -465,7 +474,7 @@ where
 
         self.prev_loc = location;
 
-        // Entry
+        // Before
 
         write!(w, r#"<td {fmt} align="left">"#, fmt = fmt)?;
         results.seek_before(location);
@@ -474,7 +483,7 @@ where
         self.prev_state.overwrite(curr_state);
         write!(w, "</td>")?;
 
-        // Exit
+        // After
 
         write!(w, r#"<td {fmt} align="left">"#, fmt = fmt)?;
         results.seek_after(location);
@@ -492,7 +501,6 @@ pub struct BlockTransferFunc<'a, 'tcx, T: Idx> {
 }
 
 impl<T: Idx> BlockTransferFunc<'mir, 'tcx, T> {
-    #![allow(unused)]
     pub fn new(
         body: &'mir mir::Body<'tcx>,
         trans_for_block: IndexVec<BasicBlock, GenKillSet<T>>,
@@ -527,12 +535,12 @@ where
         for set in &[&block_trans.gen, &block_trans.kill] {
             write!(
                 w,
-                r#"<td {fmt} rowspan="{rowspan}" align="center">"#,
+                r#"<td {fmt} rowspan="{rowspan}" balign="left" align="left">"#,
                 fmt = fmt,
                 rowspan = rowspan
             )?;
 
-            pretty_print_state_elems(&mut w, results.analysis(), set.iter(), "\n", None)?;
+            pretty_print_state_elems(&mut w, results.analysis(), set.iter(), BR_LEFT, None)?;
             write!(w, "</td>")?;
         }
 
@@ -564,25 +572,28 @@ fn write_diff<A: Analysis<'tcx>>(
 
     if !set.is_empty() {
         write!(w, r#"<font color="darkgreen">+"#)?;
-        pretty_print_state_elems(w, analysis, set.iter(), ",", LIMIT_40_ALIGN_1)?;
+        pretty_print_state_elems(w, analysis, set.iter(), ", ", LIMIT_30_ALIGN_1)?;
         write!(w, r#"</font>"#)?;
     }
 
     if !set.is_empty() && !clear.is_empty() {
-        write!(w, "<br/>")?;
+        write!(w, "{}", BR_LEFT)?;
     }
 
     if !clear.is_empty() {
         write!(w, r#"<font color="red">-"#)?;
-        pretty_print_state_elems(w, analysis, clear.iter(), ",", LIMIT_40_ALIGN_1)?;
+        pretty_print_state_elems(w, analysis, clear.iter(), ", ", LIMIT_30_ALIGN_1)?;
         write!(w, r#"</font>"#)?;
     }
 
     Ok(())
 }
 
+const BR_LEFT: &'static str = r#"<br align="left"/>"#;
+const BR_LEFT_SPACE: &'static str = r#"<br align="left"/> "#;
+
 /// Line break policy that breaks at 40 characters and starts the next line with a single space.
-const LIMIT_40_ALIGN_1: Option<LineBreak> = Some(LineBreak { sequence: "<br/> ", limit: 40 });
+const LIMIT_30_ALIGN_1: Option<LineBreak> = Some(LineBreak { sequence: BR_LEFT_SPACE, limit: 30 });
 
 struct LineBreak {
     sequence: &'static str,
@@ -613,25 +624,25 @@ where
     let mut line_break_inserted = false;
 
     for idx in elems {
-        if first {
-            first = false;
-        } else {
-            write!(w, "{}", sep)?;
-            curr_line_width += sep_width;
-        }
-
         buf.clear();
         analysis.pretty_print_idx(&mut buf, idx)?;
         let idx_str =
             str::from_utf8(&buf).expect("Output of `pretty_print_idx` must be valid UTF-8");
         let escaped = dot::escape_html(idx_str);
         let escaped_width = escaped.chars().count();
 
-        if let Some(line_break) = &line_break {
-            if curr_line_width + sep_width + escaped_width > line_break.limit {
-                write!(w, "{}", line_break.sequence)?;
-                line_break_inserted = true;
-                curr_line_width = 0;
+        if first {
+            first = false;
+        } else {
+            write!(w, "{}", sep)?;
+            curr_line_width += sep_width;
+
+            if let Some(line_break) = &line_break {
+                if curr_line_width + sep_width + escaped_width > line_break.limit {
+                    write!(w, "{}", line_break.sequence)?;
+                    line_break_inserted = true;
+                    curr_line_width = 0;
+                }
             }
         }