updated semantic to handle variants

[dean-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[orizi]

Reviewed 1 of 4 files at r1, all commit messages.
Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on @dean-starkware and @gilbens-starkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 2109 at r1 (raw file):

                }
            }
            // TODO: clarify.

?

Code quote:

            // TODO: clarify.

---

crates/cairo-lang-semantic/src/resolve/mod.rs line 1383 at r1 (raw file):

        stable_ptr: SyntaxStablePtrId,
        variant_id: VariantId,
        _generic_args: &[ast::GenericArg],

Why keep this arg?

---

crates/cairo-lang-semantic/src/expr/semantic_test_data/let_statement line 126 at r1 (raw file):

    let another_value = Var1;
    another_value
}

i don't understand this test mix.

Code quote:

//! > function_body
let _value: TestEnum = Var2;

//! > expr_code
{
    let another_value = Var1;
    another_value
}

[dean-starkware]

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

---

crates/cairo-lang-semantic/src/expr/compute.rs line 2109 at r1 (raw file):

Previously, orizi wrote…

?

@gilbens-starkware

[orizi]

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

---

crates/cairo-lang-semantic/src/expr/compute.rs line 2109 at r1 (raw file):

Previously, dean-starkware wrote…

@gilbens-starkware

You should have an assignee in the Todo, and it should be more specific in any case.

[dean-starkware]

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

---

crates/cairo-lang-semantic/src/expr/compute.rs line 2109 at r1 (raw file):

Previously, orizi wrote…

You should have an assignee in the Todo, and it should be more specific in any case.

Yes, I agree.
But in this case- @gilbens-starkware asked to marked it as is for now.
I'm not sure why though- I'm giving him the chance to explain it before removing it.

[dean-starkware]

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

---

crates/cairo-lang-semantic/src/expr/semantic_test_data/let_statement line 126 at r1 (raw file):

Previously, orizi wrote…

i don't understand this test mix.

Done.

---

crates/cairo-lang-semantic/src/resolve/mod.rs line 1383 at r1 (raw file):

Previously, orizi wrote…

Why keep this arg?

Done.

[gilbens-starkware]

Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on @orizi)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 2109 at r1 (raw file):

Previously, dean-starkware wrote…

Yes, I agree.
But in this case- @gilbens-starkware asked to marked it as is for now.
I'm not sure why though- I'm giving him the chance to explain it before removing it.

The comment below is now outdated due to your changes, so update it according to your changes.

[orizi]

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

---

crates/cairo-lang-semantic/src/expr/semantic_test_data/let_statement line 107 at r2 (raw file):

//! > ==========================================================================

//! > Test let statement with imported variants.

what is the let statement related to here?
the only thing you seem to be checking is the creation of the variant and the matching on it.
and if so - also separate into 2 tests.

Code quote:

//! > Test let statement with imported variants.

---

crates/cairo-lang-semantic/src/expr/semantic_test_data/let_statement line 129 at r2 (raw file):

    };
another_value
}

Suggestion:

match value {
    Var2(3_u32) => Var1,
    _ => Var2(0_u32),
}

[dean-starkware]

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

---

crates/cairo-lang-semantic/src/expr/compute.rs line 2109 at r1 (raw file):

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

The comment below is now outdated due to your changes, so update it according to your changes.

Done.

---

crates/cairo-lang-semantic/src/expr/semantic_test_data/let_statement line 107 at r2 (raw file):

Previously, orizi wrote…

what is the let statement related to here?
the only thing you seem to be checking is the creation of the variant and the matching on it.
and if so - also separate into 2 tests.

Done.

---

crates/cairo-lang-semantic/src/expr/semantic_test_data/let_statement line 129 at r2 (raw file):

    };
another_value
}

Done.

[orizi]

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

---

crates/cairo-lang-semantic/src/expr/semantic_test_data/let_statement line 107 at r2 (raw file):

Previously, dean-starkware wrote…

Done.

again - i don't see what this has to be with a let statement - this should just include the match, and all these tests should just not be in this file.

---

crates/cairo-lang-semantic/src/resolve/mod.rs line 1122 at r1 (raw file):

                    identifier.stable_ptr().untyped(),
                    var.id,
                    &generic_args_syntax.unwrap_or_default(),

what happens if anyone actually tries to supply generics after?
add a test to see the relevant diagnostics.

Code quote:

                    &generic_args_syntax.unwrap_or_default(),

[dean-starkware]

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

---

crates/cairo-lang-semantic/src/expr/semantic_test_data/let_statement line 107 at r2 (raw file):

Previously, orizi wrote…

again - i don't see what this has to be with a let statement - this should just include the match, and all these tests should just not be in this file.

I want to test the creation of a variant. That's the reasoning behind the first test, which I do believe should be here.

---

crates/cairo-lang-semantic/src/resolve/mod.rs line 1122 at r1 (raw file):

Previously, orizi wrote…

what happens if anyone actually tries to supply generics after?
add a test to see the relevant diagnostics.

Can you please explain what do you mean by "generics after"?
A short example would be helpful.

[orizi]

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

---

crates/cairo-lang-semantic/src/expr/semantic_test_data/let_statement line 107 at r2 (raw file):

Previously, dean-starkware wrote…

I want to test the creation of a variant. That's the reasoning behind the first test, which I do believe should be here.

you can create a variant without a let statement - just have your expression a creation of a variant.

but this means it should be tested here.

[orizi]

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

---

crates/cairo-lang-semantic/src/resolve/mod.rs line 1122 at r1 (raw file):

Previously, dean-starkware wrote…

Can you please explain what do you mean by "generics after"?
A short example would be helpful.

this code ignored its given generic args for variants - so you need to tell me where these should come from and why can you ignore them.

so - it really depends on the context you are getting the variant here.

It is probably at something like: Some<u8> which obviously shouldn't work - but i'm not sure what is going to happen.

[dean-starkware]

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

---

crates/cairo-lang-semantic/src/expr/semantic_test_data/let_statement line 107 at r2 (raw file):

Previously, orizi wrote…

you can create a variant without a let statement - just have your expression a creation of a variant.

but this means it should be tested here.

Done. (+ kept it here as requested)

---

crates/cairo-lang-semantic/src/resolve/mod.rs line 1122 at r1 (raw file):

Previously, orizi wrote…

this code ignored its given generic args for variants - so you need to tell me where these should come from and why can you ignore them.

so - it really depends on the context you are getting the variant here.

It is probably at something like: Some<u8> which obviously shouldn't work - but i'm not sure what is going to happen.

Done.