From ea4c6c4bea49db3b4c4dac7da290bbf61aae2d4d Mon Sep 17 00:00:00 2001 From: sergerad Date: Thu, 5 Jun 2025 13:21:42 +1200 Subject: [PATCH 1/8] Add Send + Sync supertraits to SourceManager trait --- assembly/src/assembler/mod.rs | 11 ++++------- assembly/src/testing.rs | 4 ++-- core/src/debuginfo/source_manager.rs | 2 +- miden/src/cli/data.rs | 6 +++--- test-utils/src/lib.rs | 2 +- 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/assembly/src/assembler/mod.rs b/assembly/src/assembler/mod.rs index e48d733c8c..34a71bd1b2 100644 --- a/assembly/src/assembler/mod.rs +++ b/assembly/src/assembler/mod.rs @@ -64,7 +64,7 @@ pub use self::{ #[derive(Clone)] pub struct Assembler { /// The source manager to use for compilation and source location information - source_manager: Arc, + source_manager: Arc, /// The global [ModuleGraph] for this assembler. module_graph: ModuleGraph, /// Whether to treat warning diagnostics as errors @@ -93,7 +93,7 @@ impl Default for Assembler { /// Constructors impl Assembler { /// Start building an [Assembler] - pub fn new(source_manager: Arc) -> Self { + pub fn new(source_manager: Arc) -> Self { let module_graph = ModuleGraph::new(source_manager.clone()); Self { source_manager, @@ -105,10 +105,7 @@ impl Assembler { } /// Start building an [`Assembler`] with a kernel defined by the provided [KernelLibrary]. - pub fn with_kernel( - source_manager: Arc, - kernel_lib: KernelLibrary, - ) -> Self { + pub fn with_kernel(source_manager: Arc, kernel_lib: KernelLibrary) -> Self { let (kernel, kernel_module, _) = kernel_lib.into_parts(); let module_graph = ModuleGraph::with_kernel(source_manager.clone(), kernel, kernel_module); Self { @@ -303,7 +300,7 @@ impl Assembler { } /// Returns a link to the source manager used by this assembler. - pub fn source_manager(&self) -> Arc { + pub fn source_manager(&self) -> Arc { self.source_manager.clone() } diff --git a/assembly/src/testing.rs b/assembly/src/testing.rs index 256f21c145..b06f8d0125 100644 --- a/assembly/src/testing.rs +++ b/assembly/src/testing.rs @@ -195,7 +195,7 @@ macro_rules! parse_module { /// /// Some of the assertion macros defined above require a [TestContext], so be aware of that. pub struct TestContext { - source_manager: Arc, + source_manager: Arc, assembler: Assembler, } @@ -233,7 +233,7 @@ impl TestContext { } #[inline(always)] - pub fn source_manager(&self) -> Arc { + pub fn source_manager(&self) -> Arc { self.source_manager.clone() } diff --git a/core/src/debuginfo/source_manager.rs b/core/src/debuginfo/source_manager.rs index 77071b51e9..887adb8af8 100644 --- a/core/src/debuginfo/source_manager.rs +++ b/core/src/debuginfo/source_manager.rs @@ -101,7 +101,7 @@ impl SourceManagerError { } } -pub trait SourceManager: Debug { +pub trait SourceManager: Debug + Send + Sync { /// Returns true if `file` is managed by this source manager fn is_manager_of(&self, file: &SourceFile) -> bool { match self.get(file.id()) { diff --git a/miden/src/cli/data.rs b/miden/src/cli/data.rs index 9b0c82ec39..b965cdbc51 100644 --- a/miden/src/cli/data.rs +++ b/miden/src/cli/data.rs @@ -109,7 +109,7 @@ impl OutputFile { pub struct ProgramFile { ast: Box, - source_manager: Arc, + source_manager: Arc, } /// Helper methods to interact with masm program file. @@ -125,7 +125,7 @@ impl ProgramFile { #[instrument(name = "read_program_file", skip(source_manager), fields(path = %path.as_ref().display()))] pub fn read_with( path: impl AsRef, - source_manager: Arc, + source_manager: Arc, ) -> Result { // parse the program into an AST let path = path.as_ref(); @@ -160,7 +160,7 @@ impl ProgramFile { } /// Returns the source manager for this program file. - pub fn source_manager(&self) -> &Arc { + pub fn source_manager(&self) -> &Arc { &self.source_manager } } diff --git a/test-utils/src/lib.rs b/test-utils/src/lib.rs index b515eb9f7a..bf5cb70f1d 100644 --- a/test-utils/src/lib.rs +++ b/test-utils/src/lib.rs @@ -171,7 +171,7 @@ macro_rules! assert_assembler_diagnostic { /// - Execution error test: check that running a program compiled from the given source causes an /// ExecutionError which contains the specified substring. pub struct Test { - pub source_manager: Arc, + pub source_manager: Arc, pub source: Arc, pub kernel_source: Option>, pub stack_inputs: StackInputs, From 2f3cdd56c69b27e8e0943965a19a54def3d05968 Mon Sep 17 00:00:00 2001 From: sergerad Date: Thu, 5 Jun 2025 13:44:31 +1200 Subject: [PATCH 2/8] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90287a730d..30038242f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ - Improve error messages for some assembler instruction (#1785) - Remove `idx` column from Kernel ROM chiplet and use chiplet bus for initialization. (#1818) -- [BREAKING] Make `Assembler::source_manager()` be `Send + Sync` (#1822) +- [BREAKING] Make `SourceManager` a subtrait of `Send + Sync`. (#1822, #1858) - Simplify and optimize the recursive verifier (#1801). - Simplify auxiliary randomness generation (#1810). - Add handling of variable length public inputs to the recursive verifier (#1813). From a3f1df48dacd5a064f6c776e23fe1b0a0b1edafd Mon Sep 17 00:00:00 2001 From: sergerad Date: Fri, 6 Jun 2025 06:10:36 +1200 Subject: [PATCH 3/8] Revert "Add Send + Sync supertraits to SourceManager trait" This reverts commit ea4c6c4bea49db3b4c4dac7da290bbf61aae2d4d. --- assembly/src/assembler/mod.rs | 11 +++++++---- assembly/src/testing.rs | 4 ++-- core/src/debuginfo/source_manager.rs | 2 +- miden/src/cli/data.rs | 6 +++--- test-utils/src/lib.rs | 2 +- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/assembly/src/assembler/mod.rs b/assembly/src/assembler/mod.rs index 34a71bd1b2..e48d733c8c 100644 --- a/assembly/src/assembler/mod.rs +++ b/assembly/src/assembler/mod.rs @@ -64,7 +64,7 @@ pub use self::{ #[derive(Clone)] pub struct Assembler { /// The source manager to use for compilation and source location information - source_manager: Arc, + source_manager: Arc, /// The global [ModuleGraph] for this assembler. module_graph: ModuleGraph, /// Whether to treat warning diagnostics as errors @@ -93,7 +93,7 @@ impl Default for Assembler { /// Constructors impl Assembler { /// Start building an [Assembler] - pub fn new(source_manager: Arc) -> Self { + pub fn new(source_manager: Arc) -> Self { let module_graph = ModuleGraph::new(source_manager.clone()); Self { source_manager, @@ -105,7 +105,10 @@ impl Assembler { } /// Start building an [`Assembler`] with a kernel defined by the provided [KernelLibrary]. - pub fn with_kernel(source_manager: Arc, kernel_lib: KernelLibrary) -> Self { + pub fn with_kernel( + source_manager: Arc, + kernel_lib: KernelLibrary, + ) -> Self { let (kernel, kernel_module, _) = kernel_lib.into_parts(); let module_graph = ModuleGraph::with_kernel(source_manager.clone(), kernel, kernel_module); Self { @@ -300,7 +303,7 @@ impl Assembler { } /// Returns a link to the source manager used by this assembler. - pub fn source_manager(&self) -> Arc { + pub fn source_manager(&self) -> Arc { self.source_manager.clone() } diff --git a/assembly/src/testing.rs b/assembly/src/testing.rs index b06f8d0125..256f21c145 100644 --- a/assembly/src/testing.rs +++ b/assembly/src/testing.rs @@ -195,7 +195,7 @@ macro_rules! parse_module { /// /// Some of the assertion macros defined above require a [TestContext], so be aware of that. pub struct TestContext { - source_manager: Arc, + source_manager: Arc, assembler: Assembler, } @@ -233,7 +233,7 @@ impl TestContext { } #[inline(always)] - pub fn source_manager(&self) -> Arc { + pub fn source_manager(&self) -> Arc { self.source_manager.clone() } diff --git a/core/src/debuginfo/source_manager.rs b/core/src/debuginfo/source_manager.rs index 887adb8af8..77071b51e9 100644 --- a/core/src/debuginfo/source_manager.rs +++ b/core/src/debuginfo/source_manager.rs @@ -101,7 +101,7 @@ impl SourceManagerError { } } -pub trait SourceManager: Debug + Send + Sync { +pub trait SourceManager: Debug { /// Returns true if `file` is managed by this source manager fn is_manager_of(&self, file: &SourceFile) -> bool { match self.get(file.id()) { diff --git a/miden/src/cli/data.rs b/miden/src/cli/data.rs index b965cdbc51..9b0c82ec39 100644 --- a/miden/src/cli/data.rs +++ b/miden/src/cli/data.rs @@ -109,7 +109,7 @@ impl OutputFile { pub struct ProgramFile { ast: Box, - source_manager: Arc, + source_manager: Arc, } /// Helper methods to interact with masm program file. @@ -125,7 +125,7 @@ impl ProgramFile { #[instrument(name = "read_program_file", skip(source_manager), fields(path = %path.as_ref().display()))] pub fn read_with( path: impl AsRef, - source_manager: Arc, + source_manager: Arc, ) -> Result { // parse the program into an AST let path = path.as_ref(); @@ -160,7 +160,7 @@ impl ProgramFile { } /// Returns the source manager for this program file. - pub fn source_manager(&self) -> &Arc { + pub fn source_manager(&self) -> &Arc { &self.source_manager } } diff --git a/test-utils/src/lib.rs b/test-utils/src/lib.rs index bf5cb70f1d..b515eb9f7a 100644 --- a/test-utils/src/lib.rs +++ b/test-utils/src/lib.rs @@ -171,7 +171,7 @@ macro_rules! assert_assembler_diagnostic { /// - Execution error test: check that running a program compiled from the given source causes an /// ExecutionError which contains the specified substring. pub struct Test { - pub source_manager: Arc, + pub source_manager: Arc, pub source: Arc, pub kernel_source: Option>, pub stack_inputs: StackInputs, From fe96f77b95d8d4e8c3ad7d9f11a342a26bef64e1 Mon Sep 17 00:00:00 2001 From: sergerad Date: Fri, 6 Jun 2025 06:16:03 +1200 Subject: [PATCH 4/8] SourceManagerSync --- assembly/src/assembler/mod.rs | 10 +++++----- assembly/src/lib.rs | 4 ++-- assembly/src/testing.rs | 8 ++++---- core/src/debuginfo/mod.rs | 2 +- core/src/debuginfo/source_manager.rs | 3 +++ miden/src/cli/data.rs | 6 +++--- test-utils/src/lib.rs | 6 ++++-- 7 files changed, 22 insertions(+), 17 deletions(-) diff --git a/assembly/src/assembler/mod.rs b/assembly/src/assembler/mod.rs index e48d733c8c..0bc93d3796 100644 --- a/assembly/src/assembler/mod.rs +++ b/assembly/src/assembler/mod.rs @@ -6,7 +6,7 @@ use module_graph::{ProcedureWrapper, WrappedModule}; use vm_core::{ AssemblyOp, Decorator, DecoratorList, Felt, Kernel, Operation, Program, WORD_SIZE, crypto::hash::RpoDigest, - debuginfo::SourceSpan, + debuginfo::{SourceManagerSync, SourceSpan}, mast::{DecoratorId, MastNodeId}, }; @@ -64,7 +64,7 @@ pub use self::{ #[derive(Clone)] pub struct Assembler { /// The source manager to use for compilation and source location information - source_manager: Arc, + source_manager: Arc, /// The global [ModuleGraph] for this assembler. module_graph: ModuleGraph, /// Whether to treat warning diagnostics as errors @@ -93,7 +93,7 @@ impl Default for Assembler { /// Constructors impl Assembler { /// Start building an [Assembler] - pub fn new(source_manager: Arc) -> Self { + pub fn new(source_manager: Arc) -> Self { let module_graph = ModuleGraph::new(source_manager.clone()); Self { source_manager, @@ -106,7 +106,7 @@ impl Assembler { /// Start building an [`Assembler`] with a kernel defined by the provided [KernelLibrary]. pub fn with_kernel( - source_manager: Arc, + source_manager: Arc, kernel_lib: KernelLibrary, ) -> Self { let (kernel, kernel_module, _) = kernel_lib.into_parts(); @@ -303,7 +303,7 @@ impl Assembler { } /// Returns a link to the source manager used by this assembler. - pub fn source_manager(&self) -> Arc { + pub fn source_manager(&self) -> Arc { self.source_manager.clone() } diff --git a/assembly/src/lib.rs b/assembly/src/lib.rs index 350199fee1..feba3f58f3 100644 --- a/assembly/src/lib.rs +++ b/assembly/src/lib.rs @@ -39,8 +39,8 @@ pub use self::{ assembler::Assembler, compile::{Compile, Options as CompileOptions}, diagnostics::{ - DefaultSourceManager, Report, SourceFile, SourceId, SourceManager, SourceSpan, Span, - Spanned, + DefaultSourceManager, Report, SourceFile, SourceId, SourceManager, SourceManagerSync, + SourceSpan, Span, Spanned, }, errors::AssemblyError, library::{ diff --git a/assembly/src/testing.rs b/assembly/src/testing.rs index 256f21c145..72a340ebad 100644 --- a/assembly/src/testing.rs +++ b/assembly/src/testing.rs @@ -1,7 +1,7 @@ use alloc::{boxed::Box, string::String, sync::Arc, vec::Vec}; use core::fmt; -use vm_core::Program; +use vm_core::{Program, debuginfo::SourceManagerSync}; #[cfg(feature = "std")] use crate::diagnostics::reporting::set_panic_hook; @@ -10,7 +10,7 @@ use crate::{ assembler::Assembler, ast::{Form, Module, ModuleKind}, diagnostics::{ - Report, SourceFile, SourceManager, + Report, SourceFile, reporting::{ReportHandlerOpts, set_hook}, }, library::Library, @@ -195,7 +195,7 @@ macro_rules! parse_module { /// /// Some of the assertion macros defined above require a [TestContext], so be aware of that. pub struct TestContext { - source_manager: Arc, + source_manager: Arc, assembler: Assembler, } @@ -233,7 +233,7 @@ impl TestContext { } #[inline(always)] - pub fn source_manager(&self) -> Arc { + pub fn source_manager(&self) -> Arc { self.source_manager.clone() } diff --git a/core/src/debuginfo/mod.rs b/core/src/debuginfo/mod.rs index 27772806de..52a11c7433 100644 --- a/core/src/debuginfo/mod.rs +++ b/core/src/debuginfo/mod.rs @@ -10,6 +10,6 @@ pub use self::{ source_file::{ ByteIndex, ByteOffset, ColumnIndex, LineIndex, SourceContent, SourceFile, SourceFileRef, }, - source_manager::{DefaultSourceManager, SourceId, SourceManager}, + source_manager::{DefaultSourceManager, SourceId, SourceManager, SourceManagerSync}, span::{SourceSpan, Span, Spanned}, }; diff --git a/core/src/debuginfo/source_manager.rs b/core/src/debuginfo/source_manager.rs index 77071b51e9..e3c72049e7 100644 --- a/core/src/debuginfo/source_manager.rs +++ b/core/src/debuginfo/source_manager.rs @@ -391,6 +391,9 @@ impl SourceManager for DefaultSourceManager { } } +pub trait SourceManagerSync: SourceManager + Send + Sync {} +impl SourceManagerSync for T {} + #[cfg(test)] mod error_assertions { use super::*; diff --git a/miden/src/cli/data.rs b/miden/src/cli/data.rs index 9b0c82ec39..ce5b6dce4e 100644 --- a/miden/src/cli/data.rs +++ b/miden/src/cli/data.rs @@ -109,7 +109,7 @@ impl OutputFile { pub struct ProgramFile { ast: Box, - source_manager: Arc, + source_manager: Arc, } /// Helper methods to interact with masm program file. @@ -125,7 +125,7 @@ impl ProgramFile { #[instrument(name = "read_program_file", skip(source_manager), fields(path = %path.as_ref().display()))] pub fn read_with( path: impl AsRef, - source_manager: Arc, + source_manager: Arc, ) -> Result { // parse the program into an AST let path = path.as_ref(); @@ -160,7 +160,7 @@ impl ProgramFile { } /// Returns the source manager for this program file. - pub fn source_manager(&self) -> &Arc { + pub fn source_manager(&self) -> &Arc { &self.source_manager } } diff --git a/test-utils/src/lib.rs b/test-utils/src/lib.rs index b515eb9f7a..bb913461a0 100644 --- a/test-utils/src/lib.rs +++ b/test-utils/src/lib.rs @@ -14,7 +14,9 @@ use alloc::{ }; use assembly::{KernelLibrary, Library}; -pub use assembly::{LibraryPath, SourceFile, SourceManager, diagnostics::Report}; +pub use assembly::{ + LibraryPath, SourceFile, SourceManager, SourceManagerSync, diagnostics::Report, +}; pub use pretty_assertions::{assert_eq, assert_ne, assert_str_eq}; pub use processor::{ AdviceInputs, AdviceProvider, ContextId, ExecutionError, ExecutionOptions, ExecutionTrace, @@ -171,7 +173,7 @@ macro_rules! assert_assembler_diagnostic { /// - Execution error test: check that running a program compiled from the given source causes an /// ExecutionError which contains the specified substring. pub struct Test { - pub source_manager: Arc, + pub source_manager: Arc, pub source: Arc, pub kernel_source: Option>, pub stack_inputs: StackInputs, From 39f9a505de5accca014ac6e1be2751ba9370ce8e Mon Sep 17 00:00:00 2001 From: sergerad Date: Fri, 6 Jun 2025 06:18:42 +1200 Subject: [PATCH 5/8] Fixes --- assembly/src/assembler/mod.rs | 5 +++-- assembly/src/testing.rs | 4 ++-- miden/src/cli/data.rs | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/assembly/src/assembler/mod.rs b/assembly/src/assembler/mod.rs index 0bc93d3796..e5a830a480 100644 --- a/assembly/src/assembler/mod.rs +++ b/assembly/src/assembler/mod.rs @@ -6,12 +6,13 @@ use module_graph::{ProcedureWrapper, WrappedModule}; use vm_core::{ AssemblyOp, Decorator, DecoratorList, Felt, Kernel, Operation, Program, WORD_SIZE, crypto::hash::RpoDigest, - debuginfo::{SourceManagerSync, SourceSpan}, + debuginfo::SourceSpan, mast::{DecoratorId, MastNodeId}, }; use crate::{ - AssemblyError, Compile, CompileOptions, LibraryNamespace, LibraryPath, SourceManager, Spanned, + AssemblyError, Compile, CompileOptions, LibraryNamespace, LibraryPath, SourceManager, + SourceManagerSync, Spanned, ast::{self, Export, InvocationTarget, InvokeKind, ModuleKind, QualifiedProcedureName}, diagnostics::Report, library::{KernelLibrary, Library}, diff --git a/assembly/src/testing.rs b/assembly/src/testing.rs index 72a340ebad..9cfb9e8996 100644 --- a/assembly/src/testing.rs +++ b/assembly/src/testing.rs @@ -1,7 +1,7 @@ use alloc::{boxed::Box, string::String, sync::Arc, vec::Vec}; use core::fmt; -use vm_core::{Program, debuginfo::SourceManagerSync}; +use vm_core::Program; #[cfg(feature = "std")] use crate::diagnostics::reporting::set_panic_hook; @@ -10,7 +10,7 @@ use crate::{ assembler::Assembler, ast::{Form, Module, ModuleKind}, diagnostics::{ - Report, SourceFile, + Report, SourceFile, SourceManagerSync, reporting::{ReportHandlerOpts, set_hook}, }, library::Library, diff --git a/miden/src/cli/data.rs b/miden/src/cli/data.rs index ce5b6dce4e..3e235a3b4d 100644 --- a/miden/src/cli/data.rs +++ b/miden/src/cli/data.rs @@ -109,7 +109,7 @@ impl OutputFile { pub struct ProgramFile { ast: Box, - source_manager: Arc, + source_manager: Arc, } /// Helper methods to interact with masm program file. @@ -125,7 +125,7 @@ impl ProgramFile { #[instrument(name = "read_program_file", skip(source_manager), fields(path = %path.as_ref().display()))] pub fn read_with( path: impl AsRef, - source_manager: Arc, + source_manager: Arc, ) -> Result { // parse the program into an AST let path = path.as_ref(); @@ -160,7 +160,7 @@ impl ProgramFile { } /// Returns the source manager for this program file. - pub fn source_manager(&self) -> &Arc { + pub fn source_manager(&self) -> &Arc { &self.source_manager } } From 5f87e7049b300c8c3b521671c5f6686e91af36df Mon Sep 17 00:00:00 2001 From: sergerad Date: Fri, 6 Jun 2025 06:21:36 +1200 Subject: [PATCH 6/8] Mv sync --- core/src/debuginfo/source_manager.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/debuginfo/source_manager.rs b/core/src/debuginfo/source_manager.rs index e3c72049e7..7a7234fe2b 100644 --- a/core/src/debuginfo/source_manager.rs +++ b/core/src/debuginfo/source_manager.rs @@ -101,6 +101,9 @@ impl SourceManagerError { } } +pub trait SourceManagerSync: SourceManager + Send + Sync {} +impl SourceManagerSync for T {} + pub trait SourceManager: Debug { /// Returns true if `file` is managed by this source manager fn is_manager_of(&self, file: &SourceFile) -> bool { @@ -391,9 +394,6 @@ impl SourceManager for DefaultSourceManager { } } -pub trait SourceManagerSync: SourceManager + Send + Sync {} -impl SourceManagerSync for T {} - #[cfg(test)] mod error_assertions { use super::*; From 6df3b986c908f87e728727ab8c013d6ef6eed703 Mon Sep 17 00:00:00 2001 From: sergerad Date: Fri, 6 Jun 2025 06:24:08 +1200 Subject: [PATCH 7/8] Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30038242f0..baf7f7547d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,8 @@ - Improve error messages for some assembler instruction (#1785) - Remove `idx` column from Kernel ROM chiplet and use chiplet bus for initialization. (#1818) -- [BREAKING] Make `SourceManager` a subtrait of `Send + Sync`. (#1822, #1858) +- [BREAKING] Make `Assembler::source_manager()` be `Send + Sync` (#1822) +- Add `SourceManagerSync` trait and generic implementation. (#1858) - Simplify and optimize the recursive verifier (#1801). - Simplify auxiliary randomness generation (#1810). - Add handling of variable length public inputs to the recursive verifier (#1813). From 6accc4460d99994d144871a66789a52c884f62ec Mon Sep 17 00:00:00 2001 From: sergerad Date: Fri, 6 Jun 2025 06:32:19 +1200 Subject: [PATCH 8/8] Lint --- miden/src/cli/data.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miden/src/cli/data.rs b/miden/src/cli/data.rs index 3e235a3b4d..f64b2bcbd4 100644 --- a/miden/src/cli/data.rs +++ b/miden/src/cli/data.rs @@ -6,7 +6,7 @@ use std::{ }; use assembly::{ - Assembler, Library, LibraryNamespace, SourceManager, + Assembler, Library, LibraryNamespace, SourceManagerSync, ast::{Module, ModuleKind}, diagnostics::{Report, WrapErr}, report,