From cfc069ea77a1788fc4ead01f76c2960fecb7b8ac Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Mon, 13 Feb 2023 05:16:52 +0200 Subject: [PATCH 01/11] print: move `Versions` to a new `multiversion` module. --- src/print/mod.rs | 199 +------------------------------------ src/print/multiversion.rs | 200 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+), 195 deletions(-) create mode 100644 src/print/multiversion.rs diff --git a/src/print/mod.rs b/src/print/mod.rs index 9ef7cab..7d75d72 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -20,6 +20,7 @@ use itertools::Itertools as _; use crate::func_at::FuncAt; +use crate::print::multiversion::Versions; use crate::visit::{DynVisit, InnerVisit, Visit, Visitor}; use crate::{ cfg, spv, AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstCtor, ConstDef, Context, @@ -32,9 +33,9 @@ use crate::{ use rustc_hash::FxHashMap; use smallvec::SmallVec; use std::collections::hash_map::Entry; -use std::fmt::Write; -use std::{fmt, mem}; +use std::mem; +mod multiversion; mod pretty; /// "Definitions-before-uses" / "topo-sorted" printing plan. @@ -448,198 +449,6 @@ impl Visit for AllCxInterned { fn visit_with<'a>(&'a self, _visitor: &mut impl Visitor<'a>) {} } -#[allow(rustdoc::private_intra_doc_links)] -/// Wrapper for handling the difference between single-version and multi-version -/// output, which aren't expressible in [`pretty::Fragment`]. -// -// FIXME(eddyb) introduce a `pretty::Node` variant capable of handling this, -// but that's complicated wrt non-HTML output, if they're to also be 2D tables. -pub enum Versions { - Single(PF), - Multiple { - // FIXME(eddyb) avoid allocating this if possible. - version_names: Vec, - - /// Each node has definitions "tagged" with an `usize` indicating the - /// number of versions that share that definition, aka "repeat count" - /// (i.e. "repeat counts" larger than `1` indicate deduplication). - per_node_versions_with_repeat_count: Vec>, - }, -} - -impl fmt::Display for Versions { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Single(fragment) => fragment.fmt(f), - Self::Multiple { - version_names, - per_node_versions_with_repeat_count, - } => { - let mut first = true; - - // HACK(eddyb) this is not the nicest output, but multi-version - // is intended for HTML input primarily anyway. - for versions_with_repeat_count in per_node_versions_with_repeat_count { - if !first { - writeln!(f)?; - } - first = false; - - let mut next_version_idx = 0; - let mut any_headings = false; - for (fragment, repeat_count) in versions_with_repeat_count { - // No headings for anything uniform across versions. - if (next_version_idx, *repeat_count) != (0, version_names.len()) { - any_headings = true; - - if next_version_idx == 0 { - write!(f, "//#IF ")?; - } else { - write!(f, "//#ELSEIF ")?; - } - let mut first_name = true; - for name in &version_names[next_version_idx..][..*repeat_count] { - if !first_name { - write!(f, " | ")?; - } - first_name = false; - - write!(f, "`{name}`")?; - } - writeln!(f)?; - } - - writeln!(f, "{fragment}")?; - - next_version_idx += repeat_count; - } - if any_headings { - writeln!(f, "//#ENDIF")?; - } - } - - Ok(()) - } - } - } -} - -impl Versions { - // FIXME(eddyb) provide a non-allocating version. - pub fn render_to_html(&self) -> pretty::HtmlSnippet { - match self { - Self::Single(fragment) => fragment.render_to_html(), - Self::Multiple { - version_names, - per_node_versions_with_repeat_count, - } => { - // HACK(eddyb) using an UUID as a class name in lieu of "scoped - " - .replace("SCOPE", &format!("table.{TABLE_CLASS_NAME}")), - ); - - let headings = { - let mut h = "".to_string(); - for name in version_names { - write!(h, "{name}").unwrap(); - } - h + "\n" - }; - - html.body = format!("\n"); - let mut last_was_uniform = true; - for versions_with_repeat_count in per_node_versions_with_repeat_count { - let is_uniform = match versions_with_repeat_count[..] { - [(_, repeat_count)] => repeat_count == version_names.len(), - _ => false, - }; - - if last_was_uniform && is_uniform { - // Headings unnecessary, they would be between uniform - // rows (or at the very start, before an uniform row). - } else { - // Repeat the headings often, where necessary. - html.body += &headings; - } - last_was_uniform = is_uniform; - - html.body += "\n"; - for (fragment, repeat_count) in versions_with_repeat_count { - writeln!(html.body, "\n"; - } - html.body += "\n"; - } - html.body += "
").unwrap(); - - let pretty::HtmlSnippet { - head_deduplicatable_elements: fragment_head, - body: fragment_body, - } = fragment.render_to_html(); - html.head_deduplicatable_elements.extend(fragment_head); - html.body += &fragment_body; - - html.body += "
"; - - html - } - } - } -} - -impl Versions { - fn map_pretty_fragments(self, f: impl Fn(PF) -> PF2) -> Versions { - match self { - Versions::Single(fragment) => Versions::Single(f(fragment)), - Versions::Multiple { - version_names, - per_node_versions_with_repeat_count, - } => Versions::Multiple { - version_names, - per_node_versions_with_repeat_count: per_node_versions_with_repeat_count - .into_iter() - .map(|versions_with_repeat_count| { - versions_with_repeat_count - .into_iter() - .map(|(fragment, repeat_count)| (f(fragment), repeat_count)) - .collect() - }) - .collect(), - }, - } - } -} - impl Plan<'_> { #[allow(rustdoc::private_intra_doc_links)] /// Print the whole [`Plan`] to a [`Versions`] and perform @@ -1439,7 +1248,7 @@ impl Print for Plan<'_> { .iter() .map(|(name, _)| name.clone()) .collect(), - per_node_versions_with_repeat_count: per_node_versions_with_repeat_count.collect(), + per_row_versions_with_repeat_count: per_node_versions_with_repeat_count.collect(), } } } diff --git a/src/print/multiversion.rs b/src/print/multiversion.rs new file mode 100644 index 0000000..13c6dd8 --- /dev/null +++ b/src/print/multiversion.rs @@ -0,0 +1,200 @@ +//! Multi-version pretty-printing support (e.g. for comparing the IR between passes). + +use crate::print::pretty; +use smallvec::SmallVec; +use std::fmt; +use std::fmt::Write; + +#[allow(rustdoc::private_intra_doc_links)] +/// Wrapper for handling the difference between single-version and multi-version +/// output, which aren't expressible in [`pretty::Fragment`]. +// +// FIXME(eddyb) introduce a `pretty::Node` variant capable of handling this, +// but that's complicated wrt non-HTML output, if they're to also be 2D tables. +pub enum Versions { + Single(PF), + Multiple { + // FIXME(eddyb) avoid allocating this if possible. + version_names: Vec, + + /// Each row consists of *deduplicated* (or "run-length encoded") + /// versions, with "repeat count"s larger than `1` indicating that + /// multiple versions (columns) have the exact same content. + /// + /// For HTML output, "repeat count"s map to `colspan` attributes. + per_row_versions_with_repeat_count: Vec>, + }, +} + +impl fmt::Display for Versions { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Single(fragment) => fragment.fmt(f), + Self::Multiple { + version_names, + per_row_versions_with_repeat_count, + } => { + let mut first = true; + + // HACK(eddyb) this is not the nicest output, but multi-version + // is intended for HTML input primarily anyway. + for versions_with_repeat_count in per_row_versions_with_repeat_count { + if !first { + writeln!(f)?; + } + first = false; + + let mut next_version_idx = 0; + let mut any_headings = false; + for (fragment, repeat_count) in versions_with_repeat_count { + // No headings for anything uniform across versions. + if (next_version_idx, *repeat_count) != (0, version_names.len()) { + any_headings = true; + + if next_version_idx == 0 { + write!(f, "//#IF ")?; + } else { + write!(f, "//#ELSEIF ")?; + } + let mut first_name = true; + for name in &version_names[next_version_idx..][..*repeat_count] { + if !first_name { + write!(f, " | ")?; + } + first_name = false; + + write!(f, "`{name}`")?; + } + writeln!(f)?; + } + + writeln!(f, "{fragment}")?; + + next_version_idx += repeat_count; + } + if any_headings { + writeln!(f, "//#ENDIF")?; + } + } + + Ok(()) + } + } + } +} + +impl Versions { + // FIXME(eddyb) provide a non-allocating version. + pub fn render_to_html(&self) -> pretty::HtmlSnippet { + match self { + Self::Single(fragment) => fragment.render_to_html(), + Self::Multiple { + version_names, + per_row_versions_with_repeat_count, + } => { + // HACK(eddyb) using an UUID as a class name in lieu of "scoped + " + .replace("SCOPE", &format!("table.{TABLE_CLASS_NAME}")), + ); + + let headings = { + let mut h = "".to_string(); + for name in version_names { + write!(h, "{name}").unwrap(); + } + h + "\n" + }; + + html.body = format!("\n"); + let mut last_was_uniform = true; + for versions_with_repeat_count in per_row_versions_with_repeat_count { + let is_uniform = match versions_with_repeat_count[..] { + [(_, repeat_count)] => repeat_count == version_names.len(), + _ => false, + }; + + if last_was_uniform && is_uniform { + // Headings unnecessary, they would be between uniform + // rows (or at the very start, before an uniform row). + } else { + // Repeat the headings often, where necessary. + html.body += &headings; + } + last_was_uniform = is_uniform; + + html.body += "\n"; + for (fragment, repeat_count) in versions_with_repeat_count { + writeln!(html.body, "\n"; + } + html.body += "\n"; + } + html.body += "
").unwrap(); + + let pretty::HtmlSnippet { + head_deduplicatable_elements: fragment_head, + body: fragment_body, + } = fragment.render_to_html(); + html.head_deduplicatable_elements.extend(fragment_head); + html.body += &fragment_body; + + html.body += "
"; + + html + } + } + } +} + +impl Versions { + pub fn map_pretty_fragments(self, f: impl Fn(PF) -> PF2) -> Versions { + match self { + Versions::Single(fragment) => Versions::Single(f(fragment)), + Versions::Multiple { + version_names, + per_row_versions_with_repeat_count, + } => Versions::Multiple { + version_names, + per_row_versions_with_repeat_count: per_row_versions_with_repeat_count + .into_iter() + .map(|versions_with_repeat_count| { + versions_with_repeat_count + .into_iter() + .map(|(fragment, repeat_count)| (f(fragment), repeat_count)) + .collect() + }) + .collect(), + }, + } + } +} From 7624d0e7a8290ec0f897a5a356a67b7597c382f3 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Mon, 13 Feb 2023 13:01:46 +0200 Subject: [PATCH 02/11] print/pretty: use the `internal-iterator` crate for `LineOp`s/`TextOp`s. --- Cargo.toml | 1 + src/print/pretty.rs | 375 ++++++++++++++++++++++++++------------------ 2 files changed, 224 insertions(+), 152 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8688bed..7a37975 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ arrayvec = "0.7.1" bytemuck = "1.12.3" elsa = { version = "1.6.0", features = ["indexmap"] } indexmap = "1.7.0" +internal-iterator = "0.2.0" itertools = "0.10.3" lazy_static = "1.4.0" rustc-hash = "1.1.0" diff --git a/src/print/pretty.rs b/src/print/pretty.rs index 71a9e71..28f82da 100644 --- a/src/print/pretty.rs +++ b/src/print/pretty.rs @@ -1,9 +1,13 @@ //! Pretty-printing functionality (such as automatic indentation). use indexmap::IndexSet; +use internal_iterator::{ + FromInternalIterator, InternalIterator, IntoInternalIterator, IteratorExt, +}; use smallvec::SmallVec; use std::borrow::Cow; use std::fmt::Write as _; +use std::ops::ControlFlow; use std::{fmt, iter, mem}; /// Part of a pretty document, made up of [`Node`]s. @@ -155,16 +159,30 @@ pub struct FragmentPostLayout(Fragment); impl fmt::Display for FragmentPostLayout { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut result = Ok(()); - self.0.render_to_line_ops( - &mut LineOp::interpret_with(|op| { - if let TextOp::Text(text) = op { - result = result.and_then(|_| f.write_str(text)); - } - }), - false, - ); - result + let result = self + .0 + .render_to_text_ops() + .filter_map(|op| match op { + TextOp::Text(text) => Some(text), + _ => None, + }) + .try_for_each(|text| { + f.write_str(text) + .map_or_else(ControlFlow::Break, ControlFlow::Continue) + }); + match result { + ControlFlow::Continue(()) => Ok(()), + ControlFlow::Break(e) => Err(e), + } + } +} + +impl FragmentPostLayout { + /// Flatten the [`Fragment`] to HTML, producing a [`HtmlSnippet`]. + // + // FIXME(eddyb) provide a non-allocating version. + pub fn render_to_html(&self) -> HtmlSnippet { + self.0.render_to_text_ops().collect() } } @@ -252,11 +270,11 @@ impl HtmlSnippet { } } -impl FragmentPostLayout { - /// Flatten the [`Fragment`] to HTML, producing a [`HtmlSnippet`]. - // - // FIXME(eddyb) provide a non-allocating version. - pub fn render_to_html(&self) -> HtmlSnippet { +impl<'a> FromInternalIterator> for HtmlSnippet { + fn from_iter(text_ops: T) -> Self + where + T: IntoInternalIterator>, + { // HACK(eddyb) using an UUID as a class name in lieu of "scoped " .replace("SCOPE", &format!("pre.{ROOT_CLASS_NAME}")); From 11718b32b3c94b14b1a93afeefa27a8ea582d004 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 14 Feb 2023 07:41:00 +0200 Subject: [PATCH 04/11] print/multiversion: align adjacent columns' lines, to match up their anchors. --- Cargo.toml | 1 + src/print/multiversion.rs | 281 +++++++++++++++++++++++++++++++++++++- src/print/pretty.rs | 12 +- 3 files changed, 287 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7a37975..d074449 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ indexmap = "1.7.0" internal-iterator = "0.2.0" itertools = "0.10.3" lazy_static = "1.4.0" +longest-increasing-subsequence = "0.1.0" rustc-hash = "1.1.0" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" diff --git a/src/print/multiversion.rs b/src/print/multiversion.rs index 13c6dd8..8eee2de 100644 --- a/src/print/multiversion.rs +++ b/src/print/multiversion.rs @@ -1,9 +1,14 @@ //! Multi-version pretty-printing support (e.g. for comparing the IR between passes). -use crate::print::pretty; +use crate::print::pretty::{self, TextOp}; +use crate::FxIndexMap; +use internal_iterator::{ + FromInternalIterator, InternalIterator, IntoInternalIterator, IteratorExt, +}; +use itertools::Itertools; use smallvec::SmallVec; -use std::fmt; use std::fmt::Write; +use std::{fmt, mem}; #[allow(rustdoc::private_intra_doc_links)] /// Wrapper for handling the difference between single-version and multi-version @@ -153,14 +158,25 @@ impl Versions { } last_was_uniform = is_uniform; + // Attempt to align as many anchors as possible between the + // columns, to improve legibility (see also `AnchorAligner`). + let mut anchor_aligner = AnchorAligner::default(); + for (fragment, _) in versions_with_repeat_count { + anchor_aligner + .add_column_and_align_anchors(fragment.render_to_text_ops().collect()); + } + html.body += "\n"; - for (fragment, repeat_count) in versions_with_repeat_count { + for ((_, repeat_count), column) in versions_with_repeat_count + .iter() + .zip(anchor_aligner.merged_columns()) + { writeln!(html.body, "").unwrap(); let pretty::HtmlSnippet { head_deduplicatable_elements: fragment_head, body: fragment_body, - } = fragment.render_to_html(); + } = column.into_internal().collect(); html.head_deduplicatable_elements.extend(fragment_head); html.body += &fragment_body; @@ -198,3 +214,260 @@ impl Versions { } } } + +/// Tool for adjusting pretty-printed columns, so that their anchors line up +/// (by adding empty lines to whichever side "is behind"). +#[derive(Default)] +struct AnchorAligner<'a> { + merged_lines: Vec, + + /// Current ("rightmost") column's anchor definitions (with indices pointing + /// into `merged_lines`), which the next column will align to. + // + // FIXME(eddyb) does this need additional interning? + anchor_def_to_merged_line_idx: FxIndexMap<&'a String, usize>, + + // FIXME(eddyb) fine-tune this inline size. + // FIXME(eddyb) maybe don't keep most of this data around anyway? + original_columns: SmallVec<[AAColumn<'a>; 4]>, +} + +/// Abstraction for one "physical" line spanning all columns, after alignment. +struct AAMergedLine { + // FIXME(eddyb) fine-tune this inline size. + // FIXME(eddyb) consider using `u32` here? + per_column_line_lengths: SmallVec<[usize; 4]>, +} + +struct AAColumn<'a> { + /// All `TextOp`s in all lines from this column, concatenated together. + text_ops: Vec>, + + /// The length, in `TextOp`s (from `text_ops`), of each line. + // + // FIXME(eddyb) consider using `u32` here? + line_lengths: Vec, +} + +impl<'a> AAColumn<'a> { + /// Reconstruct lines (made of `TextOp`s) from line lengths. + fn lines( + &self, + line_lengths: impl Iterator, + ) -> impl Iterator]> { + let mut next_start = 0; + line_lengths.map(move |len| { + let start = next_start; + let end = start + len; + next_start = end; + &self.text_ops[start..end] + }) + } +} + +// FIXME(eddyb) is this impl the best way? (maybe it should be a inherent method) +impl<'a> FromInternalIterator> for AAColumn<'a> { + fn from_iter(text_ops: T) -> Self + where + T: IntoInternalIterator>, + { + let mut column = AAColumn { + text_ops: vec![], + line_lengths: vec![0], + }; + text_ops.into_internal_iter().for_each(|op| { + if let TextOp::Text("\n") = op { + column.line_lengths.push(0); + } else { + // FIXME(eddyb) this *happens* to be true, + // but the `LineOp`/`TextOp` split could be + // improved to avoid such sanity checks. + if let TextOp::Text(text) = op { + assert!(!text.contains('\n')); + } + column.text_ops.push(op); + *column.line_lengths.last_mut().unwrap() += 1; + } + }); + column + } +} + +impl<'a> AnchorAligner<'a> { + /// Flatten all columns to `TextOp`s (including line separators). + fn merged_columns(&self) -> impl Iterator> + '_> { + self.original_columns + .iter() + .enumerate() + .map(|(column_idx, column)| { + let line_lengths = self + .merged_lines + .iter() + .map(move |line| line.per_column_line_lengths[column_idx]); + + // HACK(eddyb) trim all trailing empty lines (which is done on + // a `peekable` of the reverse, followed by reversing *again*, + // equivalent to a hypothetical `peekable_back` and no reversing). + let mut rev_line_lengths = line_lengths.rev().peekable(); + while rev_line_lengths.peek() == Some(&0) { + rev_line_lengths.next().unwrap(); + } + let line_lengths = rev_line_lengths.rev(); + + column + .lines(line_lengths) + .intersperse(&[TextOp::Text("\n")]) + .flatten() + .copied() + }) + } + + /// Merge `new_column` into the current set of columns, aligning as many + /// anchors as possible, between it, and the most recent column. + fn add_column_and_align_anchors(&mut self, new_column: AAColumn<'a>) { + // NOTE(eddyb) "old" and "new" are used to refer to the two columns being + // aligned, but "old" maps to the *merged* lines, not its original ones. + + let old_lines = mem::take(&mut self.merged_lines); + let old_anchor_def_to_line_idx = mem::take(&mut self.anchor_def_to_merged_line_idx); + + // Index all the anchor definitions in the new column. + let mut new_anchor_def_to_line_idx = FxIndexMap::default(); + for (new_line_idx, new_line_text_ops) in new_column + .lines(new_column.line_lengths.iter().copied()) + .enumerate() + { + for op in new_line_text_ops { + if let TextOp::PushStyles(styles) = op { + if let Some(anchor) = &styles.anchor { + if styles.anchor_is_def { + new_anchor_def_to_line_idx + .entry(anchor) + .or_insert(new_line_idx); + } + } + } + } + } + + // Find all the possible anchor alignments (i.e. anchors defined in both + // "old" and "new") as pairs of line indices in "old" and "new". + // + // HACK(eddyb) the order is given by the "new" line index, implicitly. + // FIXME(eddyb) fine-tune this inline size. + let common_anchors: SmallVec<[_; 8]> = new_anchor_def_to_line_idx + .iter() + .filter_map(|(anchor, &new_line_idx)| { + Some((*old_anchor_def_to_line_idx.get(anchor)?, new_line_idx)) + }) + .collect(); + + // Fast-path: if all the "old" line indices are already in (increasing) + // order (i.e. "monotonic"), they can all be used directly for alignment. + let is_already_monotonic = { + // FIXME(eddyb) should be `.is_sorted_by_key(|&(old_line_idx, _)| old_line_idx)` + // but that slice method is still unstable. + common_anchors.windows(2).all(|w| w[0].0 <= w[1].0) + }; + let monotonic_common_anchors = if is_already_monotonic { + common_anchors + } else { + // FIXME(eddyb) this could maybe avoid all the unnecessary allocations. + longest_increasing_subsequence::lis(&common_anchors) + .into_iter() + .map(|i| common_anchors[i]) + .collect() + }; + + // Allocate space for the merge of "old" and "new". + let mut merged_lines = Vec::with_capacity({ + // Cheap conservative estimate, based on the last anchor (i.e. the + // final position of the last anchor is *at least* `min_before_last`). + let &(old_last, new_last) = monotonic_common_anchors.last().unwrap_or(&(0, 0)); + let min_before_last = old_last.max(new_last); + let after_last = + (old_lines.len() - old_last).max(new_column.line_lengths.len() - new_last); + (min_before_last + after_last).next_power_of_two() + }); + + // Build the merged lines using (partially) lockstep iteration to pull + // the relevant data out of either side, and update "new" line indices. + let mut old_lines = old_lines.into_iter().enumerate().peekable(); + let mut new_lines = new_column + .line_lengths + .iter() + .copied() + .enumerate() + .peekable(); + let mut monotonic_common_anchors = monotonic_common_anchors.into_iter().peekable(); + let mut fixup_new_to_merged = new_anchor_def_to_line_idx.values_mut().peekable(); + while old_lines.len() > 0 || new_lines.len() > 0 { + let old_line_idx = old_lines.peek().map(|&(i, _)| i); + let new_line_idx = new_lines.peek().map(|&(i, _)| i); + let mut next_anchor = monotonic_common_anchors.peek().copied(); + + // Discard anchor alignments that have been used already, and also + // any others that cannot be relevant anymore - this can occur when + // multiple anchors coincide on the same line. + while let Some((anchor_old, anchor_new)) = next_anchor { + let obsolete = old_line_idx.map_or(false, |old| old > anchor_old) + || new_line_idx.map_or(false, |new| new > anchor_new); + if !obsolete { + break; + } + monotonic_common_anchors.next().unwrap(); + next_anchor = monotonic_common_anchors.peek().copied(); + } + + // Figure out which side has to wait, to align an upcoming anchor. + let (old_at_anchor, new_at_anchor) = + next_anchor.map_or((false, false), |(anchor_old, anchor_new)| { + ( + old_line_idx.map_or(false, |old| old == anchor_old), + new_line_idx.map_or(false, |new| new == anchor_new), + ) + }); + let old_line = if old_at_anchor && !new_at_anchor { + // Pausing "old", waiting for "new". + None + } else { + old_lines.next().map(|(_, old_line)| old_line) + }; + let new_line_len = if !old_at_anchor && new_at_anchor { + // Pausing "new", waiting for "old". + None + } else { + new_lines.next().map(|(_, new_line_len)| new_line_len) + }; + + // When the "new" side is advanced, that "sets" the merged line index + // of the consumed line, which can then be used for fixing up indices. + if new_line_len.is_some() { + let new_line_idx = new_line_idx.unwrap(); + let merged_line_idx = merged_lines.len(); + while fixup_new_to_merged.peek().map(|i| **i) == Some(new_line_idx) { + *fixup_new_to_merged.next().unwrap() = merged_line_idx; + } + } + + let new_line_len = new_line_len.unwrap_or(0); + let merged_line = match old_line { + Some(mut line) => { + line.per_column_line_lengths.push(new_line_len); + line + } + None => AAMergedLine { + per_column_line_lengths: (0..self.original_columns.len()) + .map(|_| 0) + .chain([new_line_len]) + .collect(), + }, + }; + merged_lines.push(merged_line); + } + + self.merged_lines = merged_lines; + self.anchor_def_to_merged_line_idx = new_anchor_def_to_line_idx; + self.original_columns.push(new_column); + } +} diff --git a/src/print/pretty.rs b/src/print/pretty.rs index dcb940d..d80c5bf 100644 --- a/src/print/pretty.rs +++ b/src/print/pretty.rs @@ -160,7 +160,6 @@ pub struct FragmentPostLayout(Fragment); impl fmt::Display for FragmentPostLayout { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let result = self - .0 .render_to_text_ops() .filter_map(|op| match op { TextOp::Text(text) => Some(text), @@ -178,11 +177,16 @@ impl fmt::Display for FragmentPostLayout { } impl FragmentPostLayout { + /// Flatten the [`Fragment`] to [`TextOp`]s. + pub(super) fn render_to_text_ops(&self) -> impl InternalIterator> { + self.0.render_to_text_ops() + } + /// Flatten the [`Fragment`] to HTML, producing a [`HtmlSnippet`]. // // FIXME(eddyb) provide a non-allocating version. pub fn render_to_html(&self) -> HtmlSnippet { - self.0.render_to_text_ops().collect() + self.render_to_text_ops().collect() } } @@ -270,6 +274,7 @@ impl HtmlSnippet { } } +// FIXME(eddyb) is this impl the best way? (maybe it should be a inherent method) impl<'a> FromInternalIterator> for HtmlSnippet { fn from_iter(text_ops: T) -> Self where @@ -821,7 +826,8 @@ impl Fragment { } /// Text-oriented operation (plain text snippets interleaved with style push/pop). -enum TextOp<'a> { +#[derive(Copy, Clone)] +pub(super) enum TextOp<'a> { PushStyles(&'a Styles), PopStyles(&'a Styles), From 37f69e548d06a964f4b395eb86a326fef785197b Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 31 Mar 2023 14:51:16 +0300 Subject: [PATCH 05/11] print: generate extra anchors for multiversion alignment. --- src/print/mod.rs | 168 +++++++++++++++++++++++++++++++------- src/print/multiversion.rs | 15 +++- 2 files changed, 152 insertions(+), 31 deletions(-) diff --git a/src/print/mod.rs b/src/print/mod.rs index 7d75d72..5749acc 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -183,6 +183,12 @@ enum Use { output_idx: u32, }, DataInstOutput(DataInst), + + // NOTE(eddyb) these overlap somewhat with other cases, but they're always + // generated, even when there is no "use", for `multiversion` alignment. + AlignmentAnchorForControlRegion(ControlRegion), + AlignmentAnchorForControlNode(ControlNode), + AlignmentAnchorForDataInst(DataInst), } impl From for Use { @@ -205,14 +211,24 @@ impl From for Use { } impl Use { + // HACK(eddyb) this is used in `AttrsAndDef::insert_name_before_def` to + // detect alignment anchors specifically, so it needs to not overlap with + // any other category name. + const ANCHOR_ALIGNMENT_CATEGORY: &str = "AA"; + fn category(self) -> &'static str { match self { Self::Node(node) => node.category().unwrap(), Self::CxInterned(interned) => interned.category(), Self::ControlRegionLabel(_) => "label", + Self::ControlRegionInput { .. } | Self::ControlNodeOutput { .. } | Self::DataInstOutput(_) => "v", + + Self::AlignmentAnchorForControlRegion(_) + | Self::AlignmentAnchorForControlNode(_) + | Self::AlignmentAnchorForDataInst(_) => Self::ANCHOR_ALIGNMENT_CATEGORY, } } } @@ -587,7 +603,10 @@ impl<'a> Printer<'a> { Use::ControlRegionLabel(_) | Use::ControlRegionInput { .. } | Use::ControlNodeOutput { .. } - | Use::DataInstOutput(_) => { + | Use::DataInstOutput(_) + | Use::AlignmentAnchorForControlRegion(_) + | Use::AlignmentAnchorForControlNode(_) + | Use::AlignmentAnchorForDataInst(_) => { unreachable!() } }; @@ -601,6 +620,7 @@ impl<'a> Printer<'a> { Use::CxInterned(CxInterned::Const(_)) => &mut ac.consts, Use::Node(Node::GlobalVar(_)) => &mut ac.global_vars, Use::Node(Node::Func(_)) => &mut ac.funcs, + Use::Node( Node::Root | Node::AllCxInterned @@ -610,7 +630,10 @@ impl<'a> Printer<'a> { | Use::ControlRegionLabel(_) | Use::ControlRegionInput { .. } | Use::ControlNodeOutput { .. } - | Use::DataInstOutput(_) => { + | Use::DataInstOutput(_) + | Use::AlignmentAnchorForControlRegion(_) + | Use::AlignmentAnchorForControlNode(_) + | Use::AlignmentAnchorForDataInst(_) => { unreachable!() } }; @@ -640,16 +663,32 @@ impl<'a> Printer<'a> { let mut control_region_label_counter = 0; let mut value_counter = 0; + let mut alignment_anchor_counter = 0; - // Assign a new label/value index, but only if: - // * the definition is actually used + // Assign a new label/value/alignment-anchor index, but only if: + // * the definition is actually used (except for alignment anchors) // * it doesn't already have an index (e.g. from a previous version) - let mut define_label_or_value = |use_kind: Use| { - if let Some(use_style @ UseStyle::Inline) = use_styles.get_mut(&use_kind) { - let counter = match use_kind { - Use::ControlRegionLabel(_) => &mut control_region_label_counter, - _ => &mut value_counter, - }; + let mut define = |use_kind: Use| { + let (counter, use_style_slot) = match use_kind { + Use::ControlRegionLabel(_) => ( + &mut control_region_label_counter, + use_styles.get_mut(&use_kind), + ), + + Use::ControlRegionInput { .. } + | Use::ControlNodeOutput { .. } + | Use::DataInstOutput(_) => (&mut value_counter, use_styles.get_mut(&use_kind)), + + Use::AlignmentAnchorForControlRegion(_) + | Use::AlignmentAnchorForControlNode(_) + | Use::AlignmentAnchorForDataInst(_) => ( + &mut alignment_anchor_counter, + Some(use_styles.entry(use_kind).or_insert(UseStyle::Inline)), + ), + + _ => unreachable!(), + }; + if let Some(use_style @ UseStyle::Inline) = use_style_slot { let idx = *counter; *counter += 1; *use_style = UseStyle::Anon { @@ -677,7 +716,8 @@ impl<'a> Printer<'a> { let visit_region = |func_at_region: FuncAt<'_, ControlRegion>| { let region = func_at_region.position; - define_label_or_value(Use::ControlRegionLabel(region)); + define(Use::AlignmentAnchorForControlRegion(region)); + define(Use::ControlRegionLabel(region)); let ControlRegionDef { inputs, @@ -686,7 +726,7 @@ impl<'a> Printer<'a> { } = func_def_body.at(region).def(); for (i, _) in inputs.iter().enumerate() { - define_label_or_value(Use::ControlRegionInput { + define(Use::ControlRegionInput { region, input_idx: i.try_into().unwrap(), }); @@ -694,20 +734,22 @@ impl<'a> Printer<'a> { for func_at_control_node in func_def_body.at(*children) { let control_node = func_at_control_node.position; + + define(Use::AlignmentAnchorForControlNode(control_node)); + let ControlNodeDef { kind, outputs } = func_at_control_node.def(); if let ControlNodeKind::Block { insts } = *kind { for func_at_inst in func_def_body.at(insts) { + define(Use::AlignmentAnchorForDataInst(func_at_inst.position)); if func_at_inst.def().output_type.is_some() { - define_label_or_value(Use::DataInstOutput( - func_at_inst.position, - )); + define(Use::DataInstOutput(func_at_inst.position)); } } } for (i, _) in outputs.iter().enumerate() { - define_label_or_value(Use::ControlNodeOutput { + define(Use::ControlNodeOutput { control_node, output_idx: i.try_into().unwrap(), }); @@ -1004,20 +1046,69 @@ impl AttrsAndDef { } = self; let mut maybe_hoisted_anchor = pretty::Fragment::default(); + let mut maybe_def_start_anchor = pretty::Fragment::default(); + let mut maybe_def_end_anchor = pretty::Fragment::default(); let mut name = name.into(); - if let [pretty::Node::StyledText(ref mut styles_and_text), ..] = name.nodes[..] { - let styles = &mut styles_and_text.0; - if !attrs.nodes.is_empty() && mem::take(&mut styles.anchor_is_def) { - maybe_hoisted_anchor = pretty::Styles { - anchor: styles.anchor.clone(), - anchor_is_def: true, - ..Default::default() + if let [pretty::Node::StyledText(name_start_styles_and_text), ..] = &mut name.nodes[..] { + let name_start_styles = &mut name_start_styles_and_text.0; + if name_start_styles.anchor_is_def { + let anchor = name_start_styles.anchor.as_ref().unwrap(); + if !attrs.nodes.is_empty() { + name_start_styles.anchor_is_def = false; + maybe_hoisted_anchor = pretty::Styles { + anchor: Some(anchor.clone()), + anchor_is_def: true, + ..Default::default() + } + .apply("") + .into(); + } + + // HACK(eddyb) add a pair of anchors "bracketing" the definition + // (though see below for why only the "start" side is currently + // in use), to help with `multiversion` alignment, as long as + // there's no alignment anchor already starting the definition. + let has_alignment_anchor = match &def_without_name.nodes[..] { + [pretty::Node::StyledText(def_start_styles_and_text), ..] => { + let (def_start_styles, def_start_text) = &**def_start_styles_and_text; + def_start_text == "" + && def_start_styles.anchor_is_def + && def_start_styles + .anchor + .as_ref() + .unwrap() + .contains(Use::ANCHOR_ALIGNMENT_CATEGORY) + } + _ => false, + }; + let mk_anchor_def = |suffix| { + pretty::Styles { + anchor: Some(format!("{anchor}.{suffix}")), + anchor_is_def: true, + ..Default::default() + } + .apply("") + .into() + }; + if !has_alignment_anchor { + maybe_def_start_anchor = mk_anchor_def("start"); + // FIXME(eddyb) having end alignment may be useful, but the + // current logic in `multiversion` would prefer aligning + // the ends, to the detriment of the rest (causing huge gaps). + if false { + maybe_def_end_anchor = mk_anchor_def("end"); + } } - .apply("") - .into(); } } - pretty::Fragment::new([maybe_hoisted_anchor, attrs, name, def_without_name]) + pretty::Fragment::new([ + maybe_hoisted_anchor, + attrs, + name, + maybe_def_start_anchor, + def_without_name, + maybe_def_end_anchor, + ]) } } @@ -1089,6 +1180,11 @@ impl Use { Self::CxInterned(CxInterned::AttrSet(_)) => { (format!("#{name}"), printer.attr_style()) } + + Self::AlignmentAnchorForControlRegion(_) + | Self::AlignmentAnchorForControlNode(_) + | Self::AlignmentAnchorForDataInst(_) => ("".to_string(), Default::default()), + _ => (name, Default::default()), }; let name = pretty::Styles { @@ -1120,6 +1216,10 @@ impl Use { | Self::ControlRegionInput { .. } | Self::ControlNodeOutput { .. } | Self::DataInstOutput(_) => "_".into(), + + Self::AlignmentAnchorForControlRegion(_) + | Self::AlignmentAnchorForControlNode(_) + | Self::AlignmentAnchorForDataInst(_) => unreachable!(), }, } } @@ -2127,6 +2227,7 @@ impl Print for FuncAt<'_, ControlRegion> { }; pretty::Fragment::new([ + Use::AlignmentAnchorForControlRegion(self.position).print_as_def(printer), self.at(*children).into_iter().print(printer), outputs_footer, ]) @@ -2182,7 +2283,14 @@ impl Print for FuncAt<'_, ControlNode> { .into_iter() .map(|func_at_inst| { let data_inst_def = func_at_inst.def(); - data_inst_def.print(printer).insert_name_before_def( + let mut data_inst_attrs_and_def = data_inst_def.print(printer); + // FIXME(eddyb) this is quite verbose for prepending. + data_inst_attrs_and_def.def_without_name = pretty::Fragment::new([ + Use::AlignmentAnchorForDataInst(func_at_inst.position) + .print_as_def(printer), + data_inst_attrs_and_def.def_without_name, + ]); + data_inst_attrs_and_def.insert_name_before_def( if data_inst_def.output_type.is_none() { pretty::Fragment::default() } else { @@ -2293,7 +2401,11 @@ impl Print for FuncAt<'_, ControlNode> { ]) } }; - pretty::Fragment::new([outputs_header, control_node_body]) + pretty::Fragment::new([ + Use::AlignmentAnchorForControlNode(self.position).print_as_def(printer), + outputs_header, + control_node_body, + ]) } } diff --git a/src/print/multiversion.rs b/src/print/multiversion.rs index 8eee2de..2b2d70d 100644 --- a/src/print/multiversion.rs +++ b/src/print/multiversion.rs @@ -410,9 +410,18 @@ impl<'a> AnchorAligner<'a> { // any others that cannot be relevant anymore - this can occur when // multiple anchors coincide on the same line. while let Some((anchor_old, anchor_new)) = next_anchor { - let obsolete = old_line_idx.map_or(false, |old| old > anchor_old) - || new_line_idx.map_or(false, |new| new > anchor_new); - if !obsolete { + // NOTE(eddyb) noop anchors (i.e. those describing an alignment + // between "old" and "new", which has already beeing reached) + // are not considered "relevant" here, and "misalignments" are + // preferred instead - the outcome is mostly identical to always + // eagerly processing noop anchors, except when another anchor + // is overlapping (in only one of "old" or "new"), as it will + // only get get processed if the noop one is skipped first. + let relevant = match (old_line_idx, new_line_idx) { + (Some(old), Some(new)) => old < anchor_old || new < anchor_new, + _ => false, + }; + if relevant { break; } monotonic_common_anchors.next().unwrap(); From b3dd18fc6204ac314f9add91b9f2c85644dded31 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 31 Mar 2023 15:28:24 +0300 Subject: [PATCH 06/11] print/pretty: work around `
\n...` behaving like
 `
...`.

---
 src/print/pretty.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/print/pretty.rs b/src/print/pretty.rs
index d80c5bf..4bd8cda 100644
--- a/src/print/pretty.rs
+++ b/src/print/pretty.rs
@@ -309,7 +309,10 @@ impl<'a> FromInternalIterator> for HtmlSnippet {
 "
         .replace("SCOPE", &format!("pre.{ROOT_CLASS_NAME}"));
 
-        let mut body = format!("
");
+        // HACK(eddyb) load-bearing newline after `
`, to front-load any
+        // weird HTML whitespace handling, and allow the actual contents to start
+        // with empty lines (i.e. `\n\n...`), without e.g. losing the first one.
+        let mut body = format!("
\n");
         text_ops.into_internal_iter().for_each(|op| match op {
             TextOp::PushStyles(styles) | TextOp::PopStyles(styles) => {
                 let mut special_tags = [

From 0cd98407548e86963fed104a48ab6becbbe4c3e1 Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu 
Date: Fri, 31 Mar 2023 15:29:46 +0300
Subject: [PATCH 07/11] print/pretty: avoid creating spurious empty lines, for
 empty anchors.

---
 src/print/pretty.rs | 132 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 107 insertions(+), 25 deletions(-)

diff --git a/src/print/pretty.rs b/src/print/pretty.rs
index 4bd8cda..0131995 100644
--- a/src/print/pretty.rs
+++ b/src/print/pretty.rs
@@ -708,6 +708,10 @@ enum LineOp<'a> {
     PushStyles(&'a Styles),
     PopStyles(&'a Styles),
 
+    // HACK(eddyb) `PushStyles`+`PopStyles`, indicating no visible text is needed
+    // (i.e. this is only for helper anchors, which only need vertical positioning).
+    StyledEmptyText(&'a Styles),
+
     AppendToLine(&'a str),
     StartNewLine,
     BreakIfWithinLine(Break),
@@ -772,8 +776,12 @@ impl Node {
                 text_render_to_line_ops(None, text).try_for_each(each_line_op)?;
             }
             Self::StyledText(styles_and_text) => {
-                text_render_to_line_ops(Some(&styles_and_text.0), &styles_and_text.1)
-                    .try_for_each(each_line_op)?;
+                let (styles, text) = &**styles_and_text;
+                if text.is_empty() {
+                    each_line_op(LineOp::StyledEmptyText(styles))?;
+                } else {
+                    text_render_to_line_ops(Some(styles), text).try_for_each(each_line_op)?;
+                }
             }
 
             Self::IndentedBlock(fragments) => {
@@ -864,14 +872,35 @@ impl<'a> LineOp<'a> {
     ) -> ControlFlow {
         let mut indent = 0;
 
-        // When `on_empty_new_line` is `true`, a new line was started, but
-        // lacks text, so the `LineOp::AppendToLine { indent_before, text }`
-        // first on that line (with non-empty `text`) needs to materialize
-        // `indent_before` levels of indentation (before its `text` content).
-        //
-        // NOTE(eddyb) indentation is not immediatelly materialized in order
-        // to avoid trailing whitespace on otherwise-empty lines.
-        let mut on_empty_new_line = true;
+        enum LineState {
+            /// This line was just started, lacking any text.
+            ///
+            /// The first (non-empty) `LineOp::AppendToLine` on that line, or
+            /// `LineOp::PushStyles`/`LineOp::PopStyles`, needs to materialize
+            /// `indent` levels of indentation (before emitting its `TextOp`s).
+            //
+            // NOTE(eddyb) indentation is not immediatelly materialized in order
+            // to avoid trailing whitespace on otherwise-empty lines.
+            Empty,
+
+            /// This line has `indent_so_far` levels of indentation, and may have
+            /// styling applied to it, but lacks any other text.
+            ///
+            /// Only used by `LineOp::StyledEmptyText` (i.e. helper anchors),
+            /// to avoid them adding trailing-whitespace-only lines.
+            //
+            // NOTE(eddyb) the new line is started by `StyledEmptyText` so that
+            // there remains separation with the previous (unrelated) line,
+            // whereas the following lines are very likely related to the
+            // helper anchor (but if that changes, this would need to be fixed).
+            // HACK(eddyb) `StyledEmptyText` uses `indent_so_far: 0` to
+            // allow lower-indentation text to follow on the same line.
+            OnlyIndentedOrStyled { indent_so_far: usize },
+
+            /// This line has had text emitted (other than indentation).
+            HasText,
+        }
+        let mut line_state = LineState::Empty;
 
         // Deferred `LineOp::BreakIfWithinLine`, which will be materialized
         // only between two consecutive `LineOp::AppendToLine { text, .. }`
@@ -884,22 +913,59 @@ impl<'a> LineOp<'a> {
                 return ControlFlow::Continue(());
             }
 
-            if let LineOp::AppendToLine(_) | LineOp::PushStyles(_) = op {
-                let need_indent = match pending_break_if_within_line.take() {
-                    Some(br) => {
-                        each_text_op(TextOp::Text(match br {
-                            Break::Space => " ",
-                            Break::NewLine => "\n",
-                        }))?;
-                        matches!(br, Break::NewLine)
+            if let LineOp::AppendToLine(_)
+            | LineOp::PushStyles(_)
+            | LineOp::PopStyles(_)
+            | LineOp::StyledEmptyText(_) = op
+            {
+                if let Some(br) = pending_break_if_within_line.take() {
+                    each_text_op(TextOp::Text(match br {
+                        Break::Space => " ",
+                        Break::NewLine => "\n",
+                    }))?;
+                    if matches!(br, Break::NewLine) {
+                        line_state = LineState::Empty;
+                    }
+                }
+
+                let target_indent = match line_state {
+                    // HACK(eddyb) `StyledEmptyText` uses `indent_so_far: 0` to
+                    // allow lower-indentation text to follow on the same line.
+                    LineState::Empty | LineState::OnlyIndentedOrStyled { indent_so_far: 0 }
+                        if matches!(op, LineOp::StyledEmptyText(_)) =>
+                    {
+                        Some(0)
                     }
-                    None => on_empty_new_line,
+
+                    LineState::Empty | LineState::OnlyIndentedOrStyled { .. } => Some(indent),
+                    LineState::HasText => None,
                 };
-                if need_indent {
-                    for _ in 0..indent {
+
+                if let Some(target_indent) = target_indent {
+                    let indent_so_far = match line_state {
+                        LineState::Empty => 0,
+
+                        // FIXME(eddyb) `StyledEmptyText` doesn't need this, so this
+                        // is perhaps unnecessarily over-engineered? (see above)
+                        LineState::OnlyIndentedOrStyled { indent_so_far } => {
+                            // Disallow reusing lines already indented too much.
+                            if indent_so_far > target_indent {
+                                each_text_op(TextOp::Text("\n"))?;
+                                line_state = LineState::Empty;
+                                0
+                            } else {
+                                indent_so_far
+                            }
+                        }
+
+                        LineState::HasText => unreachable!(),
+                    };
+                    for _ in indent_so_far..target_indent {
                         each_text_op(TextOp::Text(INDENT))?;
                     }
-                    on_empty_new_line = false;
+                    line_state = LineState::OnlyIndentedOrStyled {
+                        indent_so_far: target_indent,
+                    };
                 }
             }
 
@@ -916,17 +982,33 @@ impl<'a> LineOp<'a> {
                 LineOp::PushStyles(styles) => each_text_op(TextOp::PushStyles(styles))?,
                 LineOp::PopStyles(styles) => each_text_op(TextOp::PopStyles(styles))?,
 
-                LineOp::AppendToLine(text) => each_text_op(TextOp::Text(text))?,
+                LineOp::StyledEmptyText(styles) => {
+                    each_text_op(TextOp::PushStyles(styles))?;
+                    each_text_op(TextOp::PopStyles(styles))?;
+                }
+
+                LineOp::AppendToLine(text) => {
+                    each_text_op(TextOp::Text(text))?;
+
+                    line_state = LineState::HasText;
+                }
 
                 LineOp::StartNewLine => {
                     each_text_op(TextOp::Text("\n"))?;
 
-                    on_empty_new_line = true;
+                    line_state = LineState::Empty;
                     pending_break_if_within_line = None;
                 }
 
                 LineOp::BreakIfWithinLine(br) => {
-                    if !on_empty_new_line {
+                    let elide = match line_state {
+                        LineState::Empty => true,
+                        LineState::OnlyIndentedOrStyled { indent_so_far } => {
+                            indent_so_far <= indent
+                        }
+                        LineState::HasText => false,
+                    };
+                    if !elide {
                         // Merge two pending `Break`s if necessary,
                         // preferring newlines over spaces.
                         let br = match (pending_break_if_within_line, br) {

From d2d9cbd06126a78743926fde7cf462ee363fc0be Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu 
Date: Fri, 31 Mar 2023 15:33:00 +0300
Subject: [PATCH 08/11] print/multiversion: desaturate/dim unchanged lines and
 remove `colspan` usage.

---
 src/print/mod.rs          |   8 +-
 src/print/multiversion.rs | 182 ++++++++++++++++++++++++++++++--------
 src/print/pretty.rs       |  11 ++-
 3 files changed, 157 insertions(+), 44 deletions(-)

diff --git a/src/print/mod.rs b/src/print/mod.rs
index 5749acc..11d77a0 100644
--- a/src/print/mod.rs
+++ b/src/print/mod.rs
@@ -465,6 +465,9 @@ impl Visit for AllCxInterned {
     fn visit_with<'a>(&'a self, _visitor: &mut impl Visitor<'a>) {}
 }
 
+// FIXME(eddyb) make max line width configurable.
+const MAX_LINE_WIDTH: usize = 120;
+
 impl Plan<'_> {
     #[allow(rustdoc::private_intra_doc_links)]
     /// Print the whole [`Plan`] to a [`Versions`] and perform
@@ -474,11 +477,8 @@ impl Plan<'_> {
     /// [`fmt::Display`] for convenience, but also more specific methods
     /// (e.g. HTML output).
     pub fn pretty_print(&self) -> Versions {
-        // FIXME(eddyb) make max line width configurable.
-        let max_line_width = 120;
-
         self.print(&Printer::new(self))
-            .map_pretty_fragments(|fragment| fragment.layout_with_max_line_width(max_line_width))
+            .map_pretty_fragments(|fragment| fragment.layout_with_max_line_width(MAX_LINE_WIDTH))
     }
 }
 
diff --git a/src/print/multiversion.rs b/src/print/multiversion.rs
index 2b2d70d..982c83a 100644
--- a/src/print/multiversion.rs
+++ b/src/print/multiversion.rs
@@ -5,10 +5,10 @@ use crate::FxIndexMap;
 use internal_iterator::{
     FromInternalIterator, InternalIterator, IntoInternalIterator, IteratorExt,
 };
-use itertools::Itertools;
+use itertools::{Either, Itertools};
 use smallvec::SmallVec;
 use std::fmt::Write;
-use std::{fmt, mem};
+use std::{fmt, iter, mem};
 
 #[allow(rustdoc::private_intra_doc_links)]
 /// Wrapper for handling the difference between single-version and multi-version
@@ -27,6 +27,8 @@ pub enum Versions {
         /// multiple versions (columns) have the exact same content.
         ///
         /// For HTML output, "repeat count"s map to `colspan` attributes.
+        //
+        // FIXME(eddyb) remove the "repeat count" mechanism.
         per_row_versions_with_repeat_count: Vec>,
     },
 }
@@ -105,8 +107,6 @@ impl Versions {
                     "
 
         "
-                    .replace("SCOPE", &format!("table.{TABLE_CLASS_NAME}")),
+                    .replace("SCOPE", &format!("table.{TABLE_CLASS_NAME}"))
+                    .replace("MAX_LINE_WIDTH", &super::MAX_LINE_WIDTH.to_string()),
                 );
 
                 let headings = {
@@ -144,6 +140,7 @@ impl Versions {
                 html.body = format!("\n");
                 let mut last_was_uniform = true;
                 for versions_with_repeat_count in per_row_versions_with_repeat_count {
+                    // FIXME(eddyb) remove the "repeat count" mechanism.
                     let is_uniform = match versions_with_repeat_count[..] {
                         [(_, repeat_count)] => repeat_count == version_names.len(),
                         _ => false,
@@ -167,20 +164,115 @@ impl Versions {
                     }
 
                     html.body += "\n";
-                    for ((_, repeat_count), column) in versions_with_repeat_count
-                        .iter()
-                        .zip(anchor_aligner.merged_columns())
-                    {
-                        writeln!(html.body, "\n";
+                    } else {
+                        let mut merged_columns = versions_with_repeat_count
+                            .iter()
+                            .zip(anchor_aligner.merged_columns())
+                            .flat_map(|(&(_, repeat_count), column)| {
+                                iter::repeat(column).take(repeat_count)
+                            })
+                            .peekable();
+
+                        let mut prev_column = None;
+                        while let Some(column) = merged_columns.next() {
+                            let prev_column = prev_column.replace(column);
+                            let next_column = merged_columns.peek().copied();
+
+                            let unchanged_line_style = pretty::Styles {
+                                desaturate_and_dim_for_unchanged_multiversion_line: true,
+                                ..Default::default()
+                            };
+
+                            // NOTE(eddyb) infinite (but limited by `zip` below),
+                            // and `Some([])`/`None` distinguishes empty/missing.
+                            let prev_lines = prev_column
+                                .iter()
+                                .flat_map(|prev| prev.lines().map(Some))
+                                .chain(iter::repeat(prev_column.map(|_| &[][..])));
+                            let next_lines = next_column
+                                .iter()
+                                .flat_map(|next| next.lines().map(Some))
+                                .chain(iter::repeat(next_column.map(|_| &[][..])));
+
+                            let lines = column.lines().zip(prev_lines).zip(next_lines).map(
+                                |((line, prev_line), next_line)| {
+                                    // FIXME(eddyb) apply a `class` instead of an inline `style`,
+                                    // and allow `:hover` to disable the desaturation/dimming.
+                                    // FIXME(eddyb) maybe indicate when lines
+                                    // were removed (red "hashed" background?).
+                                    let diff = |other: Option<_>| {
+                                        // Ignore indendation-only changes.
+                                        fn strip_indents<'a, 'b>(
+                                            mut line: &'b [TextOp<'a>],
+                                        ) -> &'b [TextOp<'a>]
+                                        {
+                                            // HACK(eddyb) also ignore helper anchors,
+                                            // which can go before indents.
+                                            while let [TextOp::Text(pretty::INDENT), rest @ ..]
+                                            | [
+                                                TextOp::PushStyles(_),
+                                                TextOp::PopStyles(_),
+                                                rest @ ..,
+                                            ] = line
+                                            {
+                                                line = rest;
+                                            }
+                                            line
+                                        }
+                                        other.map_or(false, |other| {
+                                            strip_indents(line) != strip_indents(other)
+                                        })
+                                    };
+                                    let line_style = if !diff(prev_line) && !diff(next_line) {
+                                        Some(&unchanged_line_style)
+                                    } else {
+                                        None
+                                    };
+                                    line_style
+                                        .map(TextOp::PushStyles)
+                                        .into_iter()
+                                        .chain(line.iter().copied())
+                                        .chain(line_style.map(TextOp::PopStyles))
+                                },
+                            );
+
+                            let pretty::HtmlSnippet {
+                                head_deduplicatable_elements: fragment_head,
+                                body: fragment_body,
+                            } = lines
+                                .map(Either::Left)
+                                .intersperse(Either::Right([TextOp::Text("\n")].into_iter()))
+                                .flatten()
+                                .into_internal()
+                                .collect();
+
+                            html.head_deduplicatable_elements.extend(fragment_head);
+
+                            html.body += "\n";
+                        }
                     }
                     html.body += "\n";
                 }
@@ -293,32 +385,46 @@ impl<'a> FromInternalIterator> for AAColumn<'a> {
     }
 }
 
+#[derive(Copy, Clone)]
+struct AAMergedColumn<'a, 'b> {
+    original_column: &'b AAColumn<'a>,
+    column_idx: usize,
+    merged_lines: &'b [AAMergedLine],
+}
+
+impl<'a, 'b> AAMergedColumn<'a, 'b> {
+    fn lines(&self) -> impl Iterator]> + '_ {
+        let column_idx = self.column_idx;
+        let line_lengths = self
+            .merged_lines
+            .iter()
+            .map(move |line| line.per_column_line_lengths[column_idx]);
+        self.original_column.lines(line_lengths)
+    }
+}
+
 impl<'a> AnchorAligner<'a> {
     /// Flatten all columns to `TextOp`s (including line separators).
-    fn merged_columns(&self) -> impl Iterator> + '_> {
+    fn merged_columns(&self) -> impl Iterator> {
         self.original_columns
             .iter()
             .enumerate()
-            .map(|(column_idx, column)| {
-                let line_lengths = self
-                    .merged_lines
-                    .iter()
-                    .map(move |line| line.per_column_line_lengths[column_idx]);
-
-                // HACK(eddyb) trim all trailing empty lines (which is done on
-                // a `peekable` of the reverse, followed by reversing *again*,
-                // equivalent to a hypothetical `peekable_back` and no reversing).
-                let mut rev_line_lengths = line_lengths.rev().peekable();
-                while rev_line_lengths.peek() == Some(&0) {
-                    rev_line_lengths.next().unwrap();
+            .map(|(column_idx, original_column)| {
+                let mut merged_lines = &self.merged_lines[..];
+
+                // Trim all trailing lines that are empty in this column.
+                while let Some((last, before_last)) = merged_lines.split_last() {
+                    if last.per_column_line_lengths[column_idx] > 0 {
+                        break;
+                    }
+                    merged_lines = before_last;
                 }
-                let line_lengths = rev_line_lengths.rev();
 
-                column
-                    .lines(line_lengths)
-                    .intersperse(&[TextOp::Text("\n")])
-                    .flatten()
-                    .copied()
+                AAMergedColumn {
+                    original_column,
+                    column_idx,
+                    merged_lines,
+                }
             })
     }
 
diff --git a/src/print/pretty.rs b/src/print/pretty.rs
index 0131995..349fa3a 100644
--- a/src/print/pretty.rs
+++ b/src/print/pretty.rs
@@ -80,6 +80,9 @@ pub struct Styles {
 
     pub subscript: bool,
     pub superscript: bool,
+
+    // FIXME(eddyb) maybe a more general `filter` system would be better?
+    pub desaturate_and_dim_for_unchanged_multiversion_line: bool,
 }
 
 impl Styles {
@@ -352,6 +355,7 @@ impl<'a> FromInternalIterator> for HtmlSnippet {
                         size,
                         subscript: _,
                         superscript: _,
+                        desaturate_and_dim_for_unchanged_multiversion_line,
                     } = *styles;
 
                     if let Some(id) = anchor {
@@ -376,6 +380,9 @@ impl<'a> FromInternalIterator> for HtmlSnippet {
                     if let Some(size) = size {
                         write!(css_style, "font-size:{}em;", 1.0 + (size as f64) * 0.1).unwrap();
                     }
+                    if desaturate_and_dim_for_unchanged_multiversion_line {
+                        write!(css_style, "filter:saturate(0.3)opacity(0.5);").unwrap();
+                    }
                     if !css_style.is_empty() {
                         push_attr("style", &css_style);
                     }
@@ -487,7 +494,7 @@ struct MaxWidths {
 }
 
 // FIXME(eddyb) make this configurable.
-const INDENT: &str = "  ";
+pub(super) const INDENT: &str = "  ";
 
 impl Node {
     /// Determine the "rigid" component of the [`ApproxLayout`] of this [`Node`].
@@ -837,7 +844,7 @@ impl Fragment {
 }
 
 /// Text-oriented operation (plain text snippets interleaved with style push/pop).
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq)]
 pub(super) enum TextOp<'a> {
     PushStyles(&'a Styles),
     PopStyles(&'a Styles),

From 65c67b713d3eb2bbb364ef7a8bb2cd12e93a0846 Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu 
Date: Fri, 31 Mar 2023 15:33:21 +0300
Subject: [PATCH 09/11] print: reduce the max line width to 100 (from 120).

---
 src/print/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/print/mod.rs b/src/print/mod.rs
index 11d77a0..fab8175 100644
--- a/src/print/mod.rs
+++ b/src/print/mod.rs
@@ -466,7 +466,7 @@ impl Visit for AllCxInterned {
 }
 
 // FIXME(eddyb) make max line width configurable.
-const MAX_LINE_WIDTH: usize = 120;
+const MAX_LINE_WIDTH: usize = 100;
 
 impl Plan<'_> {
     #[allow(rustdoc::private_intra_doc_links)]

From 8af09b9465cff6407e54c393ae6523f2d902042e Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu 
Date: Fri, 31 Mar 2023 15:34:41 +0300
Subject: [PATCH 10/11] cfg: reverse targets ordering for RPO (so that the
 final RPO matches the IR).

---
 src/cfg.rs | 96 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 55 insertions(+), 41 deletions(-)

diff --git a/src/cfg.rs b/src/cfg.rs
index ade72b2..a0687d9 100644
--- a/src/cfg.rs
+++ b/src/cfg.rs
@@ -78,16 +78,20 @@ impl ControlFlowGraph {
         func_def_body: &FuncDefBody,
     ) -> impl DoubleEndedIterator {
         let mut post_order = SmallVec::<[_; 8]>::new();
-        {
-            let mut incoming_edge_counts = EntityOrientedDenseMap::new();
-            self.traverse_whole_func(
-                func_def_body,
-                &mut incoming_edge_counts,
-                &mut |_| {},
-                &mut |region| post_order.push(region),
-            );
-        }
+        self.traverse_whole_func(
+            func_def_body,
+            &mut TraversalState {
+                incoming_edge_counts: EntityOrientedDenseMap::new(),
+
+                pre_order_visit: |_| {},
+                post_order_visit: |region| post_order.push(region),
 
+                // NOTE(eddyb) this doesn't impact semantics, but combined with
+                // the final reversal, it should keep targets in the original
+                // order in the cases when they didn't get deduplicated.
+                reverse_targets: true,
+            },
+        );
         post_order.into_iter().rev()
     }
 }
@@ -120,15 +124,23 @@ mod sealed {
         }
     }
 }
+use itertools::Either;
 use sealed::IncomingEdgeCount;
 
+struct TraversalState {
+    incoming_edge_counts: EntityOrientedDenseMap,
+    pre_order_visit: PreVisit,
+    post_order_visit: PostVisit,
+
+    // FIXME(eddyb) should this be a generic parameter for "targets iterator"?
+    reverse_targets: bool,
+}
+
 impl ControlFlowGraph {
     fn traverse_whole_func(
         &self,
         func_def_body: &FuncDefBody,
-        incoming_edge_counts: &mut EntityOrientedDenseMap,
-        pre_order_visit: &mut impl FnMut(ControlRegion),
-        post_order_visit: &mut impl FnMut(ControlRegion),
+        state: &mut TraversalState,
     ) {
         let func_at_body = func_def_body.at_body();
 
@@ -139,47 +151,43 @@ impl ControlFlowGraph {
         ));
         assert!(func_at_body.def().outputs.is_empty());
 
-        self.traverse(
-            func_at_body,
-            incoming_edge_counts,
-            pre_order_visit,
-            post_order_visit,
-        );
+        self.traverse(func_at_body, state);
     }
 
     fn traverse(
         &self,
         func_at_region: FuncAt<'_, ControlRegion>,
-        incoming_edge_counts: &mut EntityOrientedDenseMap,
-        pre_order_visit: &mut impl FnMut(ControlRegion),
-        post_order_visit: &mut impl FnMut(ControlRegion),
+        state: &mut TraversalState,
     ) {
         let region = func_at_region.position;
 
         // FIXME(eddyb) `EntityOrientedDenseMap` should have an `entry` API.
-        if let Some(existing_count) = incoming_edge_counts.get_mut(region) {
+        if let Some(existing_count) = state.incoming_edge_counts.get_mut(region) {
             *existing_count += IncomingEdgeCount::ONE;
             return;
         }
-        incoming_edge_counts.insert(region, IncomingEdgeCount::ONE);
+        state
+            .incoming_edge_counts
+            .insert(region, IncomingEdgeCount::ONE);
 
-        pre_order_visit(region);
+        (state.pre_order_visit)(region);
 
         let control_inst = self
             .control_inst_on_exit_from
             .get(region)
             .expect("cfg: missing `ControlInst`, despite having left structured control-flow");
 
-        for &target in &control_inst.targets {
-            self.traverse(
-                func_at_region.at(target),
-                incoming_edge_counts,
-                pre_order_visit,
-                post_order_visit,
-            );
+        let targets = control_inst.targets.iter().copied();
+        let targets = if state.reverse_targets {
+            Either::Left(targets.rev())
+        } else {
+            Either::Right(targets)
+        };
+        for target in targets {
+            self.traverse(func_at_region.at(target), state);
         }
 
-        post_order_visit(region);
+        (state.post_order_visit)(region);
     }
 }
 
@@ -381,15 +389,21 @@ impl<'a> Structurizer<'a> {
             ctor_args: [].into_iter().collect(),
         });
 
-        let mut incoming_edge_counts = EntityOrientedDenseMap::new();
-        if let Some(cfg) = &func_def_body.unstructured_cfg {
-            cfg.traverse_whole_func(
-                func_def_body,
-                &mut incoming_edge_counts,
-                &mut |_| {},
-                &mut |_| {},
-            );
-        }
+        let incoming_edge_counts = func_def_body
+            .unstructured_cfg
+            .as_ref()
+            .map(|cfg| {
+                let mut state = TraversalState {
+                    incoming_edge_counts: EntityOrientedDenseMap::new(),
+
+                    pre_order_visit: |_| {},
+                    post_order_visit: |_| {},
+                    reverse_targets: false,
+                };
+                cfg.traverse_whole_func(func_def_body, &mut state);
+                state.incoming_edge_counts
+            })
+            .unwrap_or_default();
 
         Self {
             cx,

From eaf74764c898d65744b3c1d5d0a0f733c4bedcc2 Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu 
Date: Fri, 31 Mar 2023 16:20:30 +0300
Subject: [PATCH 11/11] Update CHANGELOG.

---
 CHANGELOG.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0c17360..da6e839 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,6 +33,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased] - ReleaseDate
 
+### Added ⭐
+- [PR#18](https://github.com/EmbarkStudios/spirt/pull/18) added anchor-based alignment
+  to multi-version pretty-printing output (the same definitions will be kept on
+  the same lines in all columns, wherever possible, to improve readability)
+
 ### Changed 🛠
 - [PR#26](https://github.com/EmbarkStudios/spirt/pull/26) allowed using `OpEmitMeshTasksEXT` as a terminator (by hardcoding it as `Control-Flow`)
 - [PR#25](https://github.com/EmbarkStudios/spirt/pull/25) updated SPIRV-headers from 1.5.4 to 1.6.1

").unwrap();
-
+                    if is_uniform {
+                        // FIXME(eddyb) avoid duplication with the non-uniform case.
                         let pretty::HtmlSnippet {
                             head_deduplicatable_elements: fragment_head,
                             body: fragment_body,
-                        } = column.into_internal().collect();
+                        } = anchor_aligner
+                            .merged_columns()
+                            .next()
+                            .unwrap()
+                            .lines()
+                            .intersperse(&[TextOp::Text("\n")])
+                            .flatten()
+                            .copied()
+                            .into_internal()
+                            .collect();
+
                         html.head_deduplicatable_elements.extend(fragment_head);
-                        html.body += &fragment_body;
 
+                        writeln!(html.body, "", version_names.len()).unwrap();
+                        html.body += &fragment_body;
                         html.body += "\n";
+                            html.body += &fragment_body;
+                            html.body += "