refactor(ir): centralize storage-dtype composition, de-panic array indices

src/ir/error.rs

@@ -1,3 +1,11 @@
+//! Error type for the IR layer.
+//!
+//! This module defines [`Error`], the single error type produced by IR
+//! generation and module-level type registration.  Its variants cover name
+//! and type resolution failures, duplicate or conflicting definitions,
+//! unsupported source constructs, and compile-time evaluation failures;
+//! the driver surfaces them to the user as diagnostics.
+
 use crate::ast;
 use crate::ir::types::Dtype;
 use std::path::PathBuf;
@@ -60,6 +68,9 @@ pub enum Error {
     #[error("Invalid array expression")]
     InvalidArrayExpression,
 
+    #[error("Array index {index} exceeds the IR's GEP index width (i32::MAX)")]
+    ArrayIndexTooLarge { index: usize },
+
     #[error("Reference operator '&' can only be applied to array variables, not '{symbol}'")]
     InvalidReference { symbol: String },
 

src/ir/gen/function_gen.rs

@@ -17,14 +17,13 @@ use crate::ir::Error;
 /// Builds an i32-typed [`Operand`] for a GEP index.
 ///
 /// Array/struct indices are `usize` in the AST and source-language domain
-/// but must be lowered to `i32` to match LLVM IR's GEP index width.  The
-/// compiler panics if a single declared array or struct exceeds `i32::MAX`
-/// elements — this is a hard limit on the emitted IR, not a TeaLang rule,
-/// and in practice no program will ever approach it.
-fn array_index_operand(index: usize) -> Operand {
-    Operand::from(
-        i32::try_from(index).expect("array/struct index exceeds i32::MAX (LLVM GEP width)"),
-    )
+/// but must be lowered to `i32` to match LLVM IR's GEP index width.  A
+/// source-derived index that does not fit is rejected with
+/// [`Error::ArrayIndexTooLarge`]; that limit comes from the emitted IR,
+/// not from a TeaLang rule.
+fn array_index_operand(index: usize) -> Result<Operand, Error> {
+    let index = i32::try_from(index).map_err(|_| Error::ArrayIndexTooLarge { index })?;
+    Ok(Operand::from(index))
 }
 
 /// Returns the element type of the array pointed to by `base_ptr`.
@@ -45,9 +44,6 @@ fn array_element_dtype(base_ptr: &Operand) -> Dtype {
     }
 }
 
-// -----------------------------------------------------------------------
-// Function entry-point generation
-// -----------------------------------------------------------------------
 
 impl FunctionGenerator<'_> {
     /// Generates IR for a complete function definition.
@@ -60,8 +56,9 @@ impl FunctionGenerator<'_> {
     /// - `from`: the AST node for the function being compiled.
     ///
     /// # Errors
-    /// Returns an error if the function is not registered in the type registry,
-    /// an argument name is redefined, or the return type is unsupported.
+    /// Returns [`Error::FunctionNotDefined`] when the function is absent from
+    /// the type registry, [`Error::VariableRedefinition`] when an argument
+    /// name is duplicated, and any error propagated from statement lowering.
     pub fn generate(&mut self, from: &ast::FnDef) -> Result<(), Error> {
         let identifier = &from.fn_decl.identifier;
         let function_type = self
@@ -92,12 +89,10 @@ impl FunctionGenerator<'_> {
             // Allocate a stack slot (pointer to the argument type) for the argument.
             let slot = self.fresh_local(Dtype::ptr_to(dtype.clone()));
             self.emit_alloca(Operand::from(&slot));
-            // Store the incoming value into the newly allocated stack slot.
             self.emit_store(Operand::from(arg_local), Operand::from(&slot));
             self.local_variables.insert(id.clone(), slot);
         }
 
-        // Lower the function body statement by statement.
         for stmt in &from.stmts {
             self.handle_block(stmt, None, None)?;
         }
@@ -124,10 +119,6 @@ impl FunctionGenerator<'_> {
     }
 }
 
-// -----------------------------------------------------------------------
-// Statement handlers
-// -----------------------------------------------------------------------
-
 impl FunctionGenerator<'_> {
     /// Dispatches a single code-block statement to the appropriate handler.
     ///
@@ -170,7 +161,7 @@ impl FunctionGenerator<'_> {
 
     /// Inserts a local variable into the current scope's symbol table.
     ///
