implemented iterable squashed felt252 dict

[dean-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[orizi]

Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware)

---

corelib/src/dict.cairo line 89 at r1 (raw file):

extern fn squashed_dict_into_entries<T>(
    dict: SquashedFelt252Dict<T>,
) -> Array<(felt252, T, T)> nopanic;

Have only a single method directly wrapping this function in this PR.

revert all other cairo code.

Code quote:

extern fn squashed_dict_into_entries<T>(
    dict: SquashedFelt252Dict<T>,
) -> Array<(felt252, T, T)> nopanic;

[orizi]

Reviewed 5 of 9 files at r1.
Reviewable status: 5 of 9 files reviewed, 3 unresolved discussions (waiting on @dean-starkware and @gilbens-starkware)

---

a discussion (no related file):
add a test in e2e tests.

---

crates/cairo-lang-sierra/src/extensions/modules/squashed_felt252_dict.rs line 57 at r1 (raw file):

pub struct SquashedDictAsEntriesLibfuncWrapped;
impl SignatureAndTypeGenericLibfunc for SquashedDictAsEntriesLibfuncWrapped {
    const STR_ID: &'static str = "squashed_dict_into_entries";

Suggestion:

    const STR_ID: &'static str = "squashed_felt252_dict_into_entries";

[dean-starkware]

Reviewable status: 5 of 9 files reviewed, 3 unresolved discussions (waiting on @gilbens-starkware and @orizi)

---

corelib/src/dict.cairo line 89 at r1 (raw file):

Previously, orizi wrote…

Have only a single method directly wrapping this function in this PR.

revert all other cairo code.

Done.

---

crates/cairo-lang-sierra/src/extensions/modules/squashed_felt252_dict.rs line 57 at r1 (raw file):

pub struct SquashedDictAsEntriesLibfuncWrapped;
impl SignatureAndTypeGenericLibfunc for SquashedDictAsEntriesLibfuncWrapped {
    const STR_ID: &'static str = "squashed_dict_into_entries";

Done.

[gilbens-starkware]

Reviewed all commit messages.
Reviewable status: 5 of 9 files reviewed, 4 unresolved discussions (waiting on @dean-starkware and @orizi)

---

corelib/src/dict.cairo line 288 at r2 (raw file):

/// Basic trait for the `SquashedFelt252Dict` type.
pub trait SquashedFelt252DictTrait<T> {
    /// Returns an array of `(key, value, previous_value)` tuples.

And maybe mention that the first value is always 0.

Suggestion:

`(key, first_value, last_value)` tuples.

[gilbens-starkware]

Reviewable status: 5 of 9 files reviewed, 5 unresolved discussions (waiting on @dean-starkware and @orizi)

---

corelib/src/test/dict_test.cairo line 179 at r2 (raw file):

        assert_eq!(new_value, i, "New value should match expected value");
        i += 1;
    }

Otherwise it'll pass even if the returned array is empty.
You can also zip over the entries and the range 0..n.

Suggestion:

    let n = 5_u32;
    let mut dict: Felt252Dict<u32> = (0..n).into_iter().map(|x| (x.into(), x)).collect();
    let squashed_dict_entries = dict.squash().into_entries();
    let mut i = 0;
    let mut span = squashed_dict_entries.span();
    while let Some(entry) = span.pop_front() {
        let (key, _old_value, new_value) = *entry;
        assert_eq!(key, i.into(), "Key should match expected index");
        assert_eq!(new_value, i, "New value should match expected value");
        i += 1;
    }
    assert_eq!(i, n);

[orizi]

Reviewed 1 of 9 files at r1, 2 of 3 files at r2.
Reviewable status: 8 of 9 files reviewed, 4 unresolved discussions (waiting on @dean-starkware and @gilbens-starkware)

---

corelib/src/test/dict_test.cairo line 179 at r2 (raw file):

        assert_eq!(new_value, i, "New value should match expected value");
        i += 1;
    }

Suggestion:

    assert_eq!(dict.squash().into_entries(), array![
        (0, 0, 0),
        (1, 0, 1),
        (2, 0, 2),
        (3, 0, 3),
        (4, 0, 4),
    ]);

[dean-starkware]

Reviewable status: 8 of 9 files reviewed, 4 unresolved discussions (waiting on @gilbens-starkware and @orizi)

---

a discussion (no related file):

Previously, orizi wrote…

add a test in e2e tests.

Done.

---

corelib/src/dict.cairo line 288 at r2 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

And maybe mention that the first value is always 0.

Done.

---

corelib/src/test/dict_test.cairo line 179 at r2 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

Otherwise it'll pass even if the returned array is empty.
You can also zip over the entries and the range 0..n.

After Ori's suggestion I don't think it's valid anymore. Tell me if it is.

---

corelib/src/test/dict_test.cairo line 179 at r2 (raw file):

        assert_eq!(new_value, i, "New value should match expected value");
        i += 1;
    }

Done.

[orizi]

