Derive StorageAccess impl for enums

[maciejka]

Btw should it be:
#[derive(storage_access::StorageAccess)]
or:
#[derive(starknet::StorageAccess)]
or just:
#[derive(StorageAccess)]

---

This change is 

[orizi]

Reviewed 1 of 3 files at r1, all commit messages.
Reviewable status: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @maciejka)

---

crates/cairo-lang-starknet/src/plugin/mod.rs line 51 at r2 (raw file):

                storage_access::handle_enum(db, enum_ast)
            }
            ast::Item::Enum(enum_ast) => handle_enum(db, enum_ast),

remove use ..::handle_enum(similar to how the struct is handled)

[orizi]

#[derive(starknet::StorageAccess)] AFAIK - if it is different it is a bug.

Reviewable status: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @maciejka)

[maciejka]

Reviewed 1 of 3 files at r1, all commit messages.
Reviewable status: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @maciejka)

crates/cairo-lang-starknet/src/plugin/mod.rs line 51 at r2 (raw file):

                storage_access::handle_enum(db, enum_ast)
            }
            ast::Item::Enum(enum_ast) => handle_enum(db, enum_ast),

remove use ..::handle_enum(similar to how the struct is handled)

Fixed.

[maciejka]

#[derive(starknet::StorageAccess)] AFAIK - if it is different it is a bug.

Reviewable status: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @maciejka)

It was storage_access::StorageAccess, changed to starknet::StorageAccess. It will break early adopters contracts who tried v2.0.0.

[orizi]

Reviewed 5 of 10 files at r3, 1 of 3 files at r4, all commit messages.
Reviewable status: 6 of 10 files reviewed, 2 unresolved discussions (waiting on @maciejka)

---

crates/cairo-lang-starknet/src/plugin/storage_access.rs line 186 at r4 (raw file):

                tc.ty(db).as_syntax_node().get_text_without_trivia(db)
            }
        };

instead of special type handling - just add a normal impl for empty tuples StorageAccess in the corelib.

Code quote:

        let variant_type = match variant.type_clause(db) {
            ast::OptionTypeClause::Empty(_) => "()".to_string(),
            ast::OptionTypeClause::TypeClause(tc) => {
                tc.ty(db).as_syntax_node().get_text_without_trivia(db)
            }
        };

---

crates/cairo-lang-starknet/src/plugin/storage_access.rs line 290 at r4 (raw file):

                match value {{
                    {match_size}
                }}

we are probably going to make it size and remove the internal - as well as make the function a const.

so i suggest not actually being inner value dependent:
1 + max(field1.size_internal(), max(field2.size_internal(), max(field3.size_internal(), field4.size_internal())))

Code quote:

            fn size_internal(value: {enum_name}) -> u8 {{
                match value {{
                    {match_size}
                }}

[orizi]

i understand - but this is required.

Reviewable status: 6 of 10 files reviewed, 2 unresolved discussions (waiting on @maciejka)

[orizi]

Reviewable status: 6 of 10 files reviewed, 2 unresolved discussions (waiting on @maciejka)

---

crates/cairo-lang-starknet/src/plugin/storage_access.rs line 186 at r4 (raw file):

Previously, orizi wrote…

instead of special type handling - just add a normal impl for empty tuples StorageAccess in the corelib.

automatic implementation up to size 4 now is on main.

---

crates/cairo-lang-starknet/src/plugin/storage_access.rs line 290 at r4 (raw file):

Previously, orizi wrote…

we are probably going to make it size and remove the internal - as well as make the function a const.

so i suggest not actually being inner value dependent:
1 + max(field1.size_internal(), max(field2.size_internal(), max(field3.size_internal(), field4.size_internal())))

this was changed in main to size.

[maciejka]

Previously, orizi wrote…
this was changed in main to size.

Updated implementation according to your suggestions.
Would would please share the resoning behind making size function a const?
Not sure why clippy on CI complains. Locally it works just fine.

[orizi]

The size issue:
Assume you have a field with non const size, and after it another field: any change to the first field, may cause changes to the second one just because of offsets, where this could have been avoided.
Therefore each field should have its own span in the actual strip of 256 felts.

The clippy issue? I would assume there was some stable rust version update, will check tomorrow.

Reviewed 3 of 8 files at r5, all commit messages.
Reviewable status: 5 of 10 files reviewed, 2 unresolved discussions (waiting on @maciejka)

[orizi]

Reviewed 4 of 8 files at r5.
Reviewable status: 9 of 10 files reviewed, 2 unresolved discussions (waiting on @maciejka)

[orizi]

Reviewable status: 9 of 10 files reviewed, 2 unresolved discussions (waiting on @maciejka)

---

crates/cairo-lang-starknet/src/plugin/storage_access.rs line 177 at r5 (raw file):

        match_idx.push(format!(
            "if idx == {i} {{ \

use formatdoc instead of all the ''

[maciejka]

Reviewable status: 9 of 10 files reviewed, 2 unresolved discussions (waiting on @maciejka)

crates/cairo-lang-starknet/src/plugin/storage_access.rs line 177 at r5 (raw file):

        match_idx.push(format!(
            "if idx == {i} {{ \

use formatdoc instead of all the ''

Replaced format with formatdoc, generated code is more readble but I had to add one more depencency (indent).

[orizi]

Reviewed 3 of 4 files at r6.
Reviewable status: 12 of 13 files reviewed, all discussions resolved

[orizi]

Reviewed 1 of 4 files at r6.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @maciejka)

[spapinistarkware]

Reviewed 2 of 10 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @maciejka)

[orizi]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @maciejka)