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

fix: Error when a type variable is unbound during monomorphization instead of defaulting to Field #4674

Merged
merged 5 commits into from
Mar 29, 2024

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Mar 28, 2024

Description

Problem*

Resolves

Summary*

Adds an error when encountering an unbound TypeVariableKind::Normal instead of defaulting it to a Field.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] 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.

Copy link
Contributor

@michaeljklein michaeljklein left a comment

Choose a reason for hiding this comment

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

LGTM

@michaeljklein
Copy link
Contributor

  • HirLValue's now all have both a Location and a Type
  • LValueRhs's now all have a Span

@jfecher Would it make sense to factor out the common fields into a struct, e.g.

struct HirLValue {
  location: Location,
  typ: Type,
  kind: HirLValueKind,
}

?

@jfecher
Copy link
Contributor Author

jfecher commented Mar 29, 2024

@jfecher Would it make sense to factor out the common fields into a struct, e.g.

Possibly. I'd like to keep it as the enum for now though since there's only 1-2 common fields and making these structs would prevent them from being matched on directly anymore.

@jfecher
Copy link
Contributor Author

jfecher commented Mar 29, 2024

@vezenovm @michaeljklein I've found a case I missed where array lengths were defaulting separately to zero still. I've fixed this and added a test case. Requesting a re-review for these changes.

