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.

Sign up for GitHub

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

Jump to bottom

feat: TypeVariableKind for just Integers #4118

Merged
merged 29 commits into from
Feb 13, 2024
Merged

feat: TypeVariableKind for just Integers #4118

merged 29 commits into from
Feb 13, 2024

Conversation

michaeljklein
Copy link
Contributor

@michaeljklein michaeljklein commented Jan 22, 2024 •
edited

Description

Add a new TypeVariableKind specifically for Integers: until this PR, IntegerOrField is used.

Problem*

May resolve #3639, #4290

Summary*

Adds the new TypeVariableKind

  • Unify
  • Test
  • Docs

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [Exceptional Case] Documentation to be submitted in a separate PR.

PR Checklist*

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

jfecher
jfecher reviewed Jan 25, 2024
View reviewed changes
Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start here. One thing we'll want to do is bind any type variables used in integer operations to this new integer type variable kind. You can find the integer operations in type_check/expr.rs, where we check binary operators. There's a function for all binary operators and there's a helper for comparison operators. Bitwise operators and < and friends (but not ==) will be integers only, while the rest can accept Fields.

You'll also likely see delayed_type_checks used at those locations since that's the hack we have currently before we had this integer typevar.

Let me know if you need any help/guidance working on this. It can get a bit tricky trying to work through it all and identify the corner cases.

compiler/noirc_frontend/src/hir_def/types.rs Outdated Show resolved Hide resolved
test_programs/compile_success_empty/brillig_integer_binary_operations/src/main.nr Outdated Show resolved Hide resolved
@jfecher jfecher mentioned this pull request Jan 29, 2024
Closed
@jfecher jfecher mentioned this pull request Feb 7, 2024
Closed
5 tasks
@michaeljklein michaeljklein changed the title WIP: TypeVariableKind for just Integers feat: TypeVariableKind for just Integers Feb 7, 2024
@michaeljklein michaeljklein marked this pull request as ready for review February 8, 2024 15:17
jfecher
jfecher reviewed Feb 8, 2024
View reviewed changes
Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The additional type rules look good to me.
Just some small nitpicks before we can merge.

compiler/noirc_frontend/src/hir_def/types.rs Outdated Show resolved Hide resolved
compiler/noirc_frontend/src/monomorphization/mod.rs Show resolved Hide resolved
test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr Outdated Show resolved Hide resolved
test_programs/compile_success_empty/infer_int_loop_bounds/src/main.nr Outdated Show resolved Hide resolved
test_programs/compile_success_empty/int_or_field_casting_loop/src/main.nr Outdated Show resolved Hide resolved
jfecher
jfecher reviewed Feb 13, 2024
View reviewed changes
Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment to update, and lets remove the outdated tests. Then we can merge

