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

bug: non-numeric type variable Error #2540

Closed
Tracked by #3363
ghost opened this issue Sep 3, 2023 · 8 comments · Fixed by #4602
Closed
Tracked by #3363

bug: non-numeric type variable Error #2540

ghost opened this issue Sep 3, 2023 · 8 comments · Fixed by #4602
Assignees
Labels
bug Something isn't working

Comments

@ghost
Copy link

ghost commented Sep 3, 2023

Aim

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

fn main() {
    let slice: [u1] = [1, 2];
    hello(slice);
}

Expected Behavior

The hello function should iterate over a valid array of type u1 without panicking or producing runtime errors. In the given example, calling hello with the slice array [1, 2] should execute the loop successfully.

needed for #2473

Bug

The application panicked (crashed).
Message:  Non-numeric type variable used in expression expecting a value: NotConstant
Location: crates/noirc_frontend/src/monomorphization/mod.rs:630

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

To Reproduce

  1. nargo execute

Installation Method

Compiled from source

Nargo Version

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@ghost ghost added the bug Something isn't working label Sep 3, 2023
@jfecher
Copy link
Contributor

jfecher commented Sep 5, 2023

This is an interesting one. The N in hello takes its value from the length of an array type, but in the case of slices there is no length. Instead the internal value NotConstant used to mark slices is leaked. Not sure what the fix here would be. This may need to be a compiler error of some kind.

@jfecher
Copy link
Contributor

jfecher commented Sep 5, 2023

Is this functionality actually required for the linked PR? hello could be rewritten as:

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

@ghost
Copy link
Author

ghost commented Sep 7, 2023

@jfecher, doesn't work in this case

error: Could not determine loop bound at compile-time
    ┌─ std/ec/tecurve.nr:343:20
    │
343 │             for i in 0..bits.len() {
    │                    ----------
    │
    = Call stack:
      1. noir/crates/nargo_cli/tests/execution_success/eddsa/src/main.nr:51:17
      2. std/eddsa.nr:68:45
      3. std/ec/tecurve.nr:141:13
      4. std/ec/tecurve.nr:356:13
      5. std/ec/tecurve.nr:343:25

@jfecher
Copy link
Contributor

jfecher commented Sep 7, 2023

@f01dab1e looks like a different case in the tecurve test. Using array.len() for the original example passes. I'm assuming you've modified the eddsa test as well since it passes locally for me. The "Could not determine loop bound at compile-time" error is intended for when a value derived from an input to main is used as a loop bound. I'm not sure why that'd be the case for an array length though, unless the array was a slice returned from an unconstrained function.

@jfecher
Copy link
Contributor

jfecher commented Feb 9, 2024

Reassigning to @michaeljklein, this is part of #4220

@vezenovm
Copy link
Contributor

@michaeljklein Can we confirm whether we can close this?

@michaeljklein
Copy link
Contributor

@vezenovm yes, adding a regression test here: #4602

Noting that there were two issues in the test case:

  1. 2 : u1
  • I've removed this for the purpose of the regression test
  1. Non-numeric type variable used in expression expecting a value
  • NotConstant has been removed with the array/slice separation

@vezenovm
Copy link
Contributor

Noting that there were two issues in the test case:

  1. 2 : u1
  • I've removed this for the purpose of the regression test

We used to support the u1 integer type when this issue was written, so updating the regression that way is the right move.

github-merge-queue bot pushed a commit that referenced this issue Mar 20, 2024
# Description

## Problem\*

Resolves #2540

## Summary\*

Fixed in #4504

## 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.
TomAFrench added a commit that referenced this issue Mar 27, 2024
* master:
  chore: delete `R1CSTransformer` (#4649)
  fix: Slice coercions (#4640)
  chore(ci): add warning sticky comment (#4647)
  fix(ssa): Fix slice intrinsic handling in the capacity tracker  (#4643)
  chore: fix versioning of `bn254_blackbox_solver` crate (#4638)
  chore: fix acvm crates reporting errors as JS packages (#4637)
  chore: Release Noir(0.26.0) (#4526)
  chore: convert `BlockExpression` into a standard struct (#4623)
  chore(github): Improve PR template "document later" checkbox description (#4625)
  chore: Update integers.md to note support for Fields using `from_integer` (#4536)
  chore: update docs with function names to match version 0.25.0 specifications (#4466)
  feat: add specific error for attempting `string[x] = ".."` (#4611)
  fix(ssa): Use accurate type during SSA AsSlice simplficiation (#4610)
  feat: Sync from aztec-packages (#4581)
  chore: regression test for #2540 (#4602)
  chore: fix up benchmarking scripts (#4601)
  feat: Add experimental `quote` expression to parser (#4595)
  chore(ci): fix long debugger test times (#4599)
TomAFrench pushed a commit that referenced this issue Apr 3, 2024
# Description

## Problem\*

Resolves #2540

## Summary\*

Fixed in #4504

## 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.
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