From d0c001db3ffc208231c17672c39d086ad5851b28 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Wed, 30 Oct 2024 15:19:53 -0600 Subject: [PATCH 1/9] Convert relocation table to HashMap --- vm/src/vm/vm_core.rs | 14 +++++++++++ vm/src/vm/vm_memory/memory.rs | 47 ++++++++++++++++++++++++++++------- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index a65014a7b3..803c5c9c70 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -1012,6 +1012,20 @@ impl VirtualMachine { self.segments.memory.add_relocation_rule(src_ptr, dst_ptr) } + /// Add a new relocation rule. + /// + /// Will return an error if any of the following conditions are not met: + /// - Source address's segment must be negative (temporary). + /// - Source address's offset must be zero. + /// - There shouldn't already be relocation at the source segment. + pub fn add_relocation_rule_maybe_relocatable( + &mut self, + src_ptr: Relocatable, + dst: MaybeRelocatable, + ) -> Result<(), MemoryError> { + self.segments.memory.add_relocation_rule_maybe_relocatable(src_ptr, dst) + } + pub fn gen_arg(&mut self, arg: &dyn Any) -> Result { self.segments.gen_arg(arg) } diff --git a/vm/src/vm/vm_memory/memory.rs b/vm/src/vm/vm_memory/memory.rs index 9dbfc9a790..0fdcbbadf0 100644 --- a/vm/src/vm/vm_memory/memory.rs +++ b/vm/src/vm/vm_memory/memory.rs @@ -161,7 +161,7 @@ pub struct Memory { pub(crate) temp_data: Vec>, // relocation_rules's keys map to temp_data's indices and therefore begin at // zero; that is, segment_index = -1 maps to key 0, -2 to key 1... - pub(crate) relocation_rules: HashMap, + pub(crate) relocation_rules: HashMap, pub validated_addresses: AddressSet, validation_rules: Vec>, } @@ -249,13 +249,16 @@ impl Memory { // Version of Memory.relocate_value() that doesn't require a self reference fn relocate_address( addr: Relocatable, - relocation_rules: &HashMap, + relocation_rules: &HashMap, ) -> Result { if addr.segment_index < 0 { // Adjust the segment index to begin at zero, as per the struct field's // comment. if let Some(x) = relocation_rules.get(&(-(addr.segment_index + 1) as usize)) { - return Ok((*x + addr.offset)?.into()); + return Ok(match x { + MaybeRelocatable::RelocatableValue(r) => (*r + addr.offset)?.into(), + MaybeRelocatable::Int(i) => i.into(), + }); } } Ok(addr.into()) @@ -289,6 +292,14 @@ impl Memory { for index in (0..self.temp_data.len()).rev() { if let Some(base_addr) = self.relocation_rules.get(&index) { let data_segment = self.temp_data.remove(index); + + let base_addr = match base_addr { + MaybeRelocatable::RelocatableValue(addr) => addr, + MaybeRelocatable::Int(_) => { + continue; + }, + }; + // Insert the to-be relocated segment into the real memory let mut addr = *base_addr; if let Some(s) = self.data.get_mut(addr.segment_index as usize) { @@ -310,7 +321,6 @@ impl Memory { self.relocation_rules.clear(); Ok(()) } - /// Add a new relocation rule. /// /// Will return an error if any of the following conditions are not met: @@ -321,6 +331,20 @@ impl Memory { &mut self, src_ptr: Relocatable, dst_ptr: Relocatable, + ) -> Result<(), MemoryError> { + self.add_relocation_rule_maybe_relocatable(src_ptr, MaybeRelocatable::RelocatableValue(dst_ptr)) + } + + /// Add a new relocation rule. + /// + /// Will return an error if any of the following conditions are not met: + /// - Source address's segment must be negative (temporary). + /// - Source address's offset must be zero. + /// - There shouldn't already be relocation at the source segment. + pub(crate) fn add_relocation_rule_maybe_relocatable( + &mut self, + src_ptr: Relocatable, + dst: MaybeRelocatable, ) -> Result<(), MemoryError> { if src_ptr.segment_index >= 0 { return Err(MemoryError::AddressNotInTemporarySegment( @@ -338,7 +362,7 @@ impl Memory { return Err(MemoryError::DuplicatedRelocation(src_ptr.segment_index)); } - self.relocation_rules.insert(segment_index, dst_ptr); + self.relocation_rules.insert(segment_index, dst); Ok(()) } @@ -646,8 +670,8 @@ pub(crate) trait RelocateValue<'a, Input: 'a, Output: 'a> { fn relocate_value(&self, value: Input) -> Result; } -impl RelocateValue<'_, Relocatable, Relocatable> for Memory { - fn relocate_value(&self, addr: Relocatable) -> Result { +impl RelocateValue<'_, Relocatable, MaybeRelocatable> for Memory { + fn relocate_value(&self, addr: Relocatable) -> Result { if addr.segment_index < 0 { // Adjust the segment index to begin at zero, as per the struct field's // comment. @@ -655,10 +679,15 @@ impl RelocateValue<'_, Relocatable, Relocatable> for Memory { .relocation_rules .get(&(-(addr.segment_index + 1) as usize)) { - return (*x + addr.offset).map_err(MemoryError::Math); + return Ok(match x { + MaybeRelocatable::RelocatableValue(r) => { + (*r + addr.offset).map_err(MemoryError::Math)?.into() + }, + MaybeRelocatable::Int(i) => i.into(), + }); } } - Ok(addr) + Ok(addr.into()) } } From 3a40ec50d449eb142db145aaf0efbe2c992c167a Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Thu, 31 Oct 2024 08:59:10 -0600 Subject: [PATCH 2/9] Update CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1595eb1efc..0231a38a4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ #### [2.0.0-rc0] - 2024-10-22 +* fix: [#1862](https://github.com/lambdaclass/cairo-vm/pull/1862): + * Use MaybeRelocatable for relocation table + * chore: bump `cairo-lang-` dependencies to 2.9.0-dev.0 [#1858](https://github.com/lambdaclass/cairo-vm/pull/1858/files) * chore: update Rust required version to 1.81.0 [#1857](https://github.com/lambdaclass/cairo-vm/pull/1857) From 1def9152636c27678cb58b5af2d9ff2c0ea2387c Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Thu, 31 Oct 2024 09:05:27 -0600 Subject: [PATCH 3/9] Add documentation --- vm/src/vm/vm_core.rs | 6 +++++- vm/src/vm/vm_memory/memory.rs | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index 803c5c9c70..bd257a416d 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -1012,7 +1012,11 @@ impl VirtualMachine { self.segments.memory.add_relocation_rule(src_ptr, dst_ptr) } - /// Add a new relocation rule. + /// Add a new relocation rule, allowing for dst to be a `MaybeRelocatable`. + /// + /// Relocating memory to anything other than a `Relocatable` is generally not useful, but it + /// does make the implementation consistent with the pythonic version. In most cases it is + /// advisable to use `add_relocation_rule()`, which only accepts a Relocatable. /// /// Will return an error if any of the following conditions are not met: /// - Source address's segment must be negative (temporary). diff --git a/vm/src/vm/vm_memory/memory.rs b/vm/src/vm/vm_memory/memory.rs index 0fdcbbadf0..ad190224c1 100644 --- a/vm/src/vm/vm_memory/memory.rs +++ b/vm/src/vm/vm_memory/memory.rs @@ -337,6 +337,10 @@ impl Memory { /// Add a new relocation rule. /// + /// Relocating memory to anything other than a `Relocatable` is generally not useful, but it + /// does make the implementation consistent with the pythonic version. In most cases it is + /// advisable to use `add_relocation_rule()`, which only accepts a Relocatable. + /// /// Will return an error if any of the following conditions are not met: /// - Source address's segment must be negative (temporary). /// - Source address's offset must be zero. From e87cd135b091f08cb98e16b75a119a9c1f7b9159 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Thu, 31 Oct 2024 10:08:19 -0600 Subject: [PATCH 4/9] Add a couple basic tests --- vm/src/vm/vm_memory/memory.rs | 56 +++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/vm/src/vm/vm_memory/memory.rs b/vm/src/vm/vm_memory/memory.rs index ad190224c1..9b5bd55ed2 100644 --- a/vm/src/vm/vm_memory/memory.rs +++ b/vm/src/vm/vm_memory/memory.rs @@ -1650,6 +1650,62 @@ mod memory_tests { ); } + #[test] + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] + fn relocate_address_to_integer() { + let mut memory = Memory::new(); + memory + .add_relocation_rule_maybe_relocatable((-1, 0).into(), 0.into()) + .unwrap(); + memory + .add_relocation_rule_maybe_relocatable((-2, 0).into(), 42.into()) + .unwrap(); + + assert_eq!( + Memory::relocate_address((-1, 0).into(), &memory.relocation_rules).unwrap(), + MaybeRelocatable::Int(0.into()), + ); + assert_eq!( + Memory::relocate_address((-2, 0).into(), &memory.relocation_rules).unwrap(), + MaybeRelocatable::Int(42.into()), + ); + } + + #[test] + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] + fn relocate_address_integer_no_duplicates() { + let mut memory = Memory::new(); + memory + .add_relocation_rule_maybe_relocatable((-1, 0).into(), 1.into()) + .unwrap(); + assert_eq!( + memory.add_relocation_rule_maybe_relocatable((-1, 0).into(), 42.into()), + Err(MemoryError::DuplicatedRelocation(-1)) + ); + assert_eq!( + memory.add_relocation_rule_maybe_relocatable((-1, 0).into(), (2, 0).into()), + Err(MemoryError::DuplicatedRelocation(-1)) + ); + + assert_eq!( + Memory::relocate_address((-1, 0).into(), &memory.relocation_rules).unwrap(), + MaybeRelocatable::Int(1.into()), + ); + + memory + .add_relocation_rule_maybe_relocatable((-2, 0).into(), (3, 0).into()) + .unwrap(); + assert_eq!( + memory.add_relocation_rule_maybe_relocatable((-2, 0).into(), 1.into()), + Err(MemoryError::DuplicatedRelocation(-2)) + ); + + assert_eq!( + Memory::relocate_address((-2, 0).into(), &memory.relocation_rules).unwrap(), + MaybeRelocatable::RelocatableValue((3, 0).into()), + ); + } + #[test] fn mark_address_as_accessed() { let mut memory = memory![((0, 0), 0)]; From cf36b4a7879982a001ce38c0f5403ae0c7b4c564 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Mon, 11 Nov 2024 14:35:01 -0700 Subject: [PATCH 5/9] First pass at hiding relocation table mods behind extensive_hints --- .../builtin_hint_processor/segments.rs | 3 + .../cairo_1_hint_processor/dict_manager.rs | 2 +- vm/src/vm/vm_core.rs | 25 ++----- vm/src/vm/vm_memory/memory.rs | 74 +++++++++++++++---- 4 files changed, 72 insertions(+), 32 deletions(-) diff --git a/vm/src/hint_processor/builtin_hint_processor/segments.rs b/vm/src/hint_processor/builtin_hint_processor/segments.rs index c996c2f9b1..bc95c6c1ba 100644 --- a/vm/src/hint_processor/builtin_hint_processor/segments.rs +++ b/vm/src/hint_processor/builtin_hint_processor/segments.rs @@ -19,7 +19,10 @@ pub fn relocate_segment( ap_tracking: &ApTracking, ) -> Result<(), HintError> { let src_ptr = get_ptr_from_var_name("src_ptr", vm, ids_data, ap_tracking)?; + #[cfg(not(feature = "extensive_hints"))] let dest_ptr = get_ptr_from_var_name("dest_ptr", vm, ids_data, ap_tracking)?; + #[cfg(feature = "extensive_hints")] + let dest_ptr = crate::hint_processor::builtin_hint_processor::hint_utils::get_maybe_relocatable_from_var_name("dest_ptr", vm, ids_data, ap_tracking)?; vm.add_relocation_rule(src_ptr, dest_ptr) .map_err(HintError::Memory)?; diff --git a/vm/src/hint_processor/cairo_1_hint_processor/dict_manager.rs b/vm/src/hint_processor/cairo_1_hint_processor/dict_manager.rs index a82a4fa2db..f3fe2fb9a0 100644 --- a/vm/src/hint_processor/cairo_1_hint_processor/dict_manager.rs +++ b/vm/src/hint_processor/cairo_1_hint_processor/dict_manager.rs @@ -126,7 +126,7 @@ impl DictManagerExecScope { if self.use_temporary_segments { let mut prev_end = vm.add_memory_segment(); for tracker in &self.trackers { - vm.add_relocation_rule(tracker.start, prev_end)?; + vm.add_relocation_rule(tracker.start, prev_end.into())?; prev_end += (tracker.end.unwrap_or_default() - tracker.start)?; prev_end += 1; } diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index bd257a416d..6069f3d09c 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -1000,6 +1000,10 @@ impl VirtualMachine { /// Add a new relocation rule. /// + /// When using feature "extensive_hints" the destination is allowed to be an Integer (via + /// MaybeRelocatable). Relocating memory to anything other than a `Relocatable` is generally + /// not useful, but it does make the implementation consistent with the pythonic version. + /// /// Will return an error if any of the following conditions are not met: /// - Source address's segment must be negative (temporary). /// - Source address's offset must be zero. @@ -1007,29 +1011,14 @@ impl VirtualMachine { pub fn add_relocation_rule( &mut self, src_ptr: Relocatable, + #[cfg(not(feature = "extensive_hints"))] dst_ptr: Relocatable, + #[cfg(feature = "extensive_hints")] + dst_ptr: MaybeRelocatable, ) -> Result<(), MemoryError> { self.segments.memory.add_relocation_rule(src_ptr, dst_ptr) } - /// Add a new relocation rule, allowing for dst to be a `MaybeRelocatable`. - /// - /// Relocating memory to anything other than a `Relocatable` is generally not useful, but it - /// does make the implementation consistent with the pythonic version. In most cases it is - /// advisable to use `add_relocation_rule()`, which only accepts a Relocatable. - /// - /// Will return an error if any of the following conditions are not met: - /// - Source address's segment must be negative (temporary). - /// - Source address's offset must be zero. - /// - There shouldn't already be relocation at the source segment. - pub fn add_relocation_rule_maybe_relocatable( - &mut self, - src_ptr: Relocatable, - dst: MaybeRelocatable, - ) -> Result<(), MemoryError> { - self.segments.memory.add_relocation_rule_maybe_relocatable(src_ptr, dst) - } - pub fn gen_arg(&mut self, arg: &dyn Any) -> Result { self.segments.gen_arg(arg) } diff --git a/vm/src/vm/vm_memory/memory.rs b/vm/src/vm/vm_memory/memory.rs index 9b5bd55ed2..1f98ee62f4 100644 --- a/vm/src/vm/vm_memory/memory.rs +++ b/vm/src/vm/vm_memory/memory.rs @@ -161,6 +161,9 @@ pub struct Memory { pub(crate) temp_data: Vec>, // relocation_rules's keys map to temp_data's indices and therefore begin at // zero; that is, segment_index = -1 maps to key 0, -2 to key 1... + #[cfg(not(feature = "extensive_hints"))] + pub(crate) relocation_rules: HashMap, + #[cfg(feature = "extensive_hints")] pub(crate) relocation_rules: HashMap, pub validated_addresses: AddressSet, validation_rules: Vec>, @@ -247,6 +250,21 @@ impl Memory { } // Version of Memory.relocate_value() that doesn't require a self reference + #[cfg(not(feature = "extensive_hints"))] + fn relocate_address( + addr: Relocatable, + relocation_rules: &HashMap, + ) -> Result { + if addr.segment_index < 0 { + // Adjust the segment index to begin at zero, as per the struct field's + // comment. + if let Some(x) = relocation_rules.get(&(-(addr.segment_index + 1) as usize)) { + return Ok((*x + addr.offset)?.into()); + } + } + Ok(addr.into()) + } + #[cfg(feature = "extensive_hints")] fn relocate_address( addr: Relocatable, relocation_rules: &HashMap, @@ -293,6 +311,7 @@ impl Memory { if let Some(base_addr) = self.relocation_rules.get(&index) { let data_segment = self.temp_data.remove(index); + #[cfg(feature = "extensive_hints")] let base_addr = match base_addr { MaybeRelocatable::RelocatableValue(addr) => addr, MaybeRelocatable::Int(_) => { @@ -323,29 +342,41 @@ impl Memory { } /// Add a new relocation rule. /// + /// When using feature "extensive_hints" the destination is allowed to be an Integer (via + /// MaybeRelocatable). Relocating memory to anything other than a `Relocatable` is generally + /// not useful, but it does make the implementation consistent with the pythonic version. + /// /// Will return an error if any of the following conditions are not met: /// - Source address's segment must be negative (temporary). /// - Source address's offset must be zero. /// - There shouldn't already be relocation at the source segment. + #[cfg(not(feature = "extensive_hints"))] pub(crate) fn add_relocation_rule( &mut self, src_ptr: Relocatable, dst_ptr: Relocatable, ) -> Result<(), MemoryError> { - self.add_relocation_rule_maybe_relocatable(src_ptr, MaybeRelocatable::RelocatableValue(dst_ptr)) - } + if src_ptr.segment_index >= 0 { + return Err(MemoryError::AddressNotInTemporarySegment( + src_ptr.segment_index, + )); + } + if src_ptr.offset != 0 { + return Err(MemoryError::NonZeroOffset(src_ptr.offset)); + } - /// Add a new relocation rule. - /// - /// Relocating memory to anything other than a `Relocatable` is generally not useful, but it - /// does make the implementation consistent with the pythonic version. In most cases it is - /// advisable to use `add_relocation_rule()`, which only accepts a Relocatable. - /// - /// Will return an error if any of the following conditions are not met: - /// - Source address's segment must be negative (temporary). - /// - Source address's offset must be zero. - /// - There shouldn't already be relocation at the source segment. - pub(crate) fn add_relocation_rule_maybe_relocatable( + // Adjust the segment index to begin at zero, as per the struct field's + // comment. + let segment_index = -(src_ptr.segment_index + 1) as usize; + if self.relocation_rules.contains_key(&segment_index) { + return Err(MemoryError::DuplicatedRelocation(src_ptr.segment_index)); + } + + self.relocation_rules.insert(segment_index, dst_ptr); + Ok(()) + } + #[cfg(feature = "extensive_hints")] + pub(crate) fn add_relocation_rule( &mut self, src_ptr: Relocatable, dst: MaybeRelocatable, @@ -674,6 +705,23 @@ pub(crate) trait RelocateValue<'a, Input: 'a, Output: 'a> { fn relocate_value(&self, value: Input) -> Result; } +#[cfg(not(feature = "extensive_hints"))] +impl RelocateValue<'_, Relocatable, Relocatable> for Memory { + fn relocate_value(&self, addr: Relocatable) -> Result { + if addr.segment_index < 0 { + // Adjust the segment index to begin at zero, as per the struct field's + // comment. + if let Some(x) = self + .relocation_rules + .get(&(-(addr.segment_index + 1) as usize)) + { + return (*x + addr.offset).map_err(MemoryError::Math); + } + } + Ok(addr) + } +} +#[cfg(feature = "extensive_hints")] impl RelocateValue<'_, Relocatable, MaybeRelocatable> for Memory { fn relocate_value(&self, addr: Relocatable) -> Result { if addr.segment_index < 0 { From a8a8640fb752f57a5b52ac6617bd07826bea82cf Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Thu, 21 Nov 2024 10:36:39 -0700 Subject: [PATCH 6/9] fmt --- vm/src/vm/vm_core.rs | 6 ++---- vm/src/vm/vm_memory/memory.rs | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index 6069f3d09c..5862aee093 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -1011,10 +1011,8 @@ impl VirtualMachine { pub fn add_relocation_rule( &mut self, src_ptr: Relocatable, - #[cfg(not(feature = "extensive_hints"))] - dst_ptr: Relocatable, - #[cfg(feature = "extensive_hints")] - dst_ptr: MaybeRelocatable, + #[cfg(not(feature = "extensive_hints"))] dst_ptr: Relocatable, + #[cfg(feature = "extensive_hints")] dst_ptr: MaybeRelocatable, ) -> Result<(), MemoryError> { self.segments.memory.add_relocation_rule(src_ptr, dst_ptr) } diff --git a/vm/src/vm/vm_memory/memory.rs b/vm/src/vm/vm_memory/memory.rs index 1f98ee62f4..2df98d7b69 100644 --- a/vm/src/vm/vm_memory/memory.rs +++ b/vm/src/vm/vm_memory/memory.rs @@ -316,7 +316,7 @@ impl Memory { MaybeRelocatable::RelocatableValue(addr) => addr, MaybeRelocatable::Int(_) => { continue; - }, + } }; // Insert the to-be relocated segment into the real memory @@ -734,7 +734,7 @@ impl RelocateValue<'_, Relocatable, MaybeRelocatable> for Memory { return Ok(match x { MaybeRelocatable::RelocatableValue(r) => { (*r + addr.offset).map_err(MemoryError::Math)?.into() - }, + } MaybeRelocatable::Int(i) => i.into(), }); } From e124462cb8b273c6d20e8434864039edde4c2276 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Thu, 21 Nov 2024 11:28:50 -0700 Subject: [PATCH 7/9] Run tests only for feature = "extensive_hints" --- vm/src/vm/vm_memory/memory.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/vm/src/vm/vm_memory/memory.rs b/vm/src/vm/vm_memory/memory.rs index 2df98d7b69..b8cc13b0ca 100644 --- a/vm/src/vm/vm_memory/memory.rs +++ b/vm/src/vm/vm_memory/memory.rs @@ -1700,13 +1700,14 @@ mod memory_tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] + #[cfg(feature = "extensive_hints")] fn relocate_address_to_integer() { let mut memory = Memory::new(); memory - .add_relocation_rule_maybe_relocatable((-1, 0).into(), 0.into()) + .add_relocation_rule((-1, 0).into(), 0.into()) .unwrap(); memory - .add_relocation_rule_maybe_relocatable((-2, 0).into(), 42.into()) + .add_relocation_rule((-2, 0).into(), 42.into()) .unwrap(); assert_eq!( @@ -1721,17 +1722,18 @@ mod memory_tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] + #[cfg(feature = "extensive_hints")] fn relocate_address_integer_no_duplicates() { let mut memory = Memory::new(); memory - .add_relocation_rule_maybe_relocatable((-1, 0).into(), 1.into()) + .add_relocation_rule((-1, 0).into(), 1.into()) .unwrap(); assert_eq!( - memory.add_relocation_rule_maybe_relocatable((-1, 0).into(), 42.into()), + memory.add_relocation_rule((-1, 0).into(), 42.into()), Err(MemoryError::DuplicatedRelocation(-1)) ); assert_eq!( - memory.add_relocation_rule_maybe_relocatable((-1, 0).into(), (2, 0).into()), + memory.add_relocation_rule((-1, 0).into(), (2, 0).into()), Err(MemoryError::DuplicatedRelocation(-1)) ); @@ -1741,10 +1743,10 @@ mod memory_tests { ); memory - .add_relocation_rule_maybe_relocatable((-2, 0).into(), (3, 0).into()) + .add_relocation_rule((-2, 0).into(), (3, 0).into()) .unwrap(); assert_eq!( - memory.add_relocation_rule_maybe_relocatable((-2, 0).into(), 1.into()), + memory.add_relocation_rule((-2, 0).into(), 1.into()), Err(MemoryError::DuplicatedRelocation(-2)) ); From e6fe0beab9c9b6fc7274e0884fb400174928e825 Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Fri, 22 Nov 2024 08:36:27 -0700 Subject: [PATCH 8/9] Silence clippy warning about useless conversion --- vm/src/vm/vm_memory/memory.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/vm/src/vm/vm_memory/memory.rs b/vm/src/vm/vm_memory/memory.rs index b8cc13b0ca..3cd5c0b496 100644 --- a/vm/src/vm/vm_memory/memory.rs +++ b/vm/src/vm/vm_memory/memory.rs @@ -757,6 +757,7 @@ impl<'a> RelocateValue<'a, &'a MaybeRelocatable, Cow<'a, MaybeRelocatable>> for Ok(match value { MaybeRelocatable::Int(_) => Cow::Borrowed(value), MaybeRelocatable::RelocatableValue(addr) => { + #[allow(clippy::useless_conversion)] Cow::Owned(self.relocate_value(*addr)?.into()) } }) From 72d27c8e2ba58ad3373d6f41894108558d15ba2f Mon Sep 17 00:00:00 2001 From: Stephen Shelton Date: Fri, 22 Nov 2024 15:05:56 -0700 Subject: [PATCH 9/9] Only use Into if "extensive_hints" Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> --- vm/src/vm/vm_memory/memory.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/vm/src/vm/vm_memory/memory.rs b/vm/src/vm/vm_memory/memory.rs index 3cd5c0b496..affd0a06d4 100644 --- a/vm/src/vm/vm_memory/memory.rs +++ b/vm/src/vm/vm_memory/memory.rs @@ -757,8 +757,12 @@ impl<'a> RelocateValue<'a, &'a MaybeRelocatable, Cow<'a, MaybeRelocatable>> for Ok(match value { MaybeRelocatable::Int(_) => Cow::Borrowed(value), MaybeRelocatable::RelocatableValue(addr) => { - #[allow(clippy::useless_conversion)] - Cow::Owned(self.relocate_value(*addr)?.into()) + #[cfg(not(feature = "extensive_hints"))] + let v = self.relocate_value(*addr)?.into(); + #[cfg(feature = "extensive_hints")] + let v = self.relocate_value(*addr)?; + + Cow::Owned(v) } }) }