compiler/noirc_frontend/src/hir_def/types.rs Outdated Show resolved Hide resolved
jfecher
jfecher approved these changes Feb 13, 2024
View reviewed changes
Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jfecher jfecher added this pull request to the merge queue Feb 13, 2024
Merged via the queue into master with commit c956be8 Feb 13, 2024
39 checks passed
@jfecher jfecher deleted the michaeljklein/integer-kind branch February 13, 2024 16:21
@kevaundray kevaundray mentioned this pull request Feb 13, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Mar 11, 2024
@kevaundray @Savio-Sou
chore: Release Noir(0.25.0) (#4352)
3f67605 
🤖 I have created a release *beep* *boop*
---


<details><summary>0.25.0</summary>

## [0.25.0](v0.24.0...v0.25.0)
(2024-03-11)


### ⚠ BREAKING CHANGES

* Internal as a macro
(AztecProtocol/aztec-packages#4898)
* reserve `unchecked` keyword
([#4432](#4432))
* Remove empty value from bounded vec
([#4431](#4431))
* Ban Fields in for loop indices and bitwise ops
([#4376](#4376))
* bump msrv to 1.73.0
([#4406](#4406))
* **ci:** Bump MSRV to 1.72.1 and enforce that ACVM can be published
using updated lockfile
([#4385](#4385))
* Restrict bit sizes
([#4235](#4235))
* move noir out of yarn-project
(AztecProtocol/aztec-packages#4479)
* note type ids
(AztecProtocol/aztec-packages#4500)

### Features

* Add eddsa_poseidon_to_pub function to stdlib with test + docs
([#4473](#4473))
([00d2c32](00d2c32))
* Add HashMap to the stdlib
([#4242](#4242))
([650ffc5](650ffc5))
* Add option to set max memory for bb.js
([#4227](#4227))
([8a6b131](8a6b131))
* Add overflow and underflow checks for unsigned integers in brillig
([#4445](#4445))
([21fc4b8](21fc4b8))
* Add poseidon2 opcode implementation for acvm/brillig, and Noir
([#4398](#4398))
([10e8292](10e8292))
* Added cast opcode and cast calldata
(AztecProtocol/aztec-packages#4423)
([78ef013](78ef013))
* Allow type aliases to reference other aliases
([#4353](#4353))
([c44ef14](c44ef14))
* Backpropagate constants in ACIR during optimization
([#3926](#3926))
([aad0da0](aad0da0))
* **ci:** Use wasm-opt when compiling wasm packages
([#4334](#4334))
([e382921](e382921))
* DAP Preflight and debugger compilation options
([#4185](#4185))
([e0ad0b2](e0ad0b2))
* Expose separate functions to compile programs vs contracts in
`noir_wasm` ([#4413](#4413))
([7cd5fdb](7cd5fdb))
* Internal as a macro
(AztecProtocol/aztec-packages#4898)
([5f57ebb](5f57ebb))
* Note type ids
(AztecProtocol/aztec-packages#4500)
([78ef013](78ef013))
* Restrict bit sizes
([#4235](#4235))
([1048f81](1048f81))
* Run tests in parallel in `nargo test`
([#4484](#4484))
([761734e](761734e))
* Skip redundant range checks in brillig
([#4460](#4460))
([cb4c1c5](cb4c1c5))
* Sync from aztec-packages
([#4483](#4483))
([fe8f277](fe8f277))
* Track stack frames and their variables in the debugger
([#4188](#4188))
([ae1a9d9](ae1a9d9))
* TypeVariableKind for just Integers
([#4118](#4118))
([c956be8](c956be8))
* Update error message when trying to load workspace as dependency
([#4393](#4393))
([d2585e7](d2585e7))


### Bug Fixes

* **acir:** Array dynamic flatten
([#4351](#4351))
([b2aaeab](b2aaeab))
* **acir:** Use types on dynamic arrays
([#4364](#4364))
([ba2c541](ba2c541))
* Add `follow_bindings` to follow `Type::Alias` links
([#4521](#4521))
([b94adb9](b94adb9))
* Add handling to `noir_wasm` for projects without dependencies
([#4344](#4344))
([4982251](4982251))
* Allow type aliases in main
([#4505](#4505))
([8a5359c](8a5359c))
* Ban Fields in for loop indices and bitwise ops
([#4376](#4376))
([601fd9a](601fd9a))
* Brillig range check with consistent bit size
([#4357](#4357))
([ea47d4a](ea47d4a))
* Build noir_codegen when publishing
([#4448](#4448))
([cb1ceee](cb1ceee))
* Consistent bit size for truncate
([#4370](#4370))
([dcd7a1e](dcd7a1e))
* Correct formatting for databus visibility types
([#4423](#4423))
([cd796de](cd796de))
* Correct invalid brillig codegen for `EmbeddedCurvePoint.add`
([#4382](#4382))
([5051ec4](5051ec4))
* **docs:** Update install versions
([#4396](#4396))
([b283637](b283637))
* **docs:** Update noirjs_app for 0.23
([#4378](#4378))
([f77f702](f77f702))
* Enforce matching types of binary ops in SSA
([#4391](#4391))
([70866ae](70866ae))
* Fix brillig slowdown when assigning arrays in loops
([#4472](#4472))
([2a53545](2a53545))
* **flake:** Stop flake.nix removing ignored-tests.txt
([#4455](#4455))
([ebaf05a](ebaf05a))
* Force src impl for == on slices
([#4507](#4507))
([1691274](1691274))
* Handling of gh deps in noir_wasm
([#4499](#4499))
([1d65370](1d65370))
* Iterative flattening pass
([#4492](#4492))
([33c1ef7](33c1ef7))
* Noir test incorrect reporting
(AztecProtocol/aztec-packages#4925)
([5f57ebb](5f57ebb))
* Only add `.nr` files to file manager
([#4380](#4380))
([8536c7c](8536c7c))
* Remove panic when generic array length is not resolvable
([#4408](#4408))
([00ab3db](00ab3db))
* Remove print from monomorphization pass
([#4417](#4417))
([27c66b3](27c66b3))
* **ssa:** Handle mergers of slices returned from calls
([#4496](#4496))
([f988d02](f988d02))
* Use correct type for numeric generics
([#4386](#4386))
([0a1d109](0a1d109))
* Variables from trait constraints being permanently bound over when
used within a trait impl
([#4450](#4450))
([ac60ef5](ac60ef5))


### Miscellaneous Chores

* Bump msrv to 1.73.0
([#4406](#4406))
([b5e5c30](b5e5c30))
* **ci:** Bump MSRV to 1.72.1 and enforce that ACVM can be published
using updated lockfile
([#4385](#4385))
([2fc95d2](2fc95d2))
* Move noir out of yarn-project
(AztecProtocol/aztec-packages#4479)
([78ef013](78ef013))
* Remove empty value from bounded vec
([#4431](#4431))
([b9384fb](b9384fb))
* Reserve `unchecked` keyword
([#4432](#4432))
([9544813](9544813))
</details>

<details><summary>0.41.0</summary>

## [0.41.0](v0.40.0...v0.41.0)
(2024-03-11)


### ⚠ BREAKING CHANGES

* Internal as a macro
(AztecProtocol/aztec-packages#4898)
* move noir out of yarn-project
(AztecProtocol/aztec-packages#4479)
* note type ids
(AztecProtocol/aztec-packages#4500)
* rename bigint_neg into bigint_sub
(AztecProtocol/aztec-packages#4420)
* Add expression width into acir
(AztecProtocol/aztec-packages#4014)
* init storage macro
(AztecProtocol/aztec-packages#4200)
* **acir:** Move `is_recursive` flag to be part of the circuit
definition (AztecProtocol/aztec-packages#4221)
* Sync commits from `aztec-packages`
([#4144](#4144))
* Breaking changes from aztec-packages
([#3955](#3955))
* Rename Arithmetic opcode to AssertZero
([#3840](#3840))
* Remove unused methods on ACIR opcodes
([#3841](#3841))
* Remove partial backend feature
([#3805](#3805))

### Features

* Add bit size to const opcode
(AztecProtocol/aztec-packages#4385)
([158c8ce](158c8ce))
* Add expression width into acir
(AztecProtocol/aztec-packages#4014)
([158c8ce](158c8ce))
* Add instrumentation for tracking variables in debugging
([#4122](#4122))
([c58d691](c58d691))
* Add poseidon2 opcode implementation for acvm/brillig, and Noir
([#4398](#4398))
([10e8292](10e8292))
* Add support for overriding expression width
([#4117](#4117))
([c8026d5](c8026d5))
* Added cast opcode and cast calldata
(AztecProtocol/aztec-packages#4423)
([78ef013](78ef013))
* Allow brillig to read arrays directly from memory
(AztecProtocol/aztec-packages#4460)
([158c8ce](158c8ce))
* Allow nested arrays and vectors in Brillig foreign calls
(AztecProtocol/aztec-packages#4478)
([158c8ce](158c8ce))
* Allow variables and stack trace inspection in the debugger
([#4184](#4184))
([bf263fc](bf263fc))
* **avm:** Back in avm context with macro - refactor context
(AztecProtocol/aztec-packages#4438)
([158c8ce](158c8ce))
* **aztec-nr:** Initial work for aztec public vm macro
(AztecProtocol/aztec-packages#4400)
([158c8ce](158c8ce))
* Aztec-packages
([#3754](#3754))
([c043265](c043265))
* Backpropagate constants in ACIR during optimization
([#3926](#3926))
([aad0da0](aad0da0))
* Breaking changes from aztec-packages
([#3955](#3955))
([5be049e](5be049e))
* Evaluation of dynamic assert messages
([#4101](#4101))
([c284e01](c284e01))
* Init storage macro
(AztecProtocol/aztec-packages#4200)
([158c8ce](158c8ce))
* Internal as a macro
(AztecProtocol/aztec-packages#4898)
([5f57ebb](5f57ebb))
* Note type ids
(AztecProtocol/aztec-packages#4500)
([78ef013](78ef013))
* Remove range constraints from witnesses which are constrained to be
constants ([#3928](#3928))
([afe9c7a](afe9c7a))
* Remove replacement of boolean range opcodes with `AssertZero` opcodes
([#4107](#4107))
([dac0e87](dac0e87))
* Speed up transformation of debug messages
([#3815](#3815))
([2a8af1e](2a8af1e))
* Sync `aztec-packages`
([#4011](#4011))
([fee2452](fee2452))
* Sync commits from `aztec-packages`
([#4068](#4068))
([7a8f3a3](7a8f3a3))
* Sync commits from `aztec-packages`
([#4144](#4144))
([0205d3b](0205d3b))
* Sync from aztec-packages
([#4483](#4483))
([fe8f277](fe8f277))


### Bug Fixes

* Deserialize odd length hex literals
([#3747](#3747))
([4000fb2](4000fb2))
* Noir test incorrect reporting
(AztecProtocol/aztec-packages#4925)
([5f57ebb](5f57ebb))
* Remove panic from `init_log_level` in `acvm_js`
([#4195](#4195))
([2e26530](2e26530))
* Return error rather instead of panicking on invalid circuit
([#3976](#3976))
([67201bf](67201bf))


### Miscellaneous Chores

* **acir:** Move `is_recursive` flag to be part of the circuit
definition (AztecProtocol/aztec-packages#4221)
([158c8ce](158c8ce))
* Move noir out of yarn-project
(AztecProtocol/aztec-packages#4479)
([78ef013](78ef013))
* Remove partial backend feature
([#3805](#3805))
([0383100](0383100))
* Remove unused methods on ACIR opcodes
([#3841](#3841))
([9e5d0e8](9e5d0e8))
* Rename Arithmetic opcode to AssertZero
([#3840](#3840))
([836f171](836f171))
* Rename bigint_neg into bigint_sub
(AztecProtocol/aztec-packages#4420)
([158c8ce](158c8ce))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: Savio <72797635+Savio-Sou@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jfecher jfecher jfecher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incoherence of default type casting in loops
3 participants
@michaeljklein @jfecher @TomAFrench