Skip to content

Commit

Permalink
Merge branch 'main' of github.com:lambdaclass/cairo-rs into remove-me…
Browse files Browse the repository at this point in the history
…mory-get-error
  • Loading branch information
fmoletta committed Feb 23, 2023
2 parents 90fff63 + 763fc5f commit c718073
Show file tree
Hide file tree
Showing 19 changed files with 421 additions and 424 deletions.
7 changes: 3 additions & 4 deletions src/hint_processor/builtin_hint_processor/blake2s_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,8 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Memory(
MemoryError::ExpectedRelocatable(x)
)) if x == Relocatable::from((1, 0))
Err(HintError::IdentifierNotRelocatable(x, y)
) if x == "output" && y == (1,0).into()
);
}

Expand Down Expand Up @@ -423,7 +422,7 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, HashMap::new(), hint_code),
Err(HintError::FailedToGetIds)
Err(HintError::UnknownIdentifier(x)) if x == "blake2s_ptr_end"
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,6 @@ impl HintProcessor for BuiltinHintProcessor {
#[cfg(test)]
mod tests {
use super::*;
use crate::types::relocatable::Relocatable;
use crate::vm::vm_memory::memory_segments::MemorySegmentManager;
use crate::{
any_box,
Expand Down Expand Up @@ -555,9 +554,8 @@ mod tests {
let ids_data = ids_data!["len"];
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Memory(MemoryError::ExpectedInteger(
x
))) if x == Relocatable::from((1, 1))
Err(HintError::IdentifierNotInteger(x, y))
if x == "len" && y == (1,1).into()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ mod tests {
let ids_data = ids_data!["default_value"];
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::FailedToGetIds)
Err(HintError::UnknownIdentifier(x)) if x == "default_value"
);
}

Expand Down
44 changes: 16 additions & 28 deletions src/hint_processor/builtin_hint_processor/find_element_hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ mod tests {
},
hint_processor_definition::HintProcessor,
},
types::relocatable::{MaybeRelocatable, Relocatable},
types::relocatable::MaybeRelocatable,
utils::test_utils::*,
vm::{errors::memory_errors::MemoryError, vm_core::VirtualMachine},
vm::vm_core::VirtualMachine,
};
use assert_matches::assert_matches;
use num_traits::{One, Zero};
Expand Down Expand Up @@ -269,9 +269,8 @@ mod tests {
let ids_data = ids_data!["array_ptr", "elm_size", "n_elms", "index", "key"];
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::Memory(MemoryError::UnknownMemoryCell(
x
))) if x == Relocatable::from((1, 4))
Err(HintError::IdentifierNotInteger(x, y
)) if x == "key" && y == (1,4).into()
);
}

Expand All @@ -283,9 +282,8 @@ mod tests {
)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::Memory(MemoryError::ExpectedInteger(
x
))) if x == Relocatable::from((1, 1))
Err(HintError::IdentifierNotInteger(x, y
)) if x == "elm_size" && y == (1,1).into()
);
}

Expand Down Expand Up @@ -315,16 +313,13 @@ mod tests {

#[test]
fn find_elm_not_int_n_elms() {
let relocatable = Relocatable::from((1, 2));
let (mut vm, ids_data) = init_vm_ids_data(HashMap::from([(
"n_elms".to_string(),
MaybeRelocatable::from(relocatable),
)]));
let relocatable = MaybeRelocatable::from((1, 2));
let (mut vm, ids_data) =
init_vm_ids_data(HashMap::from([("n_elms".to_string(), relocatable)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::Memory(MemoryError::ExpectedInteger(
inner
))) if inner == relocatable
Err(HintError::IdentifierNotInteger(x, y
)) if x == "n_elms" && y == (1,2).into()
);
}

Expand Down Expand Up @@ -363,12 +358,7 @@ mod tests {
init_vm_ids_data(HashMap::from([("key".to_string(), relocatable)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::Memory(MemoryError::ExpectedInteger(
Relocatable {
segment_index: 1,
offset: 4
}
)))
Err(HintError::IdentifierNotInteger(x, y)) if x == "key" && y == (1,4).into()
);
}

Expand Down Expand Up @@ -404,9 +394,8 @@ mod tests {
)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER),
Err(HintError::Memory(MemoryError::ExpectedInteger(
x
))) if x == Relocatable::from((1, 1))
Err(HintError::IdentifierNotInteger(x, y
)) if x == "elm_size" && y == (1,1).into()
);
}

Expand Down Expand Up @@ -442,9 +431,8 @@ mod tests {
)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER),
Err(HintError::Memory(MemoryError::ExpectedInteger(
x
))) if x == Relocatable::from((1, 2))
Err(HintError::IdentifierNotInteger(x, y
)) if x == "n_elms" && y == (1,2).into()
);
}

Expand Down
69 changes: 36 additions & 33 deletions src/hint_processor/builtin_hint_processor/hint_utils.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use felt::Felt;

