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

Type bound error when instantiating signed integers at negative boundary #3394

Closed
Savio-Sou opened this issue Nov 1, 2023 · 3 comments · Fixed by #3690
Closed

Type bound error when instantiating signed integers at negative boundary #3394

Savio-Sou opened this issue Nov 1, 2023 · 3 comments · Fixed by #3690
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Savio-Sou
Copy link
Collaborator

Savio-Sou commented Nov 1, 2023

Aim

Instantiate signed integers at but not exceeding their negative type boundaries.

Expected Behavior

E.g. given i8's boundary goes from -128 to 127, both of the following examples should work:

  1. Adding a value up to the type boundary:
fn main() {
    let x : i8 = -127;
    let y : i8 = -1;
    x + y;
}
  1. Instantiating variable at boundary:
fn main() {
    let x : i8 = -128;
}

Bug

Proving (1) works.

While attempting to prove (2) gave:

% nargo prove
error: 2⁷ does not fit within the type bounds for i8
┌─ ~/src/main.nr:13:19
│
│     let x : i8 = -128;
│                   ---
│
= Call stack:
  ...

This seems to only happen at the negative boundary, but not on the positive side.

Installation Method

Binary

Nargo Version

(Nightly)

nargo version = 0.18.0 noirc version = 0.18.0+a0985412594c5cbd35551afc7e94cb444723adb0 (git version hash: a098541, is dirty: false)

Additional Context

Discovered while drafting examples in #3393.

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@Savio-Sou Savio-Sou added the bug Something isn't working label Nov 1, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Nov 1, 2023
@Savio-Sou
Copy link
Collaborator Author

Assigning P-LOW under the assumption that this is a minor bug that does not affect a lot of users.

Please vote with emojis if your project is affected by this bug.

@jfecher
Copy link
Contributor

jfecher commented Nov 1, 2023

This seems to only happen at the negative boundary, but not on the positive side.

This is because the integer isn't -128, it is 128 which you then take the negative of. Since 128 doesn't fit within an i8 you get an error explaining so (note the error message says 2^7, not (-2)^7).

We can add negative integer literals to fix this, I just wanted to explain the reason for this error.

@guipublic
Copy link
Contributor

I have been looking into this and to add to Jake's comment, we translate -128 into 0-128. The expression 0-128 is invalid because it uses 128, although the original expression -128 is valid.
I tried to fix this by interpreting negative integers during ssa codegen but it is not satisfactory as -(-128) becomes -128. So I also agree that we need negative integer literals.

@Savio-Sou Savio-Sou added this to the 1.0 milestone Nov 22, 2023
github-merge-queue bot pushed a commit that referenced this issue Dec 5, 2023
# Description

## Problem\*

Resolves #3394 

## Summary\*

Add negative integer literals so that -128 becomes now a literal instead
of a unary expression.

## 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.
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Dec 5, 2023
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