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(parser): fixes for the parsing of 'where' clauses #2430

Merged

Conversation

nickysn
Copy link
Contributor

@nickysn nickysn commented Aug 24, 2023

Description

Improves the parsing of where clauses, in preparation for the future support of traits.

Adds support for:

  1. Multiple traits in the where clause:
    fn Test(a: T) where T: TraitX + TraitY;
  2. Generic traits in the where clause:
    fn Test(a: T) where T: TraitX<u32, Field>;

Part of #527 - Implement Traits.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.
    'cargo fmt' makes the code not follow the style used in the parser.

The parser function where_trait_bounds() was renamed trait_bounds().

Rationale: The same function can later be used for parsing 'impl TraitX+TraitY'
as function parameters or return values. They contain trait bounds, without
using the keyword 'where'.

No functional changes.
The function where_trait_bound was renamed trait_bound.

Rationale: Same as previous commit.

No functional changes.
@nickysn nickysn changed the title Parser fixes for the where clause fix(parser): fixes for the parsing of 'where' clauses Aug 24, 2023
@nickysn nickysn marked this pull request as ready for review August 24, 2023 19:14
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.

Allowing combining trait bounds with + is good, but trait bounds should still be allowed on arbitrary types to match rust

crates/noirc_frontend/src/ast/traits.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/ast/expression.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/parser/parser.rs Outdated Show resolved Hide resolved
Simplify the TraitConstraint structure by making it hold only a single trait
bound, instead of a vector of trait bounds. Previously, the vector would
represent constraints, containing multiple traits, concatenated with '+', such
as 'where Foo: Display + TraitX + TraitY<U, V>'. Now this is simplified in the
parser to several constraints, containing only a single bound:
  'Foo: Display',
  'Foo: TraitX',
  'Foo: TraitY<U, V>'
@nickysn nickysn requested a review from jfecher August 26, 2023 11:47
@jfecher jfecher added this pull request to the merge queue Aug 28, 2023
Merged via the queue into noir-lang:master with commit fa31015 Aug 28, 2023
18 checks passed
TomAFrench added a commit that referenced this pull request Aug 28, 2023
* master:
  feat(ssa): Reuse existing results for duplicated instructions with no side-effects (#2460)
  fix: Closure lvalue capture bugfix (#2457)
  feat: Syntax for environment types now works with generics (#2383)
  fix(parser): fixes for the parsing of 'where' clauses (#2430)
  fix: Run `wasm` nodejs tests with no fails (#2387)
@nickysn nickysn deleted the nickysn/parser_fixes_for_where_clause branch August 28, 2023 20:51
TomAFrench added a commit that referenced this pull request Aug 30, 2023
* master: (42 commits)
  fix(ssa): Handle right shift with constants (#2481)
  chore(noir): Release 0.10.4 (#2354)
  fix: Divide by zero should fail to satisfy constraints for `Field` and ints (#2475)
  fix(ssa): Remove padding from ToRadix call with constant inputs (#2479)
  fix: Implement handling of array aliasing in the mem2reg optimization pass (#2463)
  chore: resolve `Instruction` inputs fully before checking against cache (#2472)
  chore: Move independent `run_test` function into nargo core (#2468)
  feat: Standard library functions can now be called with closure args  (#2471)
  feat(frontend): aztec syntactic sugar (feature flagged) (#2403)
  chore(ci): enforce compliance with `cargo fmt` (#2467)
  chore(ci): Allow releases to have additional feature flags (#2405)
  feat: Add `assert_eq` keyword (#2137)
  fix(ssa): Do not optimize for allocates in constant folding (#2466)
  feat(ssa): Reuse existing results for duplicated instructions with no side-effects (#2460)
  fix: Closure lvalue capture bugfix (#2457)
  feat: Syntax for environment types now works with generics (#2383)
  fix(parser): fixes for the parsing of 'where' clauses (#2430)
  fix: Run `wasm` nodejs tests with no fails (#2387)
  chore: Run `cargo fmt` (#2455)
  chore: Perform formatting changes to integration tests (#2448)
  ...
TomAFrench added a commit that referenced this pull request Aug 30, 2023
* master: (42 commits)
  fix(ssa): Handle right shift with constants (#2481)
  chore(noir): Release 0.10.4 (#2354)
  fix: Divide by zero should fail to satisfy constraints for `Field` and ints (#2475)
  fix(ssa): Remove padding from ToRadix call with constant inputs (#2479)
  fix: Implement handling of array aliasing in the mem2reg optimization pass (#2463)
  chore: resolve `Instruction` inputs fully before checking against cache (#2472)
  chore: Move independent `run_test` function into nargo core (#2468)
  feat: Standard library functions can now be called with closure args  (#2471)
  feat(frontend): aztec syntactic sugar (feature flagged) (#2403)
  chore(ci): enforce compliance with `cargo fmt` (#2467)
  chore(ci): Allow releases to have additional feature flags (#2405)
  feat: Add `assert_eq` keyword (#2137)
  fix(ssa): Do not optimize for allocates in constant folding (#2466)
  feat(ssa): Reuse existing results for duplicated instructions with no side-effects (#2460)
  fix: Closure lvalue capture bugfix (#2457)
  feat: Syntax for environment types now works with generics (#2383)
  fix(parser): fixes for the parsing of 'where' clauses (#2430)
  fix: Run `wasm` nodejs tests with no fails (#2387)
  chore: Run `cargo fmt` (#2455)
  chore: Perform formatting changes to integration tests (#2448)
  ...
@nickysn nickysn mentioned this pull request Sep 5, 2023
46 tasks
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.

2 participants