Pretty diff expressions

Author: Nadrieril

src/ast/printer.rs

@@ -156,6 +156,73 @@ impl<'d> ToDisplayTree<'d> for Type<'_> {
     }
 }
 
+impl<'d> ToDisplayTree<'d> for ExprKind<'_> {
+    fn to_display_tree(&self, a: &'d Arenas<'d>) -> DisplayTree<'d> {
+        enum Symbol {
+            Deref,
+            Ref(Mutability),
+        }
+        /// Remove the refs and derefs in front of this expression.
+        fn strip_symbols<'a>(e: &'a ExprKind<'a>, symbols: &mut Vec<Symbol>) -> &'a ExprKind<'a> {
+            match e {
+                ExprKind::Ref(mtbl, e) => {
+                    symbols.push(Symbol::Ref(*mtbl));
+                    strip_symbols(&e.kind, symbols)
+                }
+                ExprKind::Deref(e) => {
+                    symbols.push(Symbol::Deref);
+                    strip_symbols(&e.kind, symbols)
+                }
+                _ => &e,
+            }
+        }
+        let mut symbols = Vec::new();
+        let e = strip_symbols(self, &mut symbols);
+        let leaf = match e {
+            ExprKind::Scrutinee => "s".to_display_tree(a),
+            ExprKind::Abstract { .. } => "e".to_display_tree(a),
+            ExprKind::Field(e, n) => {
+                let needs_parens = matches!(e.kind, ExprKind::Deref(..));
+                let (before, after) = if needs_parens { ("(", ")") } else { ("", "") };
+                DisplayTree::sep_by(
+                    a,
+                    "",
+                    [
+                        before.to_display_tree(a),
+                        e.to_display_tree(a),
+                        after.to_display_tree(a),
+                        ".".to_display_tree(a),
+                        format!("{n}").to_display_tree(a),
+                    ],
+                )
+            }
+            ExprKind::Ref(..) | ExprKind::Deref(..) => unreachable!(),
+        };
+        // We cleverly diff expressions: expressions tend to start the same then diverge; so we
+        // want to show that the innermost expressions are the same and the surrounding `&`/`*`
+        // differ. To do this, we extract the list of `&`/`*` and add them to the same list, with
+        // `Suffix` compare mode.
+        DisplayTree::sep_by_compare_mode(
+            a,
+            "",
+            symbols
+                .iter()
+                .map(|s| match s {
+                    Symbol::Deref => "*".to_display_tree(a),
+                    Symbol::Ref(mutable) => format!("&{mutable}").to_display_tree(a),
+                })
+                .chain([leaf]),
+            CompareMode::Suffix,
+        )
+    }
+}
+
+impl<'d> ToDisplayTree<'d> for Expression<'_> {
+    fn to_display_tree(&self, a: &'d Arenas<'d>) -> DisplayTree<'d> {
+        self.kind.to_display_tree(a)
+    }
+}
+
 impl<'d> ToDisplayTree<'d> for BindingAssignments<'_> {
     fn to_display_tree(&self, a: &'d Arenas<'d>) -> DisplayTree<'d> {
         DisplayTree::sep_by(
@@ -227,7 +294,7 @@ impl<'a> TypingPredicate<'a> {
                 .pat
                 .to_display_tree(a)
                 .sep_then(a, ": ", self.expr.ty)
-                .sep_then(a, " = ", self.expr.to_string())
+                .sep_then(a, " = ", self.expr)
                 .preceded(a, "let "),
             PredicateStyle::Sequent {
                 ty: toi,
@@ -385,26 +452,15 @@ impl Display for Type<'_> {
 
 impl Display for Expression<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.kind)
+        let a = &Arenas::default();
+        write!(f, "{}", self.to_display_tree(a))
     }
 }
 
 impl Display for ExprKind<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            ExprKind::Scrutinee => write!(f, "s"),
-            ExprKind::Abstract { .. } => write!(f, "e"),
-            ExprKind::Ref(mutable, e) => write!(f, "&{mutable}{e}"),
-            ExprKind::Deref(e) => write!(f, "*{e}"),
-            ExprKind::Field(e, n) => {
-                let needs_parens = matches!(e.kind, ExprKind::Deref(..));
-                if needs_parens {
-                    write!(f, "({e}).{n}")
-                } else {
-                    write!(f, "{e}.{n}")
-                }
-            }
-        }
+        let a = &Arenas::default();
+        write!(f, "{}", self.to_display_tree(a))
     }
 }
 

src/ast/printer/display_tree.rs

@@ -5,6 +5,17 @@ use itertools::Itertools;
 use crate::*;
 use DisplayTreeKind::*;
 