use crate::hint_processor::hint_processor_definition::HintReference;
use crate::hint_processor::hint_processor_utils::compute_addr_from_reference;
use crate::hint_processor::hint_processor_utils::{
compute_addr_from_reference, get_ptr_from_reference,
};
use crate::hint_processor::hint_processor_utils::{
get_integer_from_reference, get_maybe_relocatable_from_reference,
};
Expand Down Expand Up @@ -42,16 +44,14 @@ pub fn get_ptr_from_var_name(
ids_data: &HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<Relocatable, HintError> {
let var_addr = get_relocatable_from_var_name(var_name, vm, ids_data, ap_tracking)?;
//Add immediate if present in reference
let hint_reference = ids_data
.get(&String::from(var_name))
.ok_or(HintError::FailedToGetIds)?;
if hint_reference.dereference {
let value = vm.get_relocatable(var_addr)?;
Ok(value)
} else {
Ok(var_addr)
let reference = get_reference_from_var_name(var_name, ids_data)?;
match get_ptr_from_reference(vm, reference, ap_tracking) {
// Map internal errors into more descriptive variants
Ok(val) => Ok(val),
Err(HintError::WrongIdentifierTypeInternal(var_addr)) => Err(
HintError::IdentifierNotRelocatable(var_name.to_string(), var_addr),
),
_ => Err(HintError::UnknownIdentifier(var_name.to_string())),
}
}

Expand All @@ -62,11 +62,7 @@ pub fn get_address_from_var_name(
ids_data: &HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<MaybeRelocatable, HintError> {
Ok(MaybeRelocatable::from(compute_addr_from_reference(
ids_data.get(var_name).ok_or(HintError::FailedToGetIds)?,
vm,
ap_tracking,
)?))
get_relocatable_from_var_name(var_name, vm, ids_data, ap_tracking).map(|x| x.into())
}

//Gets the address, as a Relocatable of the variable given by the ids name
Expand All @@ -76,24 +72,30 @@ pub fn get_relocatable_from_var_name(
ids_data: &HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<Relocatable, HintError> {
compute_addr_from_reference(
ids_data.get(var_name).ok_or(HintError::FailedToGetIds)?,
vm,
ap_tracking,
)
ids_data
.get(var_name)
.and_then(|x| compute_addr_from_reference(x, vm, ap_tracking))
.ok_or_else(|| HintError::UnknownIdentifier(var_name.to_string()))
}

//Gets the value of a variable name.
//If the value is an MaybeRelocatable::Int(Bigint) return &Bigint
//else raises Err
pub fn get_integer_from_var_name<'a>(
var_name: &str,
var_name: &'a str,
vm: &'a VirtualMachine,
ids_data: &'a HashMap<String, HintReference>,
ap_tracking: &ApTracking,
) -> Result<Cow<'a, Felt>, HintError> {
let reference = get_reference_from_var_name(var_name, ids_data)?;
get_integer_from_reference(vm, reference, ap_tracking)
match get_integer_from_reference(vm, reference, ap_tracking) {
// Map internal errors into more descriptive variants
Ok(val) => Ok(val),
Err(HintError::WrongIdentifierTypeInternal(var_addr)) => Err(
HintError::IdentifierNotInteger(var_name.to_string(), var_addr),
),
_ => Err(HintError::UnknownIdentifier(var_name.to_string())),
}
}

//Gets the value of a variable name as a MaybeRelocatable
Expand All @@ -105,13 +107,16 @@ pub fn get_maybe_relocatable_from_var_name<'a>(
) -> Result<MaybeRelocatable, HintError> {
let reference = get_reference_from_var_name(var_name, ids_data)?;
get_maybe_relocatable_from_reference(vm, reference, ap_tracking)
.ok_or_else(|| HintError::UnknownIdentifier(var_name.to_string()))
}

pub fn get_reference_from_var_name<'a>(
var_name: &str,
var_name: &'a str,
ids_data: &'a HashMap<String, HintReference>,
) -> Result<&'a HintReference, HintError> {
ids_data.get(var_name).ok_or(HintError::FailedToGetIds)
ids_data
.get(var_name)
.ok_or(HintError::UnknownIdentifier(var_name.to_string()))
}

#[cfg(test)]
Expand Down Expand Up @@ -165,7 +170,7 @@ mod tests {

assert_matches!(
get_maybe_relocatable_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
Err(HintError::FailedToGetIds)
Err(HintError::UnknownIdentifier(x)) if x == *"value"
);
}

Expand All @@ -191,9 +196,8 @@ mod tests {

assert_matches!(
get_ptr_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
Err(HintError::Memory(
MemoryError::ExpectedRelocatable(x)
)) if x == Relocatable::from((1, 0))
Err(HintError::IdentifierNotRelocatable(x,y
)) if x == "value" && y == (1,0).into()
);
}

Expand All @@ -219,7 +223,7 @@ mod tests {

assert_matches!(
get_relocatable_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
Err(HintError::FailedToGetIds)
Err(HintError::UnknownIdentifier(x)) if x == "value"
);
}

Expand All @@ -245,9 +249,8 @@ mod tests {

assert_matches!(
get_integer_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
Err(HintError::Memory(MemoryError::ExpectedInteger(
x
))) if x == Relocatable::from((1, 0))
Err(HintError::IdentifierNotInteger(x, y
)) if x == "value" && y == (1,0).into()
);
}
}

1 comment on commit c718073

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.30.

Benchmark suite Current: c718073 Previous: 763fc5f Ratio
cairo_run(cairo_programs/benchmarks/integration_builtins.json) 521693431 ns/iter (± 3360769) 401246699 ns/iter (± 3121082) 1.30
cairo_run(cairo_programs/benchmarks/secp_integration_benchmark.json) 1819537074 ns/iter (± 10391063) 1383135907 ns/iter (± 5156377) 1.32
cairo_run(cairo_programs/benchmarks/math_cmp_and_pow_integration_benchmark.json) 23767210 ns/iter (± 183026) 18152227 ns/iter (± 79762) 1.31
cairo_run(cairo_programs/benchmarks/uint256_integration_benchmark.json) 1463557064 ns/iter (± 7953319) 1123634758 ns/iter (± 3928136) 1.30

This comment was automatically generated by workflow using github-action-benchmark.

CC: @unbalancedparentheses

Please sign in to comment.