Added cast for signed integers.

[orizi]

Stack:

• Changed bytes31 casts to use libfunc upcast. #4098
• Added cast for signed integers. #3623 ⬅

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

---

This change is 

[liorgold2]

Reviewable status: 2 of 7 files reviewed, 20 unresolved discussions (waiting on @orizi and @spapinistarkware)

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 114 at r6 (raw file):

Previously, orizi wrote…

indeed - fixing.

Is it tested?

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 121 at r7 (raw file):

        if cast_type.overflow_above || cast_type.overflow_below {
            // Finding if the detected possible overflow is not actually possible, but a backwards
            // compatibilty based one. TODO(orizi): Remove this check after the

Move TODO to a separate line.

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 121 at r7 (raw file):

        if cast_type.overflow_above || cast_type.overflow_below {
            // Finding if the detected possible overflow is not actually possible, but a backwards
            // compatibilty based one. TODO(orizi): Remove this check after the

Suggestion:

            // Finding if the detected possible overflow is not actually possible, but a backward
            // compatibility based one. TODO(orizi): Remove this check after the

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 123 at r7 (raw file):

            // compatibilty based one. TODO(orizi): Remove this check after the
            // backwards compatibility is removed at next major sierra version.
            if from_info.signed || to_info.signed || from_info.nbits != to_info.nbits {

Another option is let same_type = from_info == to_info and if (cast_type.overflow_above || cast_type.overflow_below) && !same_type {

Suggestion:

            let backward_compatibility_exception = !from_info.signed && !to_info.signed && from_info.nbits == to_info.nbits;
            if !backward_compatibility_exception {

[liorgold2]

Reviewed 1 of 4 files at r7.
Reviewable status: 2 of 7 files reviewed, 19 unresolved discussions (waiting on @orizi and @spapinistarkware)

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 121 at r7 (raw file):

Previously, liorgold2 wrote…

Move TODO to a separate line.

Never mind.

[orizi]

Reviewable status: 2 of 7 files reviewed, 18 unresolved discussions (waiting on @liorgold2 and @spapinistarkware)

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 114 at r6 (raw file):

Previously, liorgold2 wrote…

Is it tested?

it is - which confuses me on how this did not fail.

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 121 at r7 (raw file):

        if cast_type.overflow_above || cast_type.overflow_below {
            // Finding if the detected possible overflow is not actually possible, but a backwards
            // compatibilty based one. TODO(orizi): Remove this check after the

Done.

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 123 at r7 (raw file):

Previously, liorgold2 wrote…

Another option is let same_type = from_info == to_info and if (cast_type.overflow_above || cast_type.overflow_below) && !same_type {

since the comment in needed anyway, and after the shortened version - i don't think the extra var is needed.

[orizi]

Reviewable status: 2 of 7 files reviewed, 18 unresolved discussions (waiting on @liorgold2 and @spapinistarkware)

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 114 at r6 (raw file):

Previously, orizi wrote…

it is - which confuses me on how this did not fail.

oh - got confused with downcast test - added one for upcast.

[liorgold2]

Reviewed 1 of 5 files at r8, 1 of 2 files at r9.
Reviewable status: 2 of 7 files reviewed, 12 unresolved discussions (waiting on @orizi and @spapinistarkware)

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 114 at r6 (raw file):

Previously, orizi wrote…

oh - got confused with downcast test - added one for upcast.

Thanks

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 48 at r7 (raw file):

Previously, orizi wrote…

indeed the fmt.

Move TODO to a separate line.
If needed add a blank line between. Seems weird that the formatter is intervening this way.

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 121 at r9 (raw file):

        if cast_type.overflow_above || cast_type.overflow_below {
            // Finding if the detected possible overflow is not actually possible, but a backward
            // compatibility based one (as smae type can have no overflow).

Suggestion:

           // compatibility based one (as same type can have no overflow).

[liorgold2]

Reviewed 2 of 5 files at r8, 1 of 2 files at r9, all commit messages.
Reviewable status: 5 of 7 files reviewed, 9 unresolved discussions (waiting on @orizi and @spapinistarkware)

[orizi]

Reviewable status: 5 of 7 files reviewed, 9 unresolved discussions (waiting on @liorgold2 and @spapinistarkware)

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 48 at r7 (raw file):

Previously, liorgold2 wrote…

Move TODO to a separate line.
If needed add a blank line between. Seems weird that the formatter is intervening this way.

Done.

---

crates/cairo-lang-sierra/src/extensions/modules/casts.rs line 121 at r9 (raw file):

        if cast_type.overflow_above || cast_type.overflow_below {
            // Finding if the detected possible overflow is not actually possible, but a backward
            // compatibility based one (as smae type can have no overflow).

Done.

[liorgold2]

Reviewed 1 of 1 files at r10, all commit messages.
Reviewable status: 5 of 7 files reviewed, 8 unresolved discussions (waiting on @orizi and @spapinistarkware)

---

crates/cairo-lang-sierra-to-casm/src/invocations/casts.rs line 56 at r10 (raw file):

        }
        CastType { overflow_above: false, overflow_below: true } => {
            add_downcast_overflow_below(&mut casm_builder, value, range_check, &to_values.min)

Add an assert that from type is unsigned. Add an assumption to the documentation of add_downcast_overflow_below.

[liorgold2]

Reviewed 1 of 5 files at r8.
Reviewable status: 6 of 7 files reviewed, 8 unresolved discussions (waiting on @orizi and @spapinistarkware)

---

corelib/src/test/integer_test.cairo line 940 at r10 (raw file):

}

/// Checks if `a` is trivially castable to `B` and that `b` is castable to `A`, with the same value.

Fix documentation.

---

corelib/src/test/integer_test.cairo line 967 at r10 (raw file):

    OfAOnly,
    OfBOnly,
}

Suggestion:

/// Describes the type of bound (min/max) in a cast.
///
/// For example, in a `u8 -> u16` cast, the lower bound is equal, and the upper bound
/// of `A` is inside `B`.
#[derive(Drop)]
enum BoundConfig {
    /// The two bounds have the same value.
    Equal,
    /// The bound of A is inside the domain of B.
    AInsideB,
    /// The bound of B is inside the domain of A.
    BInsideA,
}

---

corelib/src/test/integer_test.cairo line 969 at r10 (raw file):

}

/// Checks that casting from `A` to `B` are valid around the bounds.

or "as-intended"

Suggestion:

/// Checks that castings from `A` to `B` are correct around the bounds.

[orizi]

Reviewable status: 5 of 7 files reviewed, 8 unresolved discussions (waiting on @liorgold2 and @spapinistarkware)

---

corelib/src/test/integer_test.cairo line 940 at r10 (raw file):

Previously, liorgold2 wrote…

Fix documentation.

Done.

---

corelib/src/test/integer_test.cairo line 967 at r10 (raw file):

    OfAOnly,
    OfBOnly,
}

Done.

---

corelib/src/test/integer_test.cairo line 969 at r10 (raw file):

Previously, liorgold2 wrote…

or "as-intended"

Done.

---

crates/cairo-lang-sierra-to-casm/src/invocations/casts.rs line 56 at r10 (raw file):

Previously, liorgold2 wrote…

Add an assert that from type is unsigned. Add an assumption to the documentation of add_downcast_overflow_below.

Done.

[liorgold2]

Reviewed 1 of 2 files at r11, all commit messages.
Reviewable status: 6 of 7 files reviewed, 7 unresolved discussions (waiting on @spapinistarkware)

[liorgold2]

Reviewed 1 of 2 files at r11.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @orizi and @spapinistarkware)

---

corelib/src/test/integer_test.cairo line 1013 at r11 (raw file):

/// Checks that castings from `SubType` to `SuperType` are correct around the bounds, where
/// `SubType` is strictly contained in `SuperType`.

Suggestion:

/// `SubType` is strictly contained (in both bounds) in `SuperType`.

[liorgold2]

Dismissed @spapinistarkware from a discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @orizi)

.

[orizi]

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