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

Remove implicit numeric generics #5156

Closed
vezenovm opened this issue May 31, 2024 · 3 comments · Fixed by #5837
Closed

Remove implicit numeric generics #5156

vezenovm opened this issue May 31, 2024 · 3 comments · Fixed by #5837
Assignees
Labels
compiler frontend `noirc_frontend` crate enhancement New feature or request

Comments

@vezenovm
Copy link
Contributor

vezenovm commented May 31, 2024

Problem

Once #4633 is closed we will have support for explicit numeric generics. You can currently use implicit numeric generics.
Example:

struct MyStruct<S> {
    data: [Field; S],
}

This is limiting and causes duplicate definition errors when working with the new explicit numeric generics. This also is confusing for users as there are now two different ways to declare numeric generics. We should move to just using explicit numeric generics.

Happy Case

In #5155 a warning has been included when implicit generics are used and we simply do not add the generic variable to scope if there is another variable already in scope.

We should remove declare_numeric_generics and only use explicit numeric generics.

We also need to make sure to generalize the kind check inside of resolve_type_inner after #5155. Right now we just check the kind of NamedGeneric's but we should be checking the kind's of all types. The existence of implicit numeric generics complicates the more thorough kind checking introduced in #5155 as it requires us to have a much more complex implicit numeric generics system that accurately resets the kinds of implicit generics. This was deemed unnecessary as implicit numeric generics are already going to be removed in favor of the more robust explicit numeric generics system.

When checking all type kinds we can fail on this code:

    struct ValueNote {}

    trait Serialize<N> {
        fn serialize(self) -> [Field; N];
    }

    impl Serialize<1> for ValueNote {
        fn serialize(self) -> [Field; 1] {
            [self.value]
        }
    }

However, if we mark trait Serialize<let N: u32> the code will succeed as the generic on Serialize has the correct kind.

Project Impact

None

Impact Context

This will be breaking for a lot of code. That is why we have settled on a tiered approach for introducing the feature.

Workaround

None

Workaround Description

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@TomAFrench
Copy link
Member

I think enough time has passed that we can push forwards on this.

@TomAFrench
Copy link
Member

@michaeljklein can you take this on?

@michaeljklein
Copy link
Contributor

@TomAFrench sure

@michaeljklein michaeljklein self-assigned this Aug 27, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 17, 2024
# Description

## Problem\*

- Resolves #5156
- Resolves #5447

## Summary\*



## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [x] Documentation included in this PR.
- [ ] **[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: Maxim Vezenov <[email protected]>
Co-authored-by: jfecher <[email protected]>
Co-authored-by: Tom French <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Sep 19, 2024
# Description

## Problem\*

Resolves #6100 

## Summary\*

It seems that #5156 has resulted in problems in unifying numeric
generics in trait impl methods.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler frontend `noirc_frontend` crate enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants