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

feat(experimental): Compile match expressions #7312

Merged
merged 33 commits into from
Feb 18, 2025
Merged

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Feb 6, 2025

Description

Problem*

Working towards #988

Summary*

Adds support for compiling match expressions. Specifically:

  • During elaboration, convert match expressions to decision trees where:
    • No tag value check is repeated
    • Nested enums, tuples, and integer types are supported
    • NO errors or warnings are issued, they're all stubbed out for future PRs (just write perfect code)
    • Match cases with identical branches are deduplicated.
      • Duplicated branches can happen quite often and we could be more aggressive with this in the future since nested matches won't be deduplicated across nested cases currently.
    • The match compiler supports more pattern types than we currently parse. In addition to basic constructors & variable patterns it also supports:
      • Range patterns for ranges of integers
      • Pattern guards (_ if .. => ...)
      • We do not support matching on strings, arrays, or slices but could consider this for future work.
  • Support for lowering decision trees into SSA via many jmpifs
    • We'll want to add a TerminatorInstruction::Switch eventually but I left this for future work

So you can now execute & prove code containing match expressions.

Additional Context

Quite a few parts of match expressions are still left as TODOs for future PRs to keep this PR "small":

  • Redundancy checking
  • Missing case analysis
  • Errors in general (e.g. error: type '&mut T' can't be matched on)
  • There is an odd stack overflow when using variants which reference themselves in their generics. E.g. MyOption<MyOption<Field>> will cause a stack overflow in Monomorphizer::convert_type.
  • We could deduplicate cases more aggressively (check all cases, not just neighboring cases, and check nested cases not just cases of the current sub-match).
  • Can trigger an SSA panic when multiple match blocks all jump to the same return block during flattening. It wasn't possible for > 2 blocks to jump to the same block directly before so this triggers an assert. Flattening has difficulty flattening matches since it is used to if branches with only 2 cases. Match expressions panic during SSA flattening if they're on a runtime value with > 2 branches #7323

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 February 6, 2025 16:43
github-actions[bot]

This comment was marked as resolved.

github-actions[bot]

This comment was marked as resolved.

@jfecher jfecher requested a review from asterite February 18, 2025 16:37
jfecher and others added 2 commits February 18, 2025 12:22
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Copy link
Contributor

@vezenovm vezenovm left a comment

Choose a reason for hiding this comment

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

Approving, just have those few comments for tracking follow-ups

@jfecher jfecher enabled auto-merge February 18, 2025 20:10
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: 0466e06 Previous: f37eedc Ratio
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_reset-kernel-lib 11 s 9 s 1.22

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

CC: @TomAFrench

@jfecher jfecher added this pull request to the merge queue Feb 18, 2025
Merged via the queue into master with commit 4c3dee1 Feb 18, 2025
102 checks passed
@jfecher jfecher deleted the jf/match-compiler branch February 18, 2025 20:37
TomAFrench added a commit that referenced this pull request Feb 19, 2025
* master:
  fix(performance): Remove redundant slice access check from brillig (#7434)
  chore(docs): updating tutorials and other nits to beta.2 (#7405)
  feat: LSP hover for integer literals (#7368)
  feat(experimental): Compile match expressions (#7312)
  feat(acir_field): Add little-endian byte serialization for FieldElement (#7258)
  feat: allow unquoting TraitConstraint in trait impl position (#7395)
  feat(brillig): Hoist shared constants across functions to the global space (#7216)
  feat(LSP): auto-import via visible reexport (#7409)
  fix(brillig): Brillig entry point analysis and function specialization through duplication (#7277)
  chore: redo typo PR by maximevtush (#7425)
  fix(ssa): Accurately mark binary ops for hoisting and check Div/Mod against induction variable lower bound (#7396)
  feat!: remove bigint from stdlib (#7411)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 19, 2025
…m brillig (noir-lang/noir#7434)

chore(docs): updating tutorials and other nits to beta.2 (noir-lang/noir#7405)
feat: LSP hover for integer literals (noir-lang/noir#7368)
feat(experimental): Compile match expressions (noir-lang/noir#7312)
feat(acir_field): Add little-endian byte serialization for FieldElement (noir-lang/noir#7258)
feat: allow unquoting TraitConstraint in trait impl position (noir-lang/noir#7395)
feat(brillig): Hoist shared constants across functions to the global space (noir-lang/noir#7216)
feat(LSP): auto-import via visible reexport (noir-lang/noir#7409)
fix(brillig): Brillig entry point analysis and function specialization through duplication (noir-lang/noir#7277)
chore: redo typo PR by maximevtush (noir-lang/noir#7425)
fix(ssa): Accurately mark binary ops for hoisting and check Div/Mod against induction variable lower bound (noir-lang/noir#7396)
feat!: remove bigint from stdlib (noir-lang/noir#7411)
chore: bump aztec-packages commit (noir-lang/noir#7415)
chore: deprecate `merkle` module of stdlib (noir-lang/noir#7413)
chore(ci): lock aztec-packages commit in CI (noir-lang/noir#7414)
feat: while statement (noir-lang/noir#7280)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 19, 2025
…oir-lang/noir#7434)

chore(docs): updating tutorials and other nits to beta.2 (noir-lang/noir#7405)
feat: LSP hover for integer literals (noir-lang/noir#7368)
feat(experimental): Compile match expressions (noir-lang/noir#7312)
feat(acir_field): Add little-endian byte serialization for FieldElement (noir-lang/noir#7258)
feat: allow unquoting TraitConstraint in trait impl position (noir-lang/noir#7395)
feat(brillig): Hoist shared constants across functions to the global space (noir-lang/noir#7216)
feat(LSP): auto-import via visible reexport (noir-lang/noir#7409)
fix(brillig): Brillig entry point analysis and function specialization through duplication (noir-lang/noir#7277)
chore: redo typo PR by maximevtush (noir-lang/noir#7425)
fix(ssa): Accurately mark binary ops for hoisting and check Div/Mod against induction variable lower bound (noir-lang/noir#7396)
feat!: remove bigint from stdlib (noir-lang/noir#7411)
chore: bump aztec-packages commit (noir-lang/noir#7415)
chore: deprecate `merkle` module of stdlib (noir-lang/noir#7413)
chore(ci): lock aztec-packages commit in CI (noir-lang/noir#7414)
feat: while statement (noir-lang/noir#7280)
TomAFrench added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 19, 2025
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(performance): Remove redundant slice access check from brillig
(noir-lang/noir#7434)
chore(docs): updating tutorials and other nits to beta.2
(noir-lang/noir#7405)
feat: LSP hover for integer literals
(noir-lang/noir#7368)
feat(experimental): Compile match expressions
(noir-lang/noir#7312)
feat(acir_field): Add little-endian byte serialization for FieldElement
(noir-lang/noir#7258)
feat: allow unquoting TraitConstraint in trait impl position
(noir-lang/noir#7395)
feat(brillig): Hoist shared constants across functions to the global
space (noir-lang/noir#7216)
feat(LSP): auto-import via visible reexport
(noir-lang/noir#7409)
fix(brillig): Brillig entry point analysis and function specialization
through duplication (noir-lang/noir#7277)
chore: redo typo PR by maximevtush
(noir-lang/noir#7425)
fix(ssa): Accurately mark binary ops for hoisting and check Div/Mod
against induction variable lower bound
(noir-lang/noir#7396)
feat!: remove bigint from stdlib
(noir-lang/noir#7411)
chore: bump aztec-packages commit
(noir-lang/noir#7415)
chore: deprecate `merkle` module of stdlib
(noir-lang/noir#7413)
chore(ci): lock aztec-packages commit in CI
(noir-lang/noir#7414)
feat: while statement (noir-lang/noir#7280)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <tom@tomfren.ch>
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