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

chore: remove temporary workaround for issue 1354 in swcurve.nr and tecurve.nr #2473

Closed
wants to merge 9 commits into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Aug 29, 2023

Description

In the curvegroup::mul function of both swcurve.nr and tecurve.nr, we previously had a temporary workaround for issue 1354. This workaround involved manually copying bits from a bit representation of the field element n into an array n_as_bits. However, this workaround is no longer necessary as issue 1354 has been resolved.

PR Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@TomAFrench
Copy link
Member

#1354

The bit arrays don't need to be mutable anymore. Can you remove this?

@ghost
Copy link
Author

ghost commented Aug 29, 2023

It appears that this pull request is dependent on the resolution of issue #2336.

minimal example:

fn hello<N>(array: [u1; N]) {
    for i in 0..N {}
}

fn main() {
    let slice: [u1] = [].push_back(1).push_back(2);
    hello(slice);
}
The application panicked (crashed).
Message:  Non-numeric type variable used in expression expecting a value
Location: crates/noirc_frontend/src/monomorphization/mod.rs:631

This is a bug. We may have already fixed this in newer versions of Nargo so try searching for similar issues at https://github.com/noir-lang/noir/issues/.
If there isn't an open issue for this bug, consider opening one at https://github.com/noir-lang/noir/issues/new?labels=bug&template=bug_report.yml

@kevaundray
Copy link
Contributor

kevaundray commented Sep 2, 2023

2336 has been closed as completed, please merge master into this branch

@ghost ghost mentioned this pull request Sep 3, 2023
@kevaundray
Copy link
Contributor

@f01dab1e can this be updated? I see that Jake's message in the issue you opened had not been resolved

@ghost
Copy link
Author

ghost commented Oct 1, 2023

@kevaundray, It seems that the array is actually a slice, and we cannot determine its size, which is why the error Could not determine loop bound at compile-time is occurring.

The "to_le_bits" function returns a slice.

The "bit_mul" function accepts an array (we are trying to convert a slice to an array), and at this point, we encounter a failure because we don't know the size: Could not determine loop bound at compile-time.

Copy link
Contributor

Thank you for your contribution to the Noir language.

Please do not force push to this branch after the Noir team have started review of this PR. Doing so will only delay us merging your PR as we will need to start the review process from scratch.

Thanks for your understanding.

@TomAFrench TomAFrench closed this Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants