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

Expand Type::Constant to support larger bit sizes and signed integers #5571

Closed
vezenovm opened this issue Jul 19, 2024 · 0 comments · Fixed by #5581
Closed

Expand Type::Constant to support larger bit sizes and signed integers #5571

vezenovm opened this issue Jul 19, 2024 · 0 comments · Fixed by #5581
Assignees
Labels
bug Something isn't working compiler frontend `noirc_frontend` crate

Comments

@vezenovm
Copy link
Contributor

vezenovm commented Jul 19, 2024

Aim

Our type-level integer is currently too restrictive with the addition of explicit numeric generics. They currently are only supported as u32 values .

Expected Behavior

We should be able to support types larger than u32 for numeric generics as well as signed integers.

Bug

Different bugs can come about. One was found here (#5552) and other unexpected behavior can occur if a value larger than a u32 is used in a numeric generic.
e.g. the following will panic:

    fn big<let N: u64>() -> u64 {
        N
    }

    fn main() {
        let _ = big::<18446744073709551615>();
    }

To Reproduce

  1. Run the following snippet with nargo compile and see the failure

Project Impact

Nice-to-have

Impact Context

In between a nice-to-have and a blocker for full numeric generics.

Workaround

None

Workaround Description

No response

Additional Context

No response

Installation Method

Compiled from source

Nargo Version

0.32.0+8897de81c088983d8c1f5a2c610c19301f0bdded

NoirJS Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@vezenovm vezenovm added bug Something isn't working compiler frontend `noirc_frontend` crate labels Jul 19, 2024
@vezenovm vezenovm self-assigned this Jul 19, 2024
michaeljklein pushed a commit that referenced this issue Jul 23, 2024
… `u32` (#5581)

# Description

## Problem\*

Resolves #5571 

The issue references expanding the supported type-level integers,
however, after a team discussion we decided to simply restrict the
supported numeric generics. If other kinds of numeric generics become
highly requested we can then re-assess whether we want to expand support
for type level constants.

## Summary\*

A new parser combinator for numeric generics was introduced to
differentiate with the existing `type_expression` combinator. This is
due to usage of `or_not` in the `generic_type_args` parser. Due to the
`or_not` combinator the expression error was being ignored. Thus, the
new numeric generic expression combinator returns an optional expression
along with its parsed type.

## Additional Context

This program:
```rust
    fn big<let N: u64>() -> u64 {
        N
    }

    fn main() {
        let _ = big::<18446744073709551615>();
    }
```
gives the following error:
<img width="972" alt="Screenshot 2024-07-23 at 12 18 00 PM"
src="https://github.com/user-attachments/assets/fbbaf78a-ab86-4250-9008-ba4d2751409d">

The previous errors were very unclear:
<img width="725" alt="Screenshot 2024-07-22 at 5 59 04 PM"
src="https://github.com/user-attachments/assets/c532de18-0240-4a4a-acbc-f0be72b5b2db">



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [X] **[For Experimental Features]** 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: jfecher <[email protected]>
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
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant