Handlng zero sized locals.

[orizi]

This change is 

[ilyalesokhin-starkware]

crates/cairo-lang-sierra-generator/src/local_variables.rs line 59 at r1 (raw file):

                let info = db.get_type_info(db.get_concrete_type_id(var.ty).ok()?).ok()?;
                if info.zero_sized { Some(id) } else { None }
            })

why is this needed?

Code quote:

            // All empty variables are not ap based.
            lowered_function.variables.iter().filter_map(|(id, var)| {
                let info = db.get_type_info(db.get_concrete_type_id(var.ty).ok()?).ok()?;
                if info.zero_sized { Some(id) } else { None }
            })

[ilyalesokhin-starkware]

crates/cairo-lang-sierra-generator/src/local_variables.rs line 297 at r1 (raw file):

                | OutputVarReferenceInfo::NewLocalVar
                | OutputVarReferenceInfo::ZeroSized => {
                    self.non_ap_based.insert(*var);

does this miss things?

Code quote:

                | OutputVarReferenceInfo::ZeroSized => {
                    self.non_ap_based.insert(*var);

[orizi]

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @gilbens-starkware and @ilyalesokhin-starkware)

---

crates/cairo-lang-sierra-generator/src/local_variables.rs line 59 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

why is this needed?

since we need all zero sized variables to be dropped.
a variable merged from a branch where it is used, is stored in one branch and not dropped in the other.
The issue test crashes.

---

crates/cairo-lang-sierra-generator/src/local_variables.rs line 297 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

does this miss things?

yes - since merge does not catch it.

[ilyalesokhin-starkware]

crates/cairo-lang-sierra-generator/src/local_variables.rs line 59 at r1 (raw file):

Previously, orizi wrote…

since we need all zero sized variables to be dropped.
a variable merged from a branch where it is used, is stored in one branch and not dropped in the other.
The issue test crashes.

why do we need all zero sized variables to be dropped?

[ilyalesokhin-starkware]

crates/cairo-lang-sierra-generator/src/local_variables.rs line 297 at r1 (raw file):

Previously, orizi wrote…

yes - since merge does not catch it.

does not catch what?

[orizi]

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

---

crates/cairo-lang-sierra-generator/src/local_variables.rs line 59 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

why do we need all zero sized variables to be dropped?

since we need all variables to be dropped.
zero sized variables are treated as temporary variables when merged (all other merged introduced variables ARE always tempvars at the end of the stack).
Therefore 0 sized require special handling.

---

crates/cairo-lang-sierra-generator/src/local_variables.rs line 297 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

does not catch what?

merge assumes all is temp var - zero sized included.

[ilyalesokhin-starkware]

crates/cairo-lang-sierra-generator/src/local_variables.rs line 297 at r1 (raw file):

Previously, orizi wrote…

merge assumes all is temp var - zero sized included.

wdyt about only hanlind remapped variables?

[ilyalesokhin-starkware]

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

---

crates/cairo-lang-sierra-generator/src/local_variables.rs line 297 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

wdyt about only hanlind remapped variables?

https://reviewable.io/reviews/starkware-libs/cairo/4418

[ilyalesokhin-starkware]

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

---

crates/cairo-lang-sierra-generator/src/local_variables.rs line 297 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

https://reviewable.io/reviews/starkware-libs/cairo/4418

I'm also ok with you approach, but I think it is less in the spirt of rest of the code.

[orizi]

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

---

crates/cairo-lang-sierra-generator/src/local_variables.rs line 297 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

I'm also ok with you approach, but I think it is less in the spirt of rest of the code.

i'm fine with it, i think the code is built to handle logically temporary values, so any way to ignore them is fine with me.

[ilyalesokhin-starkware]

crates/cairo-lang-sierra-generator/src/local_variables.rs line 297 at r1 (raw file):

Previously, orizi wrote…

i'm fine with it, i think the code is built to handle logically temporary values, so any way to ignore them is fine with me.

what do you mean by "logically temporary values"?

[orizi]

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

---

crates/cairo-lang-sierra-generator/src/local_variables.rs line 297 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

what do you mean by "logically temporary values"?

i mean to exclude empty vars.

[ilyalesokhin-starkware]

Reviewed 3 of 3 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gilbens-starkware)

[gilbens-starkware]

Reviewed 2 of 3 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @orizi)

[gilbens-starkware]

Reviewed 1 of 3 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @orizi)