Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incoherence of default type casting in loops #3639

Closed
TAdev0 opened this issue Nov 30, 2023 · 0 comments · Fixed by #4118 or #4376
Closed

Incoherence of default type casting in loops #3639

TAdev0 opened this issue Nov 30, 2023 · 0 comments · Fixed by #4118 or #4376
Assignees
Labels
bug Something isn't working compiler frontend `noirc_frontend` crate good first issue Good for newcomers

Comments

@TAdev0
Copy link
Contributor

TAdev0 commented Nov 30, 2023

Aim

While trying to use the loop index for a comparison, I ran into an error :

    for i in 0..8 {
        for j in 0..8 {                   
             if  (i + j < 8) {...}

This code will not compile either with nargo 0.19.2 or an older version. The error is as follows :

error: Comparisons are invalid on Field types. Try casting the operands to a sized integer type first

Expected Behavior

We expect both loop index and hardcoded values to default to u64 in order to seamlessly use them to do comparisons.

Bug

There are 2 ways to make this code compile with Nargo:

Option 1 : explicitly declare loop indexes i and j as u64, though they are supposed to default to u64 if no other type is specified. In this situation, 8 which is of type TypeVariableKind::IntegerOrField, seems to default to u64 only if we explicitly declare i and j as u64.

    for i in 0..8 {
        for j in 0..8 {                   
             if  (i as u64 + j as u64 < 8) {...}
        }
    )

Option 2 : explicitly declare 8 value to a u64 type, which is supposed to default to a field type if no other type is specified. In this situation, i and j are treated like u64 by default.

    for i in 0..8 {
        for j in 0..8 {                   
             if  (i + j < 8 as u64) {...}
        }
    }

Installation Method

Noirup

Nargo Version

nargo 0.19.2 and older versions

Would you like to submit a PR for this Issue?

Yes

@TAdev0 TAdev0 added the bug Something isn't working label Nov 30, 2023
@jfecher jfecher self-assigned this Nov 30, 2023
@jfecher jfecher added compiler frontend `noirc_frontend` crate P-MEDIUM labels Nov 30, 2023
@kevaundray kevaundray added the good first issue Good for newcomers label Jan 15, 2024
github-merge-queue bot pushed a commit that referenced this issue Feb 13, 2024
# Description

Add a new `TypeVariableKind` specifically for Integers: until this PR,
`IntegerOrField` is used.

## Problem\*

May resolve #3639,
#4290

## Summary\*

Adds the new `TypeVariableKind`
- [x] Unify
- [x] Test
- [ ] Docs

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [x] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
@jfecher jfecher reopened this Feb 14, 2024
github-merge-queue bot pushed a commit that referenced this issue Feb 22, 2024
# Description

## Problem\*

Resolves #3639
Resolves #4193

## Summary\*

Uses the new TypeVariableKind::Integer in for loops and bitwise
operations to prevent `Field` types from being used there.

Removes the old `delayed_type_checks` hack.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
Co-authored-by: TomAFrench <tom@tomfren.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler frontend `noirc_frontend` crate good first issue Good for newcomers
Projects
Archived in project
5 participants