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 on infinitely recursive types #7579

Merged
merged 9 commits into from
Mar 7, 2025
Merged

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Mar 4, 2025

Description

Problem*

Resolves #7173

Summary*

Adds a check during monomorphization that we never create any recursive types which are likely to be infinitely sized.

Additional Context

I also added a file for monomorphization tests so we don't have to keep creating a full test_program for these monomorphization errors.

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.

@jfecher jfecher requested a review from a team March 4, 2025 20:14
@jfecher jfecher requested a review from vezenovm March 5, 2025 19:19
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.

What about a test for Foo<Bar<()>> where both Foo and Bar are enums? Or if that case works, what about if Bar is an alias or struct-wrapper of Foo?

Naming nit: are these recursive types or simply nested enums? I'd expect the error to be more similar to the nested slices error.

@jfecher
Copy link
Contributor Author

jfecher commented Mar 7, 2025

Naming nit: are these recursive types or simply nested enums? I'd expect the error to be more similar to the nested slices error.

They're.. semi-recursive enums. Nested enums like Option<Abc> are allowed and in tests already. Structs or enums that reference themselves recursively internally will panic during monomorphization before this PR even if the recursion terminates. I'll add some more tests.

@jfecher
Copy link
Contributor Author

jfecher commented Mar 7, 2025

@michaeljklein Added more tests

@michaeljklein michaeljklein self-requested a review March 7, 2025 16:19
@jfecher jfecher added this pull request to the merge queue Mar 7, 2025
Merged via the queue into master with commit ba5beb6 Mar 7, 2025
105 checks passed
@jfecher jfecher deleted the jf/recursive-type-error branch March 7, 2025 16:32
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Mar 8, 2025
feat: add optional oracle resolver url in `acvm_cli` (noir-lang/noir#7630)
chore: Rename `StructDefinition` to `TypeDefinition` (noir-lang/noir#7614)
fix: Error on infinitely recursive types (noir-lang/noir#7579)
fix: update error message to display 128 bits as valid bit size (noir-lang/noir#7626)
chore: update docs to reflect u128 type (noir-lang/noir#7623)
feat: array concat method (noir-lang/noir#7199)
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.

Recursive enums cause stack overflow
4 participants