-    /// Records the identifier for scope-exit cleanup via [`record_scoped_local`].
+    /// Records the identifier for scope-exit cleanup via `record_scoped_local`.
     /// Returns `VariableRedefinition` if a variable with the same name already
     /// exists in the symbol table.
     fn insert_scoped_local(
@@ -202,7 +193,7 @@ impl FunctionGenerator<'_> {
 
     /// Allocates stack space for a scalar local and initializes it with `right_val`.
     ///
-    /// Combines [`allocate_pointer_local`] with an immediate `store` instruction.
+    /// Combines `allocate_pointer_local` with an immediate `store` instruction.
     fn define_scalar_local(&mut self, pointee: Dtype, right_val: Operand) -> Local {
         let local = self.allocate_pointer_local(pointee);
         self.emit_store(right_val, Operand::from(&local));
@@ -213,10 +204,11 @@ impl FunctionGenerator<'_> {
     ///
     /// For typed declarations the base comes directly from the AST annotation.
     /// For **untyped scalars**, the base comes from the resolved-types map
-    /// produced by the type inference pass (falling back to `i32` when
-    /// inference could not determine anything — e.g. a declared-but-never-used
-    /// local).  **Untyped arrays** always default to `i32` elements because
-    /// TeaLang does not support inferring an array's element type.
+    /// produced by the type inference pass; inference rejects any variable
+    /// still `Pending` at function end, so the map covers every reachable
+    /// local and the `i32` fallback only defends the invariant.  **Untyped
+    /// arrays** always default to `i32` elements because TeaLang does not
+    /// support inferring an array's element type.
     fn local_base_dtype(
         &self,
         identifier: &str,
@@ -267,15 +259,19 @@ impl FunctionGenerator<'_> {
             let element_ptr = Operand::from(self.fresh_local(elem_ptr_dtype.clone()));
             let right_elem = self.handle_right_val(val)?;
 
-            self.emit_gep(element_ptr.clone(), base_ptr.clone(), array_index_operand(i));
+            self.emit_gep(
+                element_ptr.clone(),
+                base_ptr.clone(),
+                array_index_operand(i)?,
+            );
             self.emit_store(right_elem, element_ptr);
         }
         Ok(())
     }
 
     /// Initializes an array from an [`ArrayInitializer`].
     ///
-    /// Delegates to [`init_array`] for explicit element lists.  For fill
+    /// Delegates to `init_array` for explicit element lists.  For fill
     /// initializers, evaluates the fill value once and repeats the store for
     /// every index up to `count`.
     pub fn init_array_from(
@@ -290,7 +286,11 @@ impl FunctionGenerator<'_> {
                 let fill_val = self.handle_right_val(val)?;
                 for i in 0..*count {
                     let element_ptr = Operand::from(self.fresh_local(elem_ptr_dtype.clone()));
-                    self.emit_gep(element_ptr.clone(), base_ptr.clone(), array_index_operand(i));
+                    self.emit_gep(
+                        element_ptr.clone(),
+                        base_ptr.clone(),
+                        array_index_operand(i)?,
+                    );
                     self.emit_store(fill_val.clone(), element_ptr);
                 }
                 Ok(())
@@ -376,7 +376,6 @@ impl FunctionGenerator<'_> {
         let false_label = self.alloc_basic_block();
         let after_label = self.alloc_basic_block();
 
-        // Evaluate the condition; jump to the appropriate branch.
         self.handle_bool_unit(&stmt.bool_unit, true_label.clone(), false_label.clone())?;
 
         // Emit the then-branch; a new scope is opened so that any locals are cleaned up.
@@ -386,7 +385,6 @@ impl FunctionGenerator<'_> {
             self.handle_block(s, con_label, bre_label)?;
         }
         self.exit_scope();
-        // Jump past the else-branch to the merge point.
         self.emit_jump(after_label.clone());
 
         // Emit the (possibly absent) else-branch in its own scope.
@@ -423,7 +421,6 @@ impl FunctionGenerator<'_> {
         // Jump unconditionally into the loop test from the predecessor block.
         self.emit_jump(test_label.clone());
 
-        // Emit the loop condition test.
         self.emit_label(test_label.clone());
         self.handle_bool_unit(&stmt.bool_unit, true_label.clone(), false_label.clone())?;
 
@@ -477,10 +474,6 @@ impl FunctionGenerator<'_> {
     }
 }
 
