refactor: combine Program.hints and Program.hints_ranges into custom collection

[PanGan21]

TITLE

refactor: combine Program.hints and Program.hints_ranges into custom collection

Description

Combines Program.hints and Program.hints_ranges into a new struct called hints_collection.
Closes #1342

Description of the pull request changes and motivation.

Checklist

• Linked to Github Issue
• Unit tests added
• Integration tests added.
• This change requires new documentation.
  • Documentation has been added/updated.
  • CHANGELOG has been updated.

[Oppen]

I'd suggest the following:

• The fields to be private;
• The struct to be pub(crate);
• Use a constructor taking a HashMap;
• Use a getter taking a pc and returning a slice.

[PanGan21]

Hi! Thanks for the suggestions! Just a short question.
The getter you are describing works fine for this case:

let hint_data = self 
     .program 
     .shared_program_data 
     .hints_ranges 
     .get(vm.run_context.pc.offset) 
     .and_then(|r| r.and_then(|(s, l)| hint_data.get(s..s + l.get()))) 
     .unwrap_or(&[]); 

But cases like this:

self.program
            .shared_program_data
            .hints_collection
            .hints
            .iter()
            .map(|hint| {
                hint_executor
                    .compile_hint(
                        &hint.code,
                        &hint.flow_tracking_data.ap_tracking,
                        &hint.flow_tracking_data.reference_ids,
                        references,
                    )
                    .map_err(|_| VirtualMachineError::CompileHintFail(hint.code.clone().into()))
            })
            .collect()

It seems a getter that returns all the hints is required. Let me know if I am wrong :)

[Oppen]

You can make a method that returns the iterator instead:

fn iter(&self) -> impl Iterator<Item = Option<(pc, &[HintData])>> {
    self.hint_ranges.iter().enumerate().filter_map(|(pc, range)| {
        let Some(range) = range else {
              return None;
        }
        Some((pc, self.hint_data[range.start..range.start+range.len]))
    })
}

Consider this pseudo-code, you'll probably need to make some changes to it.

[Oppen]

You could also implement some fn iter_hints(&self) -> impl Iterator<Item = &HintData> if it makes this easier.

[PanGan21]

Unfortunatelly I was forced to implement both iter_hints and iter. The iter function is used only for testing in the helper get_hints_as_map thus it has #[allow(dead_code)]. Let me know your thoughts

[Oppen]

You can avoid the #[allow(dead_code)] by using a separate impl block that is gated by the #[cfg(test)] annotation.

[PanGan21]

Added the new impl block! Thanks!

[codecov]

Codecov Report

Merging #1366 (9b716c5) into main (4d36f96) will decrease coverage by 0.01%.
The diff coverage is 98.01%.

@@            Coverage Diff             @@
##             main    #1366      +/-   ##
==========================================
- Coverage   97.49%   97.48%   -0.01%     
==========================================
  Files          93       93              
  Lines       37924    37960      +36     
==========================================
+ Hits        36973    37007      +34     
- Misses        951      953       +2     

Files Changed	Coverage Δ
vm/src/vm/runners/cairo_runner.rs	97.91% <92.30%> (-0.03%)	⬇️
vm/src/types/program.rs	99.44% <98.66%> (-0.10%)	⬇️
vm/src/serde/deserialize_program.rs	97.32% <100.00%> (-0.02%)	⬇️
vm/src/utils.rs	99.49% <100.00%> (-0.01%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[MegaRedHand]

LGTM