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: Rename StructDefinition to TypeDefinition #7614

Merged
merged 7 commits into from
Mar 7, 2025

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Mar 6, 2025

Description

Problem*

Resolves #7416

Summary*

Renames StructDefinition to TypeDefinition and renames Type::as_struct to Type::as_data_type for lack of a better name.

  • StructDefinition still works but will produce a deprecated warning
  • Type::as_struct similarly still works and calls Type::as_data_type, also producing a deprecated warning.

Due to the above, this PR is non-breaking. Eventually we'll still probably want breaking type changes though. Namely any function to retrieve the struct fields from a TypeDefinition should be changed to return a Option of the previous return type since it may be called with an enum now.

Additional Context

Compared to separating StructDefinition from EnumDefinition I think this way is more convenient since it allows users to define one attribute function to work on all data types, instead of requiring a separate function to handle structs & enums. The later would also necessitate we needed a separate struct_derive and enum_derive, etc.

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 6, 2025 20:53
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 6, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 763df5a Previous: ba5beb6 Ratio
noir-lang_noir_bigcurve_ 266 s 220 s 1.21

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@jfecher jfecher enabled auto-merge March 7, 2025 17:09
Copy link
Contributor

github-actions bot commented Mar 7, 2025

Copy link
Contributor

github-actions bot commented Mar 7, 2025

FYI @noir-lang/developerrelations on Noir doc changes.

@jfecher jfecher added this pull request to the merge queue Mar 7, 2025
Merged via the queue into master with commit e3f8398 Mar 7, 2025
103 of 105 checks passed
@jfecher jfecher deleted the jf/rename-struct-def branch March 7, 2025 17:33
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
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename StructDefinition to TypeDefinition
2 participants