[StableMIR] A few fixes to pretty printing

Improve identation, and a few other rvalue printing

Author: celinval

compiler/stable_mir/src/mir/pretty.rs

@@ -1,13 +1,14 @@
+//! Implement methods to pretty print stable MIR body.
 use std::fmt::Debug;
 use std::io::Write;
 use std::{fmt, io, iter};
 
 use fmt::{Display, Formatter};
 
-use super::{AssertMessage, BinOp, BorrowKind, FakeBorrowKind, TerminatorKind};
+use super::{AggregateKind, AssertMessage, BinOp, BorrowKind, FakeBorrowKind, TerminatorKind};
 use crate::mir::{Operand, Place, Rvalue, StatementKind, UnwindAction, VarDebugInfoContents};
-use crate::ty::{IndexedVal, MirConst, Ty, TyConst};
-use crate::{Body, Mutability, with};
+use crate::ty::{AdtKind, IndexedVal, MirConst, Ty, TyConst};
+use crate::{Body, CrateDef, Mutability, with};
 
 impl Display for Ty {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
@@ -23,10 +24,11 @@ impl Debug for Place {
 
 pub(crate) fn function_body<W: Write>(writer: &mut W, body: &Body, name: &str) -> io::Result<()> {
     write!(writer, "fn {name}(")?;
-    body.arg_locals()
-        .iter()
-        .enumerate()
-        .try_for_each(|(index, local)| write!(writer, "_{}: {}", index + 1, local.ty))?;
+    let mut sep = "";
+    for (index, local) in body.arg_locals().iter().enumerate() {
+        write!(writer, "{}_{}: {}", sep, index + 1, local.ty)?;
+        sep = ", ";
+    }
     write!(writer, ")")?;
 
     let return_local = body.ret_local();
@@ -73,39 +75,40 @@ pub(crate) fn function_body<W: Write>(writer: &mut W, body: &Body, name: &str) -
 }
 
 fn pretty_statement<W: Write>(writer: &mut W, statement: &StatementKind) -> io::Result<()> {
+    const INDENT: &str = "        ";
     match statement {
         StatementKind::Assign(place, rval) => {
-            write!(writer, "        {place:?} = ")?;
+            write!(writer, "{INDENT}{place:?} = ")?;
             pretty_rvalue(writer, rval)?;
             writeln!(writer, ";")
         }
         // FIXME: Add rest of the statements
         StatementKind::FakeRead(cause, place) => {
-            writeln!(writer, "FakeRead({cause:?}, {place:?});")
+            writeln!(writer, "{INDENT}FakeRead({cause:?}, {place:?});")
         }
         StatementKind::SetDiscriminant { place, variant_index } => {
-            writeln!(writer, "discriminant({place:?} = {};", variant_index.to_index())
+            writeln!(writer, "{INDENT}discriminant({place:?} = {};", variant_index.to_index())
         }
         StatementKind::Deinit(place) => writeln!(writer, "Deinit({place:?};"),
         StatementKind::StorageLive(local) => {
-            writeln!(writer, "StorageLive(_{local});")
+            writeln!(writer, "{INDENT}StorageLive(_{local});")
         }
         StatementKind::StorageDead(local) => {
-            writeln!(writer, "StorageDead(_{local});")
+            writeln!(writer, "{INDENT}StorageDead(_{local});")
         }
         StatementKind::Retag(kind, place) => writeln!(writer, "Retag({kind:?}, {place:?});"),
         StatementKind::PlaceMention(place) => {
-            writeln!(writer, "PlaceMention({place:?};")
+            writeln!(writer, "{INDENT}PlaceMention({place:?};")
         }
         StatementKind::ConstEvalCounter => {
-            writeln!(writer, "ConstEvalCounter;")
+            writeln!(writer, "{INDENT}ConstEvalCounter;")
         }
-        StatementKind::Nop => writeln!(writer, "nop;"),
+        StatementKind::Nop => writeln!(writer, "{INDENT}nop;"),
         StatementKind::AscribeUserType { .. }
         | StatementKind::Coverage(_)
         | StatementKind::Intrinsic(_) => {
             // FIX-ME: Make them pretty.
-            writeln!(writer, "{statement:?};")
+            writeln!(writer, "{INDENT}{statement:?};")
         }
     }
 }
@@ -322,15 +325,11 @@ fn pretty_ty_const(ct: &TyConst) -> String {
 fn pretty_rvalue<W: Write>(writer: &mut W, rval: &Rvalue) -> io::Result<()> {
     match rval {
         Rvalue::AddressOf(mutability, place) => {
-            write!(writer, "&raw {}(*{:?})", pretty_mut(*mutability), place)
+            write!(writer, "&raw {} {:?}", pretty_mut(*mutability), place)
         }
         Rvalue::Aggregate(aggregate_kind, operands) => {
             // FIXME: Add pretty_aggregate function that returns a pretty string
-            write!(writer, "{aggregate_kind:?} (")?;
-            let mut op_iter = operands.iter();
-            op_iter.next().map_or(Ok(()), |op| write!(writer, "{}", pretty_operand(op)))?;
-            op_iter.try_for_each(|op| write!(writer, ", {}", pretty_operand(op)))?;
-            write!(writer, ")")
+            pretty_aggregate(writer, aggregate_kind, operands)
         }
         Rvalue::BinaryOp(bin, op1, op2) => {
             write!(writer, "{:?}({}, {})", bin, pretty_operand(op1), pretty_operand(op2))
@@ -360,22 +359,74 @@ fn pretty_rvalue<W: Write>(writer: &mut W, rval: &Rvalue) -> io::Result<()> {
             write!(writer, "{kind}{place:?}")
         }
         Rvalue::Repeat(op, cnst) => {
-            write!(writer, "{} \" \" {}", pretty_operand(op), pretty_ty_const(cnst))
+            write!(writer, "[{}; {}]", pretty_operand(op), pretty_ty_const(cnst))
         }
         Rvalue::ShallowInitBox(_, _) => Ok(()),
         Rvalue::ThreadLocalRef(item) => {
             write!(writer, "thread_local_ref{item:?}")
         }
         Rvalue::NullaryOp(nul, ty) => {
-            write!(writer, "{nul:?} {ty} \" \"")
+            write!(writer, "{nul:?}::<{ty}>() \" \"")
         }
         Rvalue::UnaryOp(un, op) => {
-            write!(writer, "{} \" \" {:?}", pretty_operand(op), un)
+            write!(writer, "{:?}({})", un, pretty_operand(op))
         }
         Rvalue::Use(op) => write!(writer, "{}", pretty_operand(op)),
     }
 }
 
+fn pretty_aggregate<W: Write>(
+    writer: &mut W,
+    aggregate_kind: &AggregateKind,
+    operands: &Vec<Operand>,
+) -> io::Result<()> {
+    let suffix = match aggregate_kind {
+        AggregateKind::Array(_) => {
+            write!(writer, "[")?;
+            "]"
+        }
+        AggregateKind::Tuple => {
+            write!(writer, "(")?;
+            ")"
+        }
+        AggregateKind::Adt(def, var, _, _, _) => {
+            if def.kind() == AdtKind::Enum {
+                write!(writer, "{}::{}", def.name(), def.variant(*var).unwrap().name())?;
+            } else {
+                write!(writer, "{}", def.variant(*var).unwrap().name())?;
+            }
+            if operands.is_empty() {
+                return Ok(());
+            }
+            // FIXME: Change this once we have CtorKind in StableMIR.
+            write!(writer, "(")?;
+            ")"
+        }
+        AggregateKind::Closure(def, _) => {
+            write!(writer, "{{closure@{}}}(", def.span().diagnostic())?;
+            ")"
+        }
+        AggregateKind::Coroutine(def, _, _) => {
+            write!(writer, "{{coroutine@{}}}(", def.span().diagnostic())?;
+            ")"
+        }
+        AggregateKind::RawPtr(ty, mutability) => {
+            write!(
+                writer,
+                "*{} {ty} from (",
+                if *mutability == Mutability::Mut { "mut" } else { "const" }
+            )?;
+            ")"
+        }
+    };
+    let mut separator = "";
+    for op in operands {
+        write!(writer, "{}{}", separator, pretty_operand(op))?;
+        separator = ", ";
+    }
+    write!(writer, "{suffix}")
+}
+
 fn pretty_mut(mutability: Mutability) -> &'static str {
     match mutability {
         Mutability::Not => " ",

compiler/stable_mir/src/ty.rs

@@ -271,6 +271,14 @@ impl Span {
     pub fn get_lines(&self) -> LineInfo {
         with(|c| c.get_lines(self))
     }
+
+    /// Return the span location to be printed in diagnostic messages.
+    ///
+    /// This may leak local file paths and should not be used to build artifacts that may be
+    /// distributed.
+    pub fn diagnostic(&self) -> String {
+        with(|c| c.span_to_string(*self))
+    }
 }
 
 #[derive(Clone, Copy, Debug, Serialize)]

tests/ui/stable-mir-print/operands.rs

@@ -0,0 +1,48 @@
+//@ compile-flags: -Z unpretty=stable-mir --crate-type lib -C panic=abort
+//@ check-pass
+//@ only-x86_64
+//@ needs-unwind unwind edges are different with panic=abort
+//! Check how stable mir pretty printer prints different operands and abort strategy.
+
+pub fn operands(val: u8) {
+    let array = [val; 10];
+    let first = array[0];
+    let last = array[10 - 1];
+    assert_eq!(first, last);
+
+    let reference = &first;
+    let dereferenced = *reference;
+    assert_eq!(dereferenced, first);
+
+    let tuple = (first, last);
+    let (first_again, _) = tuple;
+    let first_again_again = tuple.0;
+    assert_eq!(first_again, first_again_again);
+
+    let length = array.len();
+    let size_of = std::mem::size_of_val(&length);
+    assert_eq!(length, size_of);
+}
+
+pub struct Dummy {
+    c: char,
+    i: i32,
+}
+
+pub enum Ctors {
+    Unit,
+    StructLike { d: Dummy },
+    TupLike(bool),
+}
+
+pub fn more_operands() -> [Ctors; 3] {
+    let dummy = Dummy { c: 'a', i: i32::MIN };
+    let unit = Ctors::Unit;
+    let struct_like = Ctors::StructLike { d: dummy };
+    let tup_like = Ctors::TupLike(false);
+    [unit, struct_like, tup_like]
+}
+
+pub fn closures(x: bool, z: bool) -> impl FnOnce(bool) -> bool {
+    move |y: bool| (x ^ y) || z
+}