-// -----------------------------------------------------------------------
-// Expression and value handlers
-// -----------------------------------------------------------------------
-
 impl FunctionGenerator<'_> {
     /// Lowers a comparison expression into a conditional branch.
     ///
@@ -749,21 +742,17 @@ impl FunctionGenerator<'_> {
                 self.emit_load(idx.clone(), src);
                 Ok(idx)
             }
-            ast::IndexExprInner::Num(num) => Ok(array_index_operand(*num)),
+            ast::IndexExprInner::Num(num) => array_index_operand(*num),
         }
     }
 }
 
-// -----------------------------------------------------------------------
-// Boolean expression handlers
-// -----------------------------------------------------------------------
-
 impl FunctionGenerator<'_> {
     /// Lowers a boolean expression to a materialized `i32` value (0 or 1).
     ///
     /// Allocates a temporary `i32` stack slot, evaluates the expression as a
     /// branch (writing 1 on the true path and 0 on the false path via
-    /// [`emit_bool_materialization`]), then loads and returns the result.
+    /// `emit_bool_materialization`), then loads and returns the result.
     fn handle_bool_expr_as_value(&mut self, expr: &ast::BoolExpr) -> Result<Operand, Error> {
         let true_label = self.alloc_basic_block();
         let false_label = self.alloc_basic_block();
@@ -851,7 +840,6 @@ impl FunctionGenerator<'_> {
         let eval_right_label = self.alloc_basic_block();
         match &expr.op {
             ast::BoolBiOp::And => {
-                // Short-circuit AND: only evaluate the right side if the left side is true.
                 self.handle_bool_expr_as_branch(
                     &expr.left,
                     eval_right_label.clone(),
@@ -862,7 +850,6 @@ impl FunctionGenerator<'_> {
                 self.handle_bool_expr_as_branch(&expr.right, true_label, false_label)?;
             }
             ast::BoolBiOp::Or => {
-                // Short-circuit OR: only evaluate the right side if the left side is false.
                 self.handle_bool_expr_as_branch(
                     &expr.left,
                     true_label.clone(),

src/ir/gen/module_gen.rs

@@ -10,6 +10,7 @@
 use crate::ast;
 use crate::ir::compute_link_name;
 use crate::ir::function::{BasicBlock, BlockLabel, Function, FunctionBody, FunctionGenerator};
+use crate::ir::gen::conversions::compose_var_decl_dtype;
 use crate::ir::gen::type_infer;
 use crate::ir::module::IrGenerator;
 use crate::ir::printer::IrPrinter;
@@ -24,7 +25,7 @@ use std::fs;
 use std::io::Write;
 use std::rc::Rc;
 
-/// Implements the two-phase `Generator` trait for the module-level IR generator.
+/// Drives whole-program IR generation; the pass pipeline is documented on `generate`.
 impl Generator for IrGenerator<'_> {
     type Error = Error;
 
@@ -70,10 +71,10 @@ impl Generator for IrGenerator<'_> {
         }
 
         // Pass 2.5: run every module-level plug-in pass registered by
-        // the driver.  We `mem::take` the pipeline out so the pass can
-        // receive `&mut self` without aliasing the list that dispatched
-        // it — the generator's `module_passes` field is empty for the
-        // duration of the loop and gets restored on exit.
+        // the driver.  `mem::take` moves the pipeline out of the generator
+        // so the pass can receive `&mut self` without aliasing the list that
+        // dispatched it — the `module_passes` field is empty for the
+        // duration of the loop and is restored on exit.
         {
             let passes = std::mem::take(&mut self.module_passes);
             let result = passes.run(self);
@@ -93,8 +94,8 @@ impl Generator for IrGenerator<'_> {
                 let resolved_types =
                     type_infer::infer_function(&self.registry, &self.module.global_list, fn_def)?;
 
-                // Use a scoped FunctionGenerator so its temporary state is
-                // dropped before we mutably borrow `self.module` below.
+                // Scoped so the FunctionGenerator's temporary state drops
+                // before `self.module` is mutably borrowed below.
                 let body = {
                     let mut function_generator = FunctionGenerator::new(
                         &self.registry,
@@ -132,7 +133,7 @@ impl Generator for IrGenerator<'_> {
     /// Emit the complete IR module to the provided writer in textual form.
     ///
     /// Delegates to [`IrPrinter::emit_module`] — the same helper used by
-    /// [`crate::opt::Optimizer::output`] — so that pre-optimization and
+    /// `crate::opt::Optimizer::output` — so that pre-optimization and
     /// post-optimization IR dumps share one code path and one canonical
     /// format.
     fn output<W: Write>(&self, w: &mut W) -> Result<(), Error> {
@@ -237,7 +238,6 @@ impl IrGenerator<'_> {
 
         for stmt in irs {
             if let StmtInner::Label(l) = &stmt.inner {
-                // Finalise the previous block (if any) and start a new one.
                 if let Some(prev_label) = label.take() {
                     blocks.push(BasicBlock {
                         label: prev_label,
@@ -270,9 +270,6 @@ impl IrGenerator<'_> {
             return blocks;
         }
 
-        // Hoist all allocas from non-entry blocks to the entry block, right
-        // after the entry label.  This ensures all stack allocations happen in
-        // the entry block (LLVM convention).
         let mut hoisted_allocas: Vec<Stmt> = Vec::new();
         for block in blocks.iter_mut().skip(1) {
             let (allocas, remaining): (Vec<Stmt>, Vec<Stmt>) = block
@@ -282,16 +279,14 @@ impl IrGenerator<'_> {
             hoisted_allocas.extend(allocas);
             block.stmts = remaining;
         }
-        // Insert hoisted allocas at the beginning of the entry block.
         blocks[0].stmts.splice(0..0, hoisted_allocas);
 
         // Post-hoist invariant: every block still has at least a terminator
-        // (`return` / `jump` / `cjump`) because the IR generator always emits
-        // one for reachable blocks, and the terminator is not an alloca so it
-        // is never hoisted away.  Blocks that somehow ended up empty (only an
-        // alloca-only body) would become dangling jump targets if dropped, so
-        // we verify — and drop — them together with the edges that reach
-        // them.
+        // (`return` / `jump` / `cjump`), because the generator emits one for
+        // every reachable block and a terminator is never an alloca, so
+        // hoisting cannot empty a reachable block.  An empty block that is
+        // still referenced by a jump would become a dangling target once
+        // dropped, so removal below is guarded by that reference check.
         Self::drop_empty_blocks_or_panic(&mut blocks);
 
         blocks
@@ -310,8 +305,8 @@ impl IrGenerator<'_> {
             return;
         }
 
-        // Collect the labels referenced by any remaining (non-empty) block's
-        // terminator so we can tell whether an empty block is still reachable.
+        // Collect every label targeted by a non-empty block's terminator; an
+        // empty block whose label appears in this set is still reachable.
         let mut referenced: HashSet<String> = HashSet::new();
         for block in blocks.iter().filter(|b| !b.stmts.is_empty()) {
             for stmt in &block.stmts {
@@ -534,10 +529,7 @@ impl IrGenerator<'_> {
                 decl.identifier.clone(),
                 StructMember {
                     index,
-                    dtype: match &decl.inner {
-                        ast::VarDeclInner::Scalar => base_dtype,
-                        ast::VarDeclInner::Array(array) => Dtype::array_of(base_dtype, array.len),
-                    },
+                    dtype: compose_var_decl_dtype(base_dtype, &decl.inner),
                 },
             ));
         }

src/ir/gen/static_eval.rs

@@ -1,13 +1,15 @@
+//! Compile-time (static) evaluation of constant expressions.
+//!
+//! This module folds AST expressions composed entirely of literals and
+//! constant operators into concrete `i32` values during IR generation.
+//! Failures that would otherwise only appear at runtime — division by zero
+//! and integer overflow — are reported here as compile-time errors.
+
 use crate::ast;
 use crate::ir::module::IrGenerator;
 use crate::ir::Error;
 
 /// Static evaluation methods for the IR generator.
-///
-/// These functions perform compile-time (static) evaluation of constant expressions
-/// from the AST, folding them into concrete `i32` values. This is used for constant
-/// folding during IR generation — expressions composed entirely of literals and
-/// constant operations can be reduced to a single integer value at compile time.
 impl IrGenerator<'_> {
     /// Statically evaluates a right-hand-side value.
     ///