Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double unwrap for program_base in Cairo runner initialize_state #1638

Closed
tcoratger opened this issue Feb 26, 2024 · 0 comments · Fixed by #1657
Closed

Double unwrap for program_base in Cairo runner initialize_state #1638

tcoratger opened this issue Feb 26, 2024 · 0 comments · Fixed by #1657
Labels
enhancement New feature or request

Comments

@tcoratger
Copy link
Contributor

Issue Description

In the initialize_state function of the CairoRunner structure, there is redundant code in my opinion, that can be improved for clarity and efficiency.

At the beginning of the function, there is a conditional check (if let Some(prog_base) = self.program_base) to ensure that self.program_base is not None.

if let Some(prog_base) = self.program_base {
let initial_pc = Relocatable {
segment_index: prog_base.segment_index,
offset: prog_base.offset + entrypoint,
};
self.initial_pc = Some(initial_pc);
vm.segments
.load_data(prog_base, &self.program.shared_program_data.data)
.map_err(RunnerError::MemoryInitializationError)?;
// Mark all addresses from the program segment as accessed
let base = self
.program_base
.unwrap_or_else(|| Relocatable::from((0, 0)));
for i in 0..self.program.shared_program_data.data.len() {
vm.segments.memory.mark_as_accessed((base + i)?);
}
}

However, a few lines later, the unwrap method is used again on self.program_base to obtain base, and in case of None, it sets base to Relocatable::from((0, 0)).

let base = self
.program_base
.unwrap_or_else(|| Relocatable::from((0, 0)));

This redundant check and assignment can be eliminated because the None case will never happen inside the if statement, making the base declaration unnecessary. Instead, we can directly use the prog_base variable in the loop to mark memory segments as accessed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant