Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
- [BREAKING] Removed `Process`, `VmStateIterator` and `miden_processor::execute_iter()` ([#2483](https://github.com/0xMiden/miden-vm/pull/2483)).
- [BREAKING] Removed `miden debug`, `miden analyze` and `miden repl` ([#2483](https://github.com/0xMiden/miden-vm/pull/2483)).
- [BREAKING] Change backend from winterfell to Plonky3 ([#2472](https://github.com/0xMiden/miden-vm/pull/2472)).
- Fix issue where calling `strip_decorators()` would panic on further decorator access ([#2524](https://github.com/0xMiden/miden-vm/pull/2524)).

## 0.20.1 (2025-12-14)

Expand Down
34 changes: 30 additions & 4 deletions core/src/mast/debuginfo/decorator_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,38 @@ impl OpToDecoratorIds {
/// - Last value in `node_indptr_for_op_idx` must be <= `op_indptr_for_dec_ids.len() - 1`
/// - Both `op_indptr_for_dec_ids` and `node_indptr_for_op_idx` must be strictly monotonic (each
/// successive value must be >= the previous one)
#[cfg(test)]
pub fn from_components(
pub(crate) fn from_components(
decorator_ids: Vec<DecoratorId>,
op_indptr_for_dec_ids: Vec<usize>,
node_indptr_for_op_idx: IndexVec<MastNodeId, usize>,
) -> Result<Self, DecoratorIndexError> {
// Completely empty structures are valid (no nodes, no decorators)
if decorator_ids.is_empty()
&& op_indptr_for_dec_ids.is_empty()
&& node_indptr_for_op_idx.is_empty()
{
return Ok(Self {
decorator_ids,
op_indptr_for_dec_ids,
node_indptr_for_op_idx,
});
}

// Structures with nodes but no decorators are also valid
// (empty decorator_ids and op_indptr, but non-empty node_indptr with all zeros)
if decorator_ids.is_empty() && op_indptr_for_dec_ids.is_empty() {
// Verify all node pointers are 0 (pointing to empty range)
if node_indptr_for_op_idx.iter().all(|&ptr| ptr == 0) {
return Ok(Self {
decorator_ids,
op_indptr_for_dec_ids,
node_indptr_for_op_idx,
});
} else {
return Err(DecoratorIndexError::InternalStructure);
}
}

// Validate the structure
if op_indptr_for_dec_ids.is_empty() {
return Err(DecoratorIndexError::InternalStructure);
Expand Down Expand Up @@ -729,9 +755,9 @@ mod tests {

#[test]
fn test_from_components_invalid_structure() {
// Test with empty operation pointers
// Completely empty structures are now valid (no nodes, no decorators)
let result = OpToDecoratorIds::from_components(vec![], vec![], IndexVec::new());
assert_eq!(result, Err(DecoratorIndexError::InternalStructure));
assert!(result.is_ok(), "Empty structures should be valid");

// Test with operation pointer exceeding decorator indices
let result = OpToDecoratorIds::from_components(
Expand Down
22 changes: 22 additions & 0 deletions core/src/mast/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,28 @@ impl DebugInfo {
}
}

/// Creates an empty [DebugInfo] with valid CSR structures for N nodes.
///
/// Used when stripping decorators while keeping CSR structure valid.
pub fn empty_for_nodes(num_nodes: usize) -> Self {
// For N nodes, CSR needs N+1 zero entries
let mut node_indptr_for_op_idx = IndexVec::new();
for _ in 0..=num_nodes {
let _ = node_indptr_for_op_idx.push(0);
}
Comment on lines +117 to +120
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just initialize the vectors with N+1 zeros, instead of pushing them one by one.


let op_decorator_storage =
OpToDecoratorIds::from_components(Vec::new(), Vec::new(), node_indptr_for_op_idx)
.expect("Empty CSR structure should be valid");

Self {
decorators: IndexVec::new(),
op_decorator_storage,
node_decorator_storage: NodeToDecoratorIds::new(),
error_codes: BTreeMap::new(),
}
}

// PUBLIC ACCESSORS
// --------------------------------------------------------------------------------------------

Expand Down
19 changes: 3 additions & 16 deletions core/src/mast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,24 +163,11 @@ impl MastForest {
id_remappings
}

/// Removes all decorators from this MAST forest.
/// Removes all decorators and error codes while keeping CSR structure valid.
///
/// This method modifies the forest in-place, removing all decorator information
/// including operation-indexed decorators, before-enter decorators, after-exit
/// decorators, and error codes.
///
/// # Examples
///
/// ```rust
/// use miden_core::mast::MastForest;
///
/// let mut forest = MastForest::new();
/// // Add decorators and nodes to the forest
/// forest.strip_decorators(); // forest is now stripped
/// ```
/// Useful for performance benchmarking without decorator overhead.
pub fn strip_decorators(&mut self) {
// Clear all debug info (decorators and error codes)
self.debug_info.clear();
self.debug_info = DebugInfo::empty_for_nodes(self.nodes.len());
}
Comment on lines 169 to 171
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not for this PR, but we should probably rename this into clear_debug_info() or something like that because, AFAICT, this is also removing error messages and in the future other things as well (e.g., see #2474).

Also, I still don't fully understand why we need to preserve any info in the debug info upon clearing it. The PR description mentions some code paths that rely on this - what are these code paths? Basically, is is something we can change after #2470 is merged?

I think the ideal end state would be to have debug_info in MastForest be an optional field (i.e., debug_info: Option<DebugInfo>) - what is preventing us from doing this?


/// Compacts the forest by merging duplicate nodes.
Expand Down
66 changes: 44 additions & 22 deletions core/src/mast/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,40 +241,62 @@ fn test_decorator_storage_consistency_with_multiple_blocks() {
fn test_decorator_storage_after_strip_decorators() {
let mut forest = MastForest::new();

// Create decorators
let deco1 = forest.add_decorator(Decorator::Trace(1)).unwrap();
let deco2 = forest.add_decorator(Decorator::Trace(2)).unwrap();

// Create operations and decorators
let operations = vec![Operation::Push(Felt::new(1)), Operation::Add];
let decorators = vec![(0, deco1), (1, deco2)];

// Add block to forest
let block_id = BasicBlockNodeBuilder::new(operations, decorators)
let block_id = BasicBlockNodeBuilder::new(operations, vec![(0, deco1), (1, deco2)])
.add_to_forest(&mut forest)
.unwrap();

// Verify decorators exist initially
assert!(!forest.debug_info.op_decorator_storage().is_empty());
assert_eq!(forest.debug_info.op_decorator_storage().num_nodes(), 1);
assert_eq!(forest.debug_info.num_decorators(), 2);
assert_eq!(forest.debug_info.op_decorator_storage().num_decorator_ids(), 2);

// Strip decorators
forest.strip_decorators();

// Verify decorators are cleared from storage
assert!(forest.debug_info.op_decorator_storage().is_empty());
assert_eq!(forest.debug_info.num_decorators(), 0);
assert_eq!(forest.debug_info.op_decorator_storage().num_nodes(), 1);
assert!(forest.decorator_links_for_node(block_id).unwrap().into_iter().next().is_none());
assert!(forest.get_assembly_op(block_id, Some(0)).is_none());
}

#[test]
fn test_strip_decorators_edge_cases() {
// Empty forest
let mut forest = MastForest::new();
forest.strip_decorators();
assert_eq!(forest.debug_info.num_decorators(), 0);
assert_eq!(forest.debug_info.op_decorator_storage().num_nodes(), 0);
assert_eq!(forest.debug_info.op_decorator_storage().num_decorator_ids(), 0);

// Verify block also has no decorators after stripping
let block = if let crate::mast::MastNode::Block(block) = &forest[block_id] {
block
} else {
panic!("Expected a block node");
};
let block_decorators: Vec<_> = block.indexed_decorator_iter(&forest).collect();
assert_eq!(block_decorators, []);
// Idempotent: stripping twice should be safe
let operations = vec![Operation::Push(Felt::new(1)), Operation::Add];
let block_id = BasicBlockNodeBuilder::new(operations, vec![])
.add_to_forest(&mut forest)
.unwrap();
forest.strip_decorators();
forest.strip_decorators();
assert_eq!(forest.debug_info.num_decorators(), 0);
assert_eq!(forest.debug_info.op_decorator_storage().num_nodes(), 1);
assert!(forest.decorator_links_for_node(block_id).unwrap().into_iter().next().is_none());
}

#[test]
fn test_strip_decorators_multiple_node_types() {
let mut forest = MastForest::new();
let deco = forest.add_decorator(Decorator::Trace(1)).unwrap();
let block_id = BasicBlockNodeBuilder::new(
vec![Operation::Push(Felt::new(1)), Operation::Add],
vec![(0, deco)],
)
.add_to_forest(&mut forest)
.unwrap();

JoinNodeBuilder::new([block_id, block_id]).add_to_forest(&mut forest).unwrap();
SplitNodeBuilder::new([block_id, block_id]).add_to_forest(&mut forest).unwrap();

forest.strip_decorators();

assert_eq!(forest.debug_info.op_decorator_storage().num_nodes(), 3);
assert!(forest.decorator_links_for_node(block_id).unwrap().into_iter().next().is_none());
}

#[test]
Expand Down