diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 345dd65..d46a369 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -29,7 +29,7 @@ jobs: with: toolchain: nightly-2024-11-29 # Hardcoded version, same as is in the build.rs - - name: 'Build smir_pretty' # rustfmt documentation claims it is unstable on code that doesn't build + - name: 'Build stable-mir-json' # rustfmt documentation claims it is unstable on code that doesn't build run: | cargo build -vv @@ -60,7 +60,7 @@ jobs: with: toolchain: nightly-2024-11-29 # Hardcoded version, same as is in the build.rs - - name: 'Build smir_pretty' + - name: 'Build stable-mir-json' run: | # Warning check should be redundant since code-quality runs first RUSTFLAGS='--deny warnings' cargo build -vv diff --git a/Cargo.lock b/Cargo.lock index a0fda01..ac721b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -39,7 +39,7 @@ dependencies = [ ] [[package]] -name = "smir_pretty" +name = "stable_mir_pretty" version = "0.1.0" dependencies = [ "dot-writer", @@ -48,9 +48,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.96" +version = "2.0.98" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d5d0adab1ae378d7f53bdebc67a39f1f151407ef230f0ce2883572f5d8985c80" +checksum = "36147f1a48ae0ec2b5b3bc5b537d267457555a10dc06f3dbc8cb11ba3006d3b1" dependencies = [ "proc-macro2", "quote", @@ -90,6 +90,6 @@ dependencies = [ [[package]] name = "unicode-ident" -version = "1.0.14" +version = "1.0.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "adb9e6ca4f869e1180728b7950e35922a7fc6397f7b641499e8f3ef06e50dc83" +checksum = "a210d160f08b701c8721ba1c726c11662f877ea6b7094007e1ca9a1041945034" diff --git a/Cargo.toml b/Cargo.toml index ac52992..088f98d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "smir_pretty" +name = "stable_mir_pretty" version = "0.1.0" edition = "2021" diff --git a/README.md b/README.md index 684c938..7a5b338 100644 --- a/README.md +++ b/README.md @@ -42,38 +42,25 @@ To ensure code quality, all code is required to pass `cargo clippy` and `cargo f ## Tests -### Running the Tests - -To run the tests, do the following: - -```shell -make generate_ui_tests -``` +Integration tests for `stable-mir-pretty` consist of compiling a number of (small) +programs with the wrapper compiler, and checking the output against expected JSON +data ("golden" tests). -This will generate four outputs: +The tests are stored [in `src/tests/integration/programs`](./src/tests/integration/programs). -| Path | Comment | -| --- | --- | -| `deps/rust/tests/ui/upstream` | Upstream `rustc` test outputs | -| `deps/rust/tests_ui_upstream.log` | Upstream test log | -| `deps/rust/tests/ui/smir` | `smir_pretty` test outputs (including `.smir.json` files) | -| `deps/rust/tests_ui_smir.log` | `smir_pretty` test log | +To compensate for any non-determinism in the output, the JSON file is first processed +to sort the array contents and remove data which changes with dependencies (such as +the crate hash suffix in the symbol names). -### Test Rationale +The JSON post-processing is performed [with `jq` using the script in `src/tests/integration/normalise-filter.jq`](./src/tests/integration/normalise-filter.jq). -Since this crate is a Stable MIR serialization tool, there are two main features we are interested in: +Some tests have non-deterministic output and are therefore expected to fail. +These tests are stored [in `src/tests/integration/failing`](./src/tests/integration/failing). -1. the serialization facilities should be stable (i.e. not crash) -2. the serialized output should be correct - -Since this tool is currently in its early stages, it is hard to test (2). -However, to test (1) and to make progress towards (2), we currently do the following: - -1. in the rustc test suite, we gather all of the run-pass tests, i.e., tests where the compiler is able to generate a binary _and_ subsequently execute the binary such that it exits successfully -2. we extract the test runner invocation from the `x.py test` command -3. we execute the test runner with upstream `rustc` against the test inputs from (1) --- this gives us a baseline on which tests should pass/fail -4. we re-execute the test runner but use our wrapper binary against the test inputs from (1) --- this generates the corresponding `.smir.json` files and shows us where any regressions occur +### Running the Tests +To run the tests, do the following: -**NOTE:** In order to speed up test time, we setup the test runner, by default, such that it skips codegen and compiler-generated binary execution. -**NOTE:** Points (1,4) also means that our test _outputs_ from this phase can become test _inputs_ for KMIR. +```shell +make integration-test +``` diff --git a/src/main.rs b/src/main.rs index ea72d5c..1cd2e11 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,7 +4,7 @@ pub mod driver; pub mod printer; use driver::stable_mir_driver; use printer::emit_smir; -use smir_pretty::mk_graph::emit_dotfile; +use stable_mir_pretty::mk_graph::emit_dotfile; fn main() { let mut args: Vec = env::args().collect(); diff --git a/src/mk_graph.rs b/src/mk_graph.rs index 2c57e3e..cbc4425 100644 --- a/src/mk_graph.rs +++ b/src/mk_graph.rs @@ -5,7 +5,7 @@ use std::{ io::{self, Write}, }; -use dot_writer::{Attributes, Color, DotWriter, Scope, Style}; +use dot_writer::{Attributes, Color, DotWriter, Scope, Shape, Style}; extern crate rustc_middle; use rustc_middle::ty::TyCtxt; @@ -14,8 +14,11 @@ extern crate stable_mir; use rustc_session::config::{OutFileName, OutputType}; extern crate rustc_session; -use stable_mir::mir::{BasicBlock, ConstOperand, Operand, Place, TerminatorKind, UnwindAction}; -use stable_mir::ty::Ty; +use stable_mir::mir::{ + AggregateKind, BasicBlock, ConstOperand, Mutability, NonDivergingIntrinsic, NullOp, Operand, + Place, Rvalue, Statement, StatementKind, TerminatorKind, UnwindAction, +}; +use stable_mir::ty::{IndexedVal, Ty}; use crate::{ printer::{collect_smir, FnSymType, SmirJson}, @@ -51,6 +54,7 @@ impl SmirJson<'_> { let mut graph = writer.digraph(); graph.set_label(&self.name[..]); + graph.node_attributes().set_shape(Shape::Rectangle); let func_map: HashMap = self .functions @@ -76,8 +80,10 @@ impl SmirJson<'_> { MonoItemKind::MonoItemFn { name, body, id: _ } => { let mut c = graph.cluster(); c.set_label(&name_lines(&name)); + c.set_style(Style::Filled); if is_unqualified(&name) { - c.set_style(Style::Filled); + c.set_color(Color::PaleGreen); + } else { c.set_color(Color::LightGrey); } @@ -86,19 +92,19 @@ impl SmirJson<'_> { |cluster: &mut Scope<'_, '_>, node_id: usize, b: &BasicBlock| { let name = &item.symbol_name; let this_block = block_name(name, node_id); - let mut n = cluster.node_named(&this_block); + + let mut label_strs: Vec = + b.statements.iter().map(render_stmt).collect(); // TODO: render statements and terminator as text label (with line breaks) // switch on terminator kind, add inner and out-edges according to terminator use TerminatorKind::*; match &b.terminator.kind { Goto { target } => { - n.set_label("Goto"); - drop(n); // so we can borrow `cluster` again below + label_strs.push("Goto".to_string()); cluster.edge(&this_block, block_name(name, *target)); } - SwitchInt { discr: _, targets } => { - n.set_label("SwitchInt"); - drop(n); // so we can borrow `cluster` again below + SwitchInt { discr, targets } => { + label_strs.push(format!("SwitchInt {}", discr.label())); for (d, t) in targets.clone().branches() { cluster .edge(&this_block, block_name(name, t)) @@ -114,24 +120,23 @@ impl SmirJson<'_> { .set_label("other"); } Resume {} => { - n.set_label("Resume"); + label_strs.push("Resume".to_string()); } Abort {} => { - n.set_label("Abort"); + label_strs.push("Abort".to_string()); } Return {} => { - n.set_label("Return"); + label_strs.push("Return".to_string()); } Unreachable {} => { - n.set_label("Unreachable"); + label_strs.push("Unreachable".to_string()); } TerminatorKind::Drop { place, target, unwind, } => { - n.set_label(&format!("Drop {}", show_place(place))); - drop(n); + label_strs.push(format!("Drop {}", place.label())); if let UnwindAction::Cleanup(t) = unwind { cluster .edge(&this_block, block_name(name, *t)) @@ -147,8 +152,7 @@ impl SmirJson<'_> { target, unwind, } => { - n.set_label("Call()"); - drop(n); + label_strs.push("Call".to_string()); if let UnwindAction::Cleanup(t) = unwind { cluster .edge(&this_block, block_name(name, *t)) @@ -156,7 +160,7 @@ impl SmirJson<'_> { .set_label("Cleanup"); } if let Some(t) = target { - let dest = show_place(destination); + let dest = destination.label(); cluster .edge(&this_block, block_name(name, *t)) .attributes() @@ -166,9 +170,24 @@ impl SmirJson<'_> { // The call edge has to be drawn outside the cluster, outside this function (cluster borrows &mut graph)! // Code for that is therefore separated into its own second function below. } - Assert { target, .. } => { - n.set_label("Assert"); - drop(n); + Assert { + cond, + expected, + msg: _, + target, + unwind, + } => { + label_strs.push(format!( + "Assert {} == {}", + cond.label(), + expected + )); + if let UnwindAction::Cleanup(t) = unwind { + cluster + .edge(&this_block, block_name(name, *t)) + .attributes() + .set_label("Cleanup"); + } cluster.edge(&this_block, block_name(name, *target)); } InlineAsm { @@ -176,8 +195,7 @@ impl SmirJson<'_> { unwind, .. } => { - n.set_label("Inline ASM"); - drop(n); + label_strs.push("Inline ASM".to_string()); if let Some(t) = destination { cluster.edge(&this_block, block_name(name, *t)); } @@ -189,6 +207,9 @@ impl SmirJson<'_> { } } } + let mut n = cluster.node_named(&this_block); + label_strs.push("".to_string()); + n.set_label(&label_strs.join("\\l")); }; let process_blocks = @@ -248,24 +269,16 @@ impl SmirJson<'_> { } Operand::Copy(place) => graph.edge( &this_block, - format!( - "{}: {}", - &this_block, - show_place(place) - ), + format!("{}: {}", &this_block, place.label()), ), Operand::Move(place) => graph.edge( &this_block, - format!( - "{}: {}", - &this_block, - show_place(place) - ), + format!("{}: {}", &this_block, place.label()), ), }; let arg_str = args .iter() - .map(show_op) + .map(|op| op.label()) .collect::>() .join(","); e.attributes().set_label(&arg_str); @@ -311,26 +324,6 @@ impl SmirJson<'_> { } } -fn show_op(op: &Operand) -> String { - match op { - Operand::Constant(ConstOperand { const_, .. }) => format!("const :: {}", const_.ty()), - Operand::Copy(place) => show_place(place), - Operand::Move(place) => show_place(place), - } -} - -fn show_place(p: &Place) -> String { - format!( - "_{}{}", - p.local, - if !p.projection.is_empty() { - "(...)" - } else { - "" - } - ) -} - fn is_unqualified(name: &str) -> bool { !name.contains("::") } @@ -345,8 +338,8 @@ fn function_string(f: FnSymType) -> String { fn name_lines(name: &str) -> String { name.split_inclusive(" ") - .flat_map(|s| s.split_inclusive("::")) - .map(|s| s.to_string()) + .flat_map(|s| s.as_bytes().chunks(25)) + .map(|bs| core::str::from_utf8(bs).unwrap().to_string()) .collect::>() .join("\\n") } @@ -364,3 +357,133 @@ fn block_name(function_name: &String, id: usize) -> String { function_name.hash(&mut h); format!("X{:x}_{}", h.finish(), id) } + +fn render_stmt(s: &Statement) -> String { + use StatementKind::*; + match &s.kind { + Assign(p, v) => format!("{} <- {}", p.label(), v.label()), + FakeRead(_cause, p) => format!("Fake-Read {}", p.label()), + SetDiscriminant { + place, + variant_index, + } => format!( + "set discriminant {}({})", + place.label(), + variant_index.to_index() + ), + Deinit(p) => format!("Deinit {}", p.label()), + StorageLive(l) => format!("Storage Live _{}", &l), + StorageDead(l) => format!("Storage Dead _{}", &l), + Retag(_retag_kind, p) => format!("Retag {}", p.label()), + PlaceMention(p) => format!("Mention {}", p.label()), + AscribeUserType { + place, + projections, + variance: _, + } => format!("Ascribe {}.{}", place.label(), projections.base), + Coverage(_) => "Coverage".to_string(), + Intrinsic(intr) => format!("Intr: {}", intr.label()), + ConstEvalCounter {} => "ConstEvalCounter".to_string(), + Nop {} => "Nop".to_string(), + } +} + +/// Rendering things as part of graph node labels +trait GraphLabelString { + fn label(&self) -> String; +} + +impl GraphLabelString for Operand { + fn label(&self) -> String { + match &self { + Operand::Constant(ConstOperand { const_, .. }) => format!("const :: {}", const_.ty()), + Operand::Copy(place) => place.label(), + Operand::Move(place) => place.label(), + } + } +} + +impl GraphLabelString for Place { + fn label(&self) -> String { + format!( + "_{}{}", + &self.local, + if !&self.projection.is_empty() { + "(...)" + } else { + "" + } + ) + } +} + +impl GraphLabelString for AggregateKind { + fn label(&self) -> String { + use AggregateKind::*; + use Mutability::*; + match &self { + Array(_ty) => "Array".to_string(), + Tuple {} => "Tuple".to_string(), + Adt(_, _, _, _, _) => "Adt".to_string(), // (AdtDef, VariantIdx, GenericArgs, Option, Option), + Closure(_, _) => "Closure".to_string(), // (ClosureDef, GenericArgs), + Coroutine(_, _, _) => "Coroutine".to_string(), // (CoroutineDef, GenericArgs, Movability), + // CoroutineClosure{} => "CoroutineClosure".to_string(), // (CoroutineClosureDef, GenericArgs), + RawPtr(ty, Mut) => format!("*mut ({})", ty), + RawPtr(ty, Not) => format!("*({})", ty), + } + } +} + +impl GraphLabelString for Rvalue { + fn label(&self) -> String { + use Rvalue::*; + match &self { + AddressOf(kind, p) => format!("&{:?} {}", kind, p.label()), + Aggregate(kind, operands) => { + let os: Vec = operands.iter().map(|op| op.label()).collect(); + format!("{} ({})", kind.label(), os.join(", ")) + } + BinaryOp(binop, op1, op2) => format!("{:?}({}, {})", binop, op1.label(), op2.label()), + Cast(kind, op, _ty) => format!("Cast-{:?} {}", kind, op.label()), + CheckedBinaryOp(binop, op1, op2) => { + format!("chkd-{:?}({}, {})", binop, op1.label(), op2.label()) + } + CopyForDeref(p) => format!("CopyForDeref({})", p.label()), + Discriminant(p) => format!("Discriminant({})", p.label()), + Len(p) => format!("Len({})", p.label()), + Ref(region, borrowkind, p) => { + format!("{:?} ({:?}): {}", region.kind, borrowkind, p.label()) + } + Repeat(op, _ty_const) => format!("Repeat {}", op.label()), + ShallowInitBox(op, _ty) => format!("ShallowInitBox({})", op.label()), + ThreadLocalRef(_item) => "ThreadLocalRef".to_string(), + NullaryOp(nullop, ty) => format!("{} :: {}", nullop.label(), ty), + UnaryOp(unop, op) => format!("{:?}({})", unop, op.label()), + Use(op) => format!("Use({})", op.label()), + } + } +} + +impl GraphLabelString for NullOp { + fn label(&self) -> String { + match &self { + NullOp::OffsetOf(_vec) => "OffsetOf(..)".to_string(), + other => format!("{:?}", other), + } + } +} + +impl GraphLabelString for NonDivergingIntrinsic { + fn label(&self) -> String { + use NonDivergingIntrinsic::*; + match &self { + Assume(op) => format!("Assume {}", op.label()), + CopyNonOverlapping(c) => format!( + "CopyNonOverlapping: {} <- {}({}))", + c.dst.label(), + c.src.label(), + c.count.label() + ), + } + } +} diff --git a/src/printer.rs b/src/printer.rs index a851334..c3d66ca 100644 --- a/src/printer.rs +++ b/src/printer.rs @@ -605,7 +605,7 @@ fn collect_ty(val_collector: &mut InternedValueCollector, val: stable_mir::ty::T // HACK: std::fmt::Arguments has escaping bounds and will error if trying to get the layout. // We will just ban producing the layout for now see, this issue for more info - // https://github.com/runtimeverification/smir_pretty/issues/27 + // https://github.com/runtimeverification/stable-mir-json/issues/27 let maybe_layout = match val.kind() { stable_mir::ty::TyKind::RigidTy(Adt(AdtDef(def_id_stable), _)) => { let def_id_internal = rustc_internal::internal(val_collector.tcx, def_id_stable); diff --git a/tests/test_basic.rs b/tests/test_basic.rs deleted file mode 100644 index a0c6a87..0000000 --- a/tests/test_basic.rs +++ /dev/null @@ -1,14 +0,0 @@ -mod common; -use common::*; -use smir_pretty::{print_all_items_verbose, stable_mir_driver}; - -#[test] -fn test_pretty_print() { - stable_mir_driver( - &vec![ - "rustc".into(), - get_resource_path(vec!["tests", "resources", "println.rs"]), - ], - print_all_items_verbose, - ); -}