@jfecher jfecher enabled auto-merge March 29, 2024 15:00
@jfecher jfecher added this pull request to the merge queue Mar 29, 2024
Merged via the queue into master with commit 03cdba4 Mar 29, 2024
43 checks passed
@jfecher jfecher deleted the jf/typevar-default-error branch March 29, 2024 15:17
TomAFrench pushed a commit that referenced this pull request Apr 3, 2024
…stead of defaulting to Field (#4674)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

Adds an error when encountering an unbound `TypeVariableKind::Normal`
instead of defaulting it to a Field.

## 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.
github-merge-queue bot pushed a commit that referenced this pull request Apr 10, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>0.27.0</summary>

## [0.27.0](v0.26.0...v0.27.0)
(2024-04-10)


### ⚠ BREAKING CHANGES

* Brillig typed memory
(AztecProtocol/aztec-packages#5395)

### Features

* **acir_gen:** Fold attribute at compile-time and initial non inlined
ACIR (AztecProtocol/aztec-packages#5341)
([a0f7474](a0f7474))
* **acvm_js:** Execute program
([#4694](#4694))
([386f6d0](386f6d0))
* **acvm:** Execute multiple circuits
(AztecProtocol/aztec-packages#5380)
([a0f7474](a0f7474))
* Add `remove_enable_side_effects` SSA pass
([#4224](#4224))
([94952db](94952db))
* Allow slices to brillig entry points
([#4713](#4713))
([62423d5](62423d5))
* Brillig typed memory
(AztecProtocol/aztec-packages#5395)
([0bc18c4](0bc18c4))
* **docs:** Documenting noir codegen
([#4454](#4454))
([24f6d85](24f6d85))
* Improve nargo check cli with --override flag and feedback for existing
files ([#4575](#4575))
([5e7fbd4](5e7fbd4))
* Improve optimisations on range constraints
([#4690](#4690))
([96b8110](96b8110))
* Improve SSA type-awareness in EQ and MUL instructions
([#4691](#4691))
([669f1a0](669f1a0))
* **nargo:** Multiple circuits info for binary programs
([#4719](#4719))
([50d2735](50d2735))


### Bug Fixes

* "Types in a binary operation should match, but found T and T"
([#4648](#4648))
([30c9f31](30c9f31))
* **acvm:** Mark outputs of Opcode::Call solvable
([#4708](#4708))
([8fea405](8fea405))
* Correct ICE panic messages in brillig `convert_black_box_call`
([#4761](#4761))
([f3eee6c](f3eee6c))
* Error when a type variable is unbound during monomorphization instead
of defaulting to Field
([#4674](#4674))
([03cdba4](03cdba4))
* Field comparisons
([#4704](#4704))
([079cb2a](079cb2a))
* Impl search no longer selects an impl if multiple are applicable
([#4662](#4662))
([0150600](0150600))
* Last use analysis & make it an SSA pass
([#4686](#4686))
([0d3d5fd](0d3d5fd))
* Slice coercions
([#4640](#4640))
([c0bae17](c0bae17))
* **ssa:** Accurate constant type for slice dummy data in flattening
([#4661](#4661))
([b87654e](b87654e))
* **ssa:** Do not use get_value_max_num_bits when we want pure type
information ([#4700](#4700))
([b55a580](b55a580))
* **ssa:** Fix slice intrinsic handling in the capacity tracker
([#4643](#4643))
([1b50ce1](1b50ce1))
* Unknown slice lengths coming from as_slice
([#4725](#4725))
([f21129e](f21129e))
* Update commit for noir-gates-diff
([#4773](#4773))
([a9766c5](a9766c5))
</details>

<details><summary>0.43.0</summary>

## [0.43.0](v0.42.0...v0.43.0)
(2024-04-10)


### ⚠ BREAKING CHANGES

* Brillig typed memory
(AztecProtocol/aztec-packages#5395)
* **acir:** Program and witness stack structure
(AztecProtocol/aztec-packages#5149)
* automatic NoteInterface and NoteGetterOptions auto select
(AztecProtocol/aztec-packages#4508)
* Acir call opcode
(AztecProtocol/aztec-packages#4773)
* Support contracts with no constructor
(AztecProtocol/aztec-packages#5175)
* 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))

### Features

* Acir call opcode
(AztecProtocol/aztec-packages#4773)
([c3c9e19](c3c9e19))
* **acir_gen:** Fold attribute at compile-time and initial non inlined
ACIR (AztecProtocol/aztec-packages#5341)
([a0f7474](a0f7474))
* **acir:** Program and witness stack structure
(AztecProtocol/aztec-packages#5149)
([13eb71b](13eb71b))
* **acvm_js:** Execute program
([#4694](#4694))
([386f6d0](386f6d0))
* **acvm:** Execute multiple circuits
(AztecProtocol/aztec-packages#5380)
([a0f7474](a0f7474))
* Add bit size to const opcode
(AztecProtocol/aztec-packages#4385)
([158c8ce](158c8ce))
* Add CMOV instruction to brillig and brillig gen
(AztecProtocol/aztec-packages#5308)
([13eb71b](13eb71b))
* 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))
* Automatic NoteInterface and NoteGetterOptions auto select
(AztecProtocol/aztec-packages#4508)
([13eb71b](13eb71b))
* **avm:** Back in avm context with macro - refactor context
(AztecProtocol/aztec-packages#4438)
([158c8ce](158c8ce))
* **avm:** Brillig CONST of size &gt; u128
(AztecProtocol/aztec-packages#5217)
([c3c9e19](c3c9e19))
* **aztec-nr:** Initial work for aztec public vm macro
(AztecProtocol/aztec-packages#4400)
([158c8ce](158c8ce))
* Backpropagate constants in ACIR during optimization
([#3926](#3926))
([aad0da0](aad0da0))
* Breaking changes from aztec-packages
([#3955](#3955))
([5be049e](5be049e))
* Brillig IR refactor
(AztecProtocol/aztec-packages#5233)
([c3c9e19](c3c9e19))
* Brillig typed memory
(AztecProtocol/aztec-packages#5395)
([0bc18c4](0bc18c4))
* Check initializer msg.sender matches deployer from address preimage
(AztecProtocol/aztec-packages#5222)
([c3c9e19](c3c9e19))
* Evaluation of dynamic assert messages
([#4101](#4101))
([c284e01](c284e01))
* Init storage macro
(AztecProtocol/aztec-packages#4200)
([158c8ce](158c8ce))
* Initial Earthly CI
(AztecProtocol/aztec-packages#5069)
([c3c9e19](c3c9e19))
* Internal as a macro
(AztecProtocol/aztec-packages#4898)
([5f57ebb](5f57ebb))
* New brillig field operations and refactor of binary operations
(AztecProtocol/aztec-packages#5208)
([c3c9e19](c3c9e19))
* 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))
* Signed integer division and modulus in brillig gen
(AztecProtocol/aztec-packages#5279)
([c3c9e19](c3c9e19))
* Support contracts with no constructor
(AztecProtocol/aztec-packages#5175)
([c3c9e19](c3c9e19))
* 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))
* Sync from noir
(AztecProtocol/aztec-packages#5234)
([c3c9e19](c3c9e19))
* Sync from noir
(AztecProtocol/aztec-packages#5286)
([c3c9e19](c3c9e19))


### Bug Fixes

* **acvm:** Mark outputs of Opcode::Call solvable
([#4708](#4708))
([8fea405](8fea405))
* 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 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).
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.

3 participants