+/// How to compare two separated lists of different lengths.
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub enum CompareMode {
+    /// We consider the whole subtree to differ.
+    None,
+    /// The first elements are diffed together until exhaustion.
+    Prefix,
+    /// The last elements are diffed together until exhaustion.
+    Suffix,
+}
+
 /// A structured representation of a string to be displayed. Used to compute structured diffs.
 #[derive(Clone, Copy)]
 enum DisplayTreeKind<'a> {
@@ -17,10 +28,8 @@ enum DisplayTreeKind<'a> {
         sep: &'a str,
         /// The children.
         children: &'a [DisplayTree<'a>],
-        /// If the lengths differ and this is `true`, the first elements are diffed together until
-        /// exhaustion. If the lengths differ and this is `false`, we consider the whole subtree to
-        /// differ.
-        compare_common_prefix: bool,
+        /// How to compare elements if the lengths differ.
+        compare_mode: CompareMode,
     },
 }
 
@@ -119,11 +128,11 @@ impl<'a> DisplayTree<'a> {
         Self::leaf_noalloc(a.alloc_str(s))
     }
 
-    fn mk_separated(
+    pub fn sep_by_compare_mode(
         a: &'a Arenas<'a>,
         sep: &str,
         children: impl IntoIterator<Item: ToDisplayTree<'a>>,
-        compare_common_prefix: bool,
+        compare_mode: CompareMode,
     ) -> Self {
         let children = children
             .into_iter()
@@ -132,7 +141,7 @@ impl<'a> DisplayTree<'a> {
         Self::new_from_kind(Separated {
             sep: a.alloc_str(sep),
             children: a.bump.alloc_slice_copy(&children),
-            compare_common_prefix,
+            compare_mode,
         })
     }
 
@@ -141,15 +150,15 @@ impl<'a> DisplayTree<'a> {
         sep: &str,
         children: impl IntoIterator<Item: ToDisplayTree<'a>>,
     ) -> Self {
-        Self::mk_separated(a, sep, children, false)
+        Self::sep_by_compare_mode(a, sep, children, CompareMode::None)
     }
 
     pub fn sep_by_compare_prefix(
         a: &'a Arenas<'a>,
         sep: &str,
         children: impl IntoIterator<Item: ToDisplayTree<'a>>,
     ) -> Self {
-        Self::mk_separated(a, sep, children, true)
+        Self::sep_by_compare_mode(a, sep, children, CompareMode::Prefix)
     }
 
     /// Concatenates `self` and `x`, separated by `sep`.
@@ -235,26 +244,44 @@ impl<'a> DisplayTree<'a> {
                 Separated {
                     sep,
                     children: c1,
-                    compare_common_prefix: ccp1,
+                    compare_mode: ccp1,
                 },
                 Separated {
                     sep: sep2,
                     children: c2,
-                    compare_common_prefix: ccp2,
+                    compare_mode: _, // we only look at one of tham
                 },
             ) if strip_markup(sep) == strip_markup(sep2)
-                && (c1.len() == c2.len() || ccp1 || ccp2) =>
+                && (c1.len() == c2.len() || ccp1 != CompareMode::None) =>
             {
+                use std::iter::repeat_n;
+                // Pad the two lists so they have the same length; pad at the start or end
+                // depending on how we want to match the lists up.
+                let len1 = c1.len();
+                let len2 = c2.len();
+                let len = std::cmp::max(len1, len2);
+                let (prefix1, prefix2, suffix1, suffix2) = match ccp1 {
+                    CompareMode::None => (0, 0, 0, 0),
+                    CompareMode::Prefix => (0, 0, len - len1, len - len2),
+                    CompareMode::Suffix => (len - len1, len - len2, 0, 0),
+                };
+                let c1 = repeat_n(None, prefix1)
+                    .chain(c1.iter().copied().map(Some))
+                    .chain(repeat_n(None, suffix1));
+                let c2 = repeat_n(None, prefix2)
+                    .chain(c2.iter().copied().map(Some))
+                    .chain(repeat_n(None, suffix2));
+
                 let mut is_first = true;
                 let mut any_diff = false;
-                for either_or_both in c1.iter().copied().zip_longest(c2.iter().copied()) {
-                    if !is_first && !either_or_both.is_right() {
+                for (l, r) in c1.zip(c2) {
+                    if !is_first && !l.is_none() {
                         write!(left, "{sep}")?;
                     }
-                    if !is_first && !either_or_both.is_left() {
+                    if !is_first && !r.is_none() {
                         write!(right, "{sep}")?;
                     }
-                    let (c1, c2) = either_or_both.or_default();
+                    let (c1, c2) = (l.unwrap_or_default(), r.unwrap_or_default());
                     any_diff |= c1.diff_display_inner(&c2, left, right)?;
                     is_first = false;
                 }