feat(assembly): track source locations in debug mode

CHANGELOG.md

@@ -27,11 +27,18 @@
 - Added `Assembler::assemble_library()` (#1413).
 - Added `Assembler::assemble_kernel()` (#1418).
 - Added `miden_core::prettier::pretty_print_csv` helper, for formatting of iterators over `PrettyPrint` values as comma-separated items
+- Added source location tracking to assembled MAST (#1419)
+- Added source code management primitives in `miden-core` (#1419)
+- Added `make test-fast` and `make test-skip-proptests` Makefile targets for faster testing during local development
 
 #### Changed
 
 - When using `if.(true|false) .. end`, the parser used to emit an empty block for the branch that was elided. The parser now emits a block containing a single `nop` instruction instead, which is equivalent to the code emitted by the assembler when lowering to MAST.
 - `internals` configuration feature was renamed to `testing` (#1399).
+- The `AssemblyOp` decorator now contains an optional `Location` (#1419)
+- The `Assembler` now requires passing in a `Arc<dyn SourceManager>`, for use in rendering diagnostics
+- The `Module::parse_file` and `Module::parse_str` functions have been removed in favor of calling `Module::parser` and then using the `ModuleParser` methods
+- The `Compile` trait now requires passing a `SourceManager` reference along with the item to be compiled
 
 ## 0.9.2 (2024-05-22) - `stdlib` crate only
 

Makefile

@@ -50,9 +50,21 @@ mdbook: ## Generates mdbook documentation
 # --- testing -------------------------------------------------------------------------------------
 
 .PHONY: test
-test: ## Runs all tests
+test: ## Runs all tests with the release profile
 	$(DEBUG_ASSERTIONS) cargo nextest run --cargo-profile test-release --features testing
 
+.PHONY: test-fast
+test-fast: ## Runs all tests with the debug profile
+	$(DEBUG_ASSERTIONS) cargo nextest run --features testing
+
+.PHONY: test-skip-proptests
+test-skip-proptests: ## Runs all tests, except property-based tests
+	$(DEBUG_ASSERTIONS) cargo nextest run --features testing -E 'not test(#*proptest)'
+
+.PHONY: test-loom
+test-loom: ## Runs all loom-based tests
+	RUSTFLAGS="--cfg loom" cargo nextest run --cargo-profile test-release --features testing -E 'test(#*loom)'
+
 # --- checking ------------------------------------------------------------------------------------
 
 .PHONY: check

assembly/Cargo.toml

@@ -49,7 +49,9 @@ tracing = { version = "0.1", default-features = false, features = [
 ] }
 thiserror = { version = "1.0", git = "https://github.com/bitwalker/thiserror", branch = "no-std", default-features = false }
 unicode-width = { version = "0.1", features = ["no_std"] }
-vm-core = { package = "miden-core", path = "../core", version = "0.10", default-features = false }
+vm-core = { package = "miden-core", path = "../core", version = "0.10", default-features = false, features = [
+    "diagnostics",
+] }
 
 [dev-dependencies]
 pretty_assertions = "1.4"

assembly/src/assembler/basic_block_builder.rs

@@ -1,4 +1,4 @@
-use crate::{ast::Instruction, AssemblyError};
+use crate::{ast::Instruction, AssemblyError, Span};
 
 use super::{
     mast_forest_builder::MastForestBuilder, BodyWrapper, Decorator, DecoratorList, ProcedureContext,
@@ -83,12 +83,18 @@ impl BasicBlockBuilder {
     ///
     /// This indicates that the provided instruction should be tracked and the cycle count for
     /// this instruction will be computed when the call to set_instruction_cycle_count() is made.
-    pub fn track_instruction(&mut self, instruction: &Instruction, proc_ctx: &ProcedureContext) {
+    pub fn track_instruction(
+        &mut self,
+        instruction: &Span<Instruction>,
+        proc_ctx: &ProcedureContext,
+    ) {
+        let span = instruction.span();
+        let location = proc_ctx.source_manager().location(span).ok();
         let context_name = proc_ctx.name().to_string();
         let num_cycles = 0;
         let op = instruction.to_string();
         let should_break = instruction.should_break();
-        let op = AssemblyOp::new(context_name, num_cycles, op, should_break);
+        let op = AssemblyOp::new(location, context_name, num_cycles, op, should_break);
         self.push_decorator(Decorator::AsmOp(op));
         self.last_asmop_pos = self.decorators.len() - 1;
     }

assembly/src/assembler/instruction/env_ops.rs

@@ -1,5 +1,5 @@
 use super::{mem_ops::local_to_absolute_addr, push_felt, BasicBlockBuilder};
-use crate::{assembler::ProcedureContext, AssemblyError, Felt, Spanned};
+use crate::{assembler::ProcedureContext, AssemblyError, Felt, SourceSpan};
 use vm_core::Operation::*;
 
 // CONSTANT INPUTS
@@ -54,11 +54,12 @@ pub fn locaddr(
 pub fn caller(
     span: &mut BasicBlockBuilder,
     proc_ctx: &ProcedureContext,
+    source_span: SourceSpan,
 ) -> Result<(), AssemblyError> {
     if !proc_ctx.is_kernel() {
         return Err(AssemblyError::CallerOutsideOfKernel {
-            span: proc_ctx.span(),
-            source_file: proc_ctx.source_file(),
+            span: source_span,
+            source_file: proc_ctx.source_manager().get(source_span.source_id()).ok(),
         });
     }
     span.push_op(Caller);

assembly/src/assembler/instruction/field_ops.rs

@@ -93,8 +93,9 @@ pub fn div_imm(
     imm: Span<Felt>,
 ) -> Result<(), AssemblyError> {
     if imm == ZERO {
-        let source_file = proc_ctx.source_file();
-        let error = Report::new(crate::parser::ParsingError::DivisionByZero { span: imm.span() });
+        let source_span = imm.span();
+        let source_file = proc_ctx.source_manager().get(source_span.source_id()).ok();
+        let error = Report::new(crate::parser::ParsingError::DivisionByZero { span: source_span });
         return Err(if let Some(source_file) = source_file {
             AssemblyError::Other(RelatedError::new(error.with_source_code(source_file)))
         } else {

assembly/src/assembler/instruction/mod.rs

@@ -2,7 +2,9 @@ use super::{
     ast::InvokeKind, mast_forest_builder::MastForestBuilder, Assembler, BasicBlockBuilder, Felt,
     Operation, ProcedureContext,
 };
-use crate::{ast::Instruction, diagnostics::Report, utils::bound_into_included_u64, AssemblyError};
+use crate::{
+    ast::Instruction, diagnostics::Report, utils::bound_into_included_u64, AssemblyError, Span,
+};
 use core::ops::RangeBounds;
 use vm_core::{mast::MastNodeId, Decorator, ONE, ZERO};
 
@@ -21,7 +23,7 @@ use self::u32_ops::U32OpMode::*;
 impl Assembler {
     pub(super) fn compile_instruction(
         &self,
-        instruction: &Instruction,
+        instruction: &Span<Instruction>,
         span_builder: &mut BasicBlockBuilder,
         proc_ctx: &mut ProcedureContext,
         mast_forest_builder: &mut MastForestBuilder,
@@ -50,14 +52,14 @@ impl Assembler {
 
     fn compile_instruction_impl(
         &self,
-        instruction: &Instruction,
+        instruction: &Span<Instruction>,
         span_builder: &mut BasicBlockBuilder,
         proc_ctx: &mut ProcedureContext,
         mast_forest_builder: &mut MastForestBuilder,
     ) -> Result<Option<MastNodeId>, AssemblyError> {
         use Operation::*;
 
-        match instruction {
+        match &**instruction {
             Instruction::Nop => span_builder.push_op(Noop),
             Instruction::Assert => span_builder.push_op(Assert(0)),
             Instruction::AssertWithError(err_code) => {
@@ -311,7 +313,7 @@ impl Assembler {
             Instruction::PushU32List(imms) => env_ops::push_many(imms, span_builder),
             Instruction::PushFeltList(imms) => env_ops::push_many(imms, span_builder),
             Instruction::Sdepth => span_builder.push_op(SDepth),
-            Instruction::Caller => env_ops::caller(span_builder, proc_ctx)?,
+            Instruction::Caller => env_ops::caller(span_builder, proc_ctx, instruction.span())?,
             Instruction::Clk => span_builder.push_op(Clk),
             Instruction::AdvPipe => span_builder.push_op(Pipe),
             Instruction::AdvPush(n) => adv_ops::adv_push(span_builder, n.expect_value())?,

assembly/src/assembler/instruction/procedures.rs

@@ -31,7 +31,7 @@ impl Assembler {
         mast_forest_builder: &mut MastForestBuilder,
     ) -> Result<Option<MastNodeId>, AssemblyError> {
         // Get the procedure from the assembler
-        let current_source_file = proc_ctx.source_file();
+        let current_source_file = self.source_manager.get(span.source_id()).ok();
 
         // If the procedure is cached, register the call to ensure the callset
         // is updated correctly.

assembly/src/assembler/instruction/u32_ops.rs

@@ -372,9 +372,9 @@ fn handle_division(
 ) -> Result<(), AssemblyError> {
     if let Some(imm) = imm {
         if imm == 0 {
-            let source_file = proc_ctx.source_file();
-            let error =
-                Report::new(crate::parser::ParsingError::DivisionByZero { span: imm.span() });
+            let imm_span = imm.span();
+            let source_file = proc_ctx.source_manager().get(imm_span.source_id()).ok();
+            let error = Report::new(crate::parser::ParsingError::DivisionByZero { span: imm_span });
             return if let Some(source_file) = source_file {
                 Err(AssemblyError::Other(RelatedError::new(error.with_source_code(source_file))))
             } else {