Reviewed 1 of 3 files at r3.
Reviewable status: 8 of 10 files reviewed, 5 unresolved discussions (waiting on @gilbens-starkware)

---

tests/e2e_test_data/libfuncs/felt252_dict line 684 at r3 (raw file):

test::foo@0([0]: Felt252Dict<felt252>, [1]: felt252, [2]: felt252, [3]: felt252, [4]: felt252) -> (Tuple<Felt252Dict<felt252>, felt252, felt252>);

add another test with a nullable.

---

tests/e2e_test_data/libfuncs/felt252_dict line 697 at r3 (raw file):

fn test_squashed_dict_entries(dict: SquashedFelt252Dict<felt252>) -> Array<(felt252, felt252, felt252)> {
    dict::SquashedFelt252DictImpl::<felt252>::into_entries(dict)
}

run through formatter and re-paste here.

Code quote:

fn test_squashed_dict_entries(dict: SquashedFelt252Dict<felt252>) -> Array<(felt252, felt252, felt252)> {
    dict::SquashedFelt252DictImpl::<felt252>::into_entries(dict)
}

[gilbens-starkware]

Reviewed 4 of 9 files at r1, 2 of 3 files at r3.
Reviewable status: 6 of 11 files reviewed, 4 unresolved discussions (waiting on @dean-starkware and @orizi)

---

crates/cairo-lang-sierra-ap-change/src/core_libfunc_ap_change.rs line 66 at r3 (raw file):

        CoreConcreteLibfunc::Felt252SquashedDict(_) => {
            vec![ApChange::Known(0)]
        }

And move to be after Felt252Dict. Same for the cost match.

Suggestion:

        Felt252SquashedDict(_) => {
            vec![ApChange::Known(0)]
        }

[orizi]

Reviewable status: 6 of 11 files reviewed, 4 unresolved discussions (waiting on @dean-starkware and @gilbens-starkware)

---

corelib/src/dict.cairo line 298 at r3 (raw file):

    fn into_entries(self: SquashedFelt252Dict<T>) -> Array<(felt252, T, T)>;
}
impl SquashedFelt252DictImpl<T, +Drop<T>> of SquashedFelt252DictTrait<T> {

not required.

possibly also just use #[generate_trait] now.

Suggestion:

impl SquashedFelt252DictImpl<T> of SquashedFelt252DictTrait<T> {

[dean-starkware]

Reviewable status: 6 of 11 files reviewed, 4 unresolved discussions (waiting on @gilbens-starkware and @orizi)

---

corelib/src/dict.cairo line 298 at r3 (raw file):

Previously, orizi wrote…

not required.

possibly also just use #[generate_trait] now.

Done.

---

crates/cairo-lang-sierra-ap-change/src/core_libfunc_ap_change.rs line 66 at r3 (raw file):

Previously, gilbens-starkware (Gil Ben-Shachar) wrote…

And move to be after Felt252Dict. Same for the cost match.

Done.

---

tests/e2e_test_data/libfuncs/felt252_dict line 684 at r3 (raw file):

Previously, orizi wrote…

add another test with a nullable.

Is this what you meant? If so, why is it important?

---

tests/e2e_test_data/libfuncs/felt252_dict line 697 at r3 (raw file):

Previously, orizi wrote…

run through formatter and re-paste here.

Done.

[orizi]

Reviewed 2 of 5 files at r4, 4 of 5 files at r5, all commit messages.
Reviewable status: 10 of 11 files reviewed, 2 unresolved discussions (waiting on @gilbens-starkware)

---

tests/e2e_test_data/libfuncs/felt252_dict line 684 at r3 (raw file):

Previously, dean-starkware wrote…

Is this what you meant? If so, why is it important?

because it is different enough as a case.

---

tests/e2e_test_data/libfuncs/felt252_dict line 740 at r5 (raw file):

) -> core::option::Option<Array<(felt252, felt252, felt252)>> {
    Some(dict::SquashedFelt252DictImpl::<felt252>::into_entries(dict))
}

Suggestion:

fn test_squashed_dict_entries(
    dict: SquashedFelt252Dict<Nullable<u256>>,
) -> core::option::Option<Array<(felt252, Nullable<u256>, Nullable<u256>)>> {
    Some(dict::SquashedFelt252DictImpl::<felt252>::into_entries(dict))
}

[orizi]

Reviewed 1 of 5 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dean-starkware and @gilbens-starkware)

[dean-starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @gilbens-starkware and @orizi)

---

tests/e2e_test_data/libfuncs/felt252_dict line 740 at r5 (raw file):

) -> core::option::Option<Array<(felt252, felt252, felt252)>> {
    Some(dict::SquashedFelt252DictImpl::<felt252>::into_entries(dict))
}

Done.

[orizi]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware)

[gilbens-starkware]

Reviewed 2 of 9 files at r1, 4 of 5 files at r4, 3 of 5 files at r5, 1 of 1 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dean-starkware)