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.

Sign up for GitHub

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

Jump to bottom

fix(parser): fixes for the parsing of 'where' clauses #2430

Merged
merged 12 commits into from
Aug 28, 2023
Merged

fix(parser): fixes for the parsing of 'where' clauses #2430

merged 12 commits into from
Aug 28, 2023

Conversation

nickysn
Copy link
Contributor

@nickysn nickysn commented Aug 24, 2023 •
edited
Loading

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.

@nickysn
fix(parser): first item in a where clause must be an identifier, not …
2f8d7bb 
…a type
@nickysn
fix(parser): support trait constraints with multiple and/or generic t…
0f56339 
…raits
@nickysn
refactor(parser): where_trait_bounds() renamed trait_bounds()
1f00ea6 
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.
@nickysn
refactor(parser): where_trait_bound() renamed trait_bound()
d381599 
The function where_trait_bound was renamed trait_bound.

Rationale: Same as previous commit.

No functional changes.
@nickysn
fix: don't display empty generic trait bounds as '<>'
10d667d
@nickysn
fix(parser): print the where clause in function definitions
f900f34
@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
jfecher
jfecher requested changes Aug 24, 2023
View reviewed changes
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
@nickysn
refactor(parser): use !is_empty() instead of len()>0
2e7b590
@nickysn
fix(parser): again allow any type on the left side of the ':' in the …
a95e10c 
…where clause
@nickysn
test(parser): added test for several comma-separated conditions in th…
7abdc1e 
…e where clause
@nickysn
refactor(parser): simplify the TraitConstraint structure
fd489e1 
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
refactor(parser): use !generics.is_empty() instead of generics.len()>0
cbf7c23
@nickysn
Merge branch 'noir-lang:master' into nickysn/parser_fixes_for_where_c…
d4ac54b 
…lause
@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
@kevaundray kevaundray mentioned this pull request Aug 28, 2023
Merged
TomAFrench added a commit that referenced this pull request Aug 28, 2023
@TomAFrench
Merge branch 'master' into assert-eq-builtin
78a57fc 
* 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
@TomAFrench
Merge branch 'master' into kw/bb-binaries
47f2ab6 
* 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
@TomAFrench
Merge branch 'master' into remove-keys-from-artifact
b04ed6b 
* 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
Open
46 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jfecher jfecher jfecher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nickysn @jfecher