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

Incorrect compiler error when comparing the index of a "for" loop with a non-casted value. #4193

Closed
pventuzelo opened this issue Jan 29, 2024 · 1 comment · Fixed by #4376
Assignees
Labels
bug Something isn't working

Comments

@pventuzelo
Copy link

Aim

We (@FuzzingLabs) find that the compiler incorrectly raises an error when comparing the index of a "for" loop with a non-casted value.

Expected Behavior

The compiler should not treat the compared value as a field type but as a u64

Bug

This error seems to be caused by the compiler treating the compared value as a field type, while the index of a 'for' loop is always treated as a u64, as stated here in the documentation: https://noir-lang.org/docs/noir/syntax/control_flow

As indicated in the code comments, this error doesn't appear with a u64 variable, and it specifically occurs when using the '>'; '<'; ‘≥’; ‘≤’ operators."

The panic crash occurs with the latest compiler version of the main branch. The error message is as follows:

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

To Reproduce

  1. Create a Noir project with the following command: nargo new test_issue
  2. Paste the code below into the file main.nr
  3. Compile the project using the following command: nargo compile
fn main() {
    // Raises an error because the compiler interprets '1' as a Field and not as a u64.
    for i in 0..10 {
        assert(i > 1);
    }

    // Same issue persists even if the value is stored in a variable.
    for i in 0..10 {
        let val = 1;
        assert(i < val);
    }

    // Error only occurs with '<' '>' '<=' '>=' operators.
    for i in 0..10 {
        assert(i == 1);
    }

    // Error only occurs with the index of a "for" loop, not with every u64.
    let i: u64 = 5;
    assert(i > 1);

    // Casting the value as u64 resolves the problem.
    for i in 0..10 {
        assert(i < 1 as u64);
    }
}

Installation Method

None

Nargo Version

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@pventuzelo pventuzelo added the bug Something isn't working label Jan 29, 2024
@jfecher
Copy link
Contributor

jfecher commented Jan 29, 2024

Possibly a duplicate of #3639

The PR which fixes the duplicate #4118 should test this case as well.

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
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants