From 95b4a62c538071f400a0298bb079500624d052eb Mon Sep 17 00:00:00 2001 From: Yi Sun Date: Sat, 1 Aug 2026 17:47:05 +0800 Subject: [PATCH 1/2] refactor(ir): move indentation and GEP layout out of model Display Stmt's Display used to bake a leading tab into every instruction, making the model own the printer's layout policy; GepStmt's Display embedded struct/array layout computation and could fail with fmt::Error, so formatting doubled as a late error path. Both concerns now live at the printing boundary: instructions render as bare text, IrPrinter::emit_function_def owns the indent column, and the fallible GEP computation moved into the explicit GepStmt::render helper, which the printer consumes through its Result-returning emit path. GepStmt keeps no Display impl of its own; the Stmt::Gep Display arm preserves historical behaviour for hypothetical direct consumers (text for well-typed GEPs, fmt::Error for ill-typed ones). Emitted IR is byte-identical (verified by diffing --emit ir output against the pre-change binary across the tests/ corpus). --- src/ir/printer.rs | 40 +++++++++++++++++- src/ir/stmt.rs | 101 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 114 insertions(+), 27 deletions(-) diff --git a/src/ir/printer.rs b/src/ir/printer.rs index 3bcd6b5..c5ba5ec 100644 --- a/src/ir/printer.rs +++ b/src/ir/printer.rs @@ -1,6 +1,22 @@ +//! Textual printer for teac's IR (the `--emit ir` dump). +//! +//! [`IrPrinter`] is the single source of truth for what a teac IR file +//! looks like: target header, struct types, globals, then every function +//! as a definition or declaration. All presentation policy lives here +//! rather than on the model types: the printer owns the leading `\t` that +//! indents instructions inside a function body (statement `Display` +//! impls in [`super::stmt`] deliberately emit bare text), and it consumes +//! the fallible [`GepStmt::render`] directly so that a GEP with an +//! ill-typed base surfaces as a proper [`Error`] through the +//! `Result`-returning emit path instead of as an opaque `fmt::Error` +//! buried inside a `writeln!`. +//! +//! [`GepStmt::render`]: super::stmt::GepStmt::render + use super::error::Error; use super::function::FunctionBody; use super::module::{Module, Registry}; +use super::stmt::{Stmt, StmtInner}; use super::types::{Dtype, FunctionType, StructType}; use super::value::GlobalDef; use std::io::Write; @@ -140,7 +156,7 @@ impl IrPrinter { for block in &body.blocks { writeln!(self.writer, "{}:", block.label)?; for stmt in &block.stmts { - writeln!(self.writer, "{stmt}")?; + self.emit_stmt(stmt)?; } } writeln!(self.writer, "}}")?; @@ -148,6 +164,28 @@ impl IrPrinter { Ok(()) } + /// Emit a single instruction line inside a function body, owning the + /// leading `\t` indent that [`Stmt`]'s `Display` deliberately leaves + /// to the printer. Block labels are not statements (`BasicBlock` + /// carries its label separately and [`emit_function_def`] prints it + /// unindented above), so every statement through this path is an + /// indented instruction. + /// + /// GEPs take a special route because their rendering is fallible — + /// [`GepStmt::render`] resolves the base operand's layout and reports + /// ill-typed bases as an [`Error`]; every other instruction renders + /// through its infallible `Display`. + /// + /// [`emit_function_def`]: Self::emit_function_def + /// [`GepStmt::render`]: super::stmt::GepStmt::render + fn emit_stmt(&mut self, stmt: &Stmt) -> Result<(), Error> { + match &stmt.inner { + StmtInner::Gep(gep) => writeln!(self.writer, "\t{}", gep.render()?)?, + _ => writeln!(self.writer, "\t{stmt}")?, + } + Ok(()) + } + pub fn emit_function_decl( &mut self, link_name: &str, diff --git a/src/ir/stmt.rs b/src/ir/stmt.rs index e1e1bbd..6c655ec 100644 --- a/src/ir/stmt.rs +++ b/src/ir/stmt.rs @@ -1,5 +1,20 @@ +//! IR statement (instruction) definitions for teac's LLVM-like IR. +//! +//! Each instruction is a small struct (`GepStmt`, `CallStmt`, …) wrapped +//! into a [`Stmt`] by the [`StmtInner`] enum. Instructions implement +//! [`Display`] as *bare* instruction text — no leading indentation, no +//! trailing newline — because laying out a function body (indent column, +//! label placement) is the printer's business, not the model's: that is +//! owned by [`super::printer::IrPrinter`]. The one instruction that +//! cannot be rendered infallibly is the GEP — its text depends on the +//! base operand's layout and an ill-typed base has no rendering — so it +//! carries no `Display` impl at all; instead the fallible logic lives in +//! the explicit [`GepStmt::render`] helper, which the printer consumes +//! through its `Result`-returning emit path. + use crate::ast; +use super::error::Error; use super::function::BlockLabel; use super::types::Dtype; use super::value::Operand; @@ -213,21 +228,36 @@ impl Stmt { } } +/// Emits the bare instruction text — never a leading `\t`. +/// +/// Indentation inside a function body is the printer's concern +/// ([`super::printer::IrPrinter`]), so that the model types carry no +/// presentation policy and other consumers (diagnostics, debugging) get +/// unadorned text. impl Display for Stmt { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match &self.inner { - StmtInner::Alloca(s) => write!(f, "\t{s}"), - StmtInner::BiOp(s) => write!(f, "\t{s}"), - StmtInner::CJump(s) => write!(f, "\t{s}"), - StmtInner::Call(s) => write!(f, "\t{s}"), - StmtInner::Cmp(s) => write!(f, "\t{s}"), - StmtInner::Gep(s) => write!(f, "\t{s}"), + StmtInner::Alloca(s) => write!(f, "{s}"), + StmtInner::BiOp(s) => write!(f, "{s}"), + StmtInner::CJump(s) => write!(f, "{s}"), + StmtInner::Call(s) => write!(f, "{s}"), + StmtInner::Cmp(s) => write!(f, "{s}"), + // `GepStmt` has no `Display`: its rendering is fallible + // ([`GepStmt::render`]), and the printer consumes that + // fallibility through its `Result`-returning emit path. + // This arm only serves hypothetical direct `Display` + // consumers and preserves their historical behaviour: + // text for well-typed GEPs, `fmt::Error` for ill-typed ones. + StmtInner::Gep(s) => match s.render() { + Ok(text) => write!(f, "{text}"), + Err(_) => Err(fmt::Error), + }, StmtInner::Label(s) => write!(f, "{s}"), - StmtInner::Load(s) => write!(f, "\t{s}"), - StmtInner::Phi(s) => write!(f, "\t{s}"), - StmtInner::Return(s) => write!(f, "\t{s}"), - StmtInner::Store(s) => write!(f, "\t{s}"), - StmtInner::Jump(s) => write!(f, "\t{s}"), + StmtInner::Load(s) => write!(f, "{s}"), + StmtInner::Phi(s) => write!(f, "{s}"), + StmtInner::Return(s) => write!(f, "{s}"), + StmtInner::Store(s) => write!(f, "{s}"), + StmtInner::Jump(s) => write!(f, "{s}"), } } } @@ -420,8 +450,24 @@ impl Display for LabelStmt { } } -impl Display for GepStmt { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { +impl GepStmt { + /// Renders this GEP as teac IR text, choosing the `getelementptr` + /// form from the base operand's layout: + /// + /// - pointer to a fixed-size array or struct → two-index form + /// (`i32 0, i32 `), i.e. aggregate/field addressing; + /// - pointer to an unsized array (a decayed `&[T]` argument) → + /// element-typed single-index form; + /// - pointer to a scalar → single-index form; + /// - first-class array operand (e.g. a global array) → two-index form. + /// + /// The lookup is fallible: a base that is neither a pointer nor an + /// array is ill-formed IR with no valid rendering. Keeping that + /// fallibility in an explicit helper — rather than inside a `Display` + /// impl where it could only surface as an opaque `fmt::Error` — lets + /// the printer report it through its own `Result`-returning emit path. + /// This is why [`GepStmt`] deliberately has no `Display` impl. + pub fn render(&self) -> Result { let Self { new_ptr, base_ptr, @@ -432,27 +478,30 @@ impl Display for GepStmt { Dtype::Array { length: Some(_), .. } - | Dtype::Struct { .. } => write!( - f, + | Dtype::Struct { .. } => Ok(format!( "{new_ptr} = getelementptr {pointee}, ptr {base_ptr}, i32 0, i32 {index}", - ), + )), Dtype::Array { element, length: None, - } => write!( - f, + } => Ok(format!( "{new_ptr} = getelementptr {element}, ptr {base_ptr}, i32 {index}", - ), - _ => write!( - f, + )), + _ => Ok(format!( "{new_ptr} = getelementptr {pointee}, ptr {base_ptr}, i32 {index}", - ), + )), }, - dtype @ Dtype::Array { .. } => write!( - f, + dtype @ Dtype::Array { .. } => Ok(format!( "{new_ptr} = getelementptr {dtype}, ptr {base_ptr}, i32 0, i32 {index}", - ), - _ => Err(fmt::Error), + )), + // Ill-formed IR that the front end never produces; surfaced as + // an I/O error because that is exactly how the previous + // `fmt::Error`-based failure reached the caller (via + // `writeln!`'s `io::Write`). + other => Err(Error::Io(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("getelementptr on non-indexable base of type '{other}'"), + ))), } } } From 7f641f5442bfbe2082080be4920f223b13edd1a0 Mon Sep 17 00:00:00 2001 From: Yi Sun Date: Sat, 1 Aug 2026 18:35:20 +0800 Subject: [PATCH 2/2] docs(ir): discipline comments per comment spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrote 4 historical narratives as present-tense contracts (§3.1/§3.2): the removed-`\t` reference on Stmt's Display doc, "preserves their historical behaviour" in the GEP Display arm, "the previous fmt::Error-based failure" in GepStmt::render, and a "now needs" temporal marker on TARGET_TRIPLE. Fixed 1 stale claim against current code (§4.1/§8.1): emit_stmt's doc no longer asserts block labels are not statements — Label statements exist in the flat IR and are consumed when BasicBlocks are built. No commented-out code, dividers, or annotation tags found in these files. --- src/ir/printer.rs | 12 ++++++------ src/ir/stmt.rs | 19 ++++++++++--------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/ir/printer.rs b/src/ir/printer.rs index c5ba5ec..c41fb93 100644 --- a/src/ir/printer.rs +++ b/src/ir/printer.rs @@ -23,8 +23,8 @@ use std::io::Write; /// LLVM-style target triple baked into every IR dump. Kept alongside /// the printer rather than on `IrGenerator` because the printer is the -/// component that actually writes it, and `Optimizer::output` now needs -/// the same constant — making it a free constant in `ir::printer` +/// component that writes it; both `IrGenerator::output` and +/// `Optimizer::output` consume it, so a free constant in `ir::printer` /// avoids introducing an `ir::gen`-to-`opt` dependency. pub const TARGET_TRIPLE: &str = "aarch64-unknown-linux-gnu"; @@ -166,10 +166,10 @@ impl IrPrinter { /// Emit a single instruction line inside a function body, owning the /// leading `\t` indent that [`Stmt`]'s `Display` deliberately leaves - /// to the printer. Block labels are not statements (`BasicBlock` - /// carries its label separately and [`emit_function_def`] prints it - /// unindented above), so every statement through this path is an - /// indented instruction. + /// to the printer. Label statements never reach this path: the flat + /// IR is split into `BasicBlock`s that each carry their label, and + /// [`emit_function_def`] prints a label unindented above its block, + /// so every statement here is an indented instruction. /// /// GEPs take a special route because their rendering is fallible — /// [`GepStmt::render`] resolves the base operand's layout and reports diff --git a/src/ir/stmt.rs b/src/ir/stmt.rs index 6c655ec..c9fed41 100644 --- a/src/ir/stmt.rs +++ b/src/ir/stmt.rs @@ -228,10 +228,11 @@ impl Stmt { } } -/// Emits the bare instruction text — never a leading `\t`. +/// Emits the bare instruction text — no leading indentation, no +/// trailing newline. /// /// Indentation inside a function body is the printer's concern -/// ([`super::printer::IrPrinter`]), so that the model types carry no +/// ([`super::printer::IrPrinter`]), so the model types carry no /// presentation policy and other consumers (diagnostics, debugging) get /// unadorned text. impl Display for Stmt { @@ -243,11 +244,10 @@ impl Display for Stmt { StmtInner::Call(s) => write!(f, "{s}"), StmtInner::Cmp(s) => write!(f, "{s}"), // `GepStmt` has no `Display`: its rendering is fallible - // ([`GepStmt::render`]), and the printer consumes that + // (`GepStmt::render`), and the printer consumes that // fallibility through its `Result`-returning emit path. - // This arm only serves hypothetical direct `Display` - // consumers and preserves their historical behaviour: - // text for well-typed GEPs, `fmt::Error` for ill-typed ones. + // This arm keeps direct `Display` calls working: text for + // well-typed GEPs, `fmt::Error` for ill-typed ones. StmtInner::Gep(s) => match s.render() { Ok(text) => write!(f, "{text}"), Err(_) => Err(fmt::Error), @@ -495,9 +495,10 @@ impl GepStmt { "{new_ptr} = getelementptr {dtype}, ptr {base_ptr}, i32 0, i32 {index}", )), // Ill-formed IR that the front end never produces; surfaced as - // an I/O error because that is exactly how the previous - // `fmt::Error`-based failure reached the caller (via - // `writeln!`'s `io::Write`). + // an I/O error so the failure shares the printer's + // `io::Write`-based error channel — `writeln!` failures reach + // the caller as `std::io::Error` wrapped in `Error::Io`, + // exactly like this one. other => Err(Error::Io(std::io::Error::new( std::io::ErrorKind::InvalidData, format!("getelementptr on non-indexable base of type '{other}'"),