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: Basic implementation of traits #2368

Merged
merged 31 commits into from
Aug 30, 2023

Conversation

yordanmadzhunkov
Copy link
Contributor

@yordanmadzhunkov yordanmadzhunkov commented Aug 18, 2023

Description

Make the most basic test case for traits execute correctly. Although this review looks big, it's mostly test code.

Next step will be to refactor tests in resolver, so I can add more unit tests and move the errors from Def Collecter to Resolver.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

After this PR this noir programs work:

Execution of specialized for the specific type:

trait Default {
    fn default(x: Field, y: Field) -> Self;
    fn method2(x: Field) -> Field {
	x
    }
}

struct Foo {
    bar: Field,
    array: [Field; 2],
}

impl Default for Foo {
    fn default(x: Field,y: Field) -> Self {
        Self { bar: x, array: [x,y] }
    }

    fn method2(x: Field) -> Field {
	x * 3
    }
}

fn main(x: Field) {
    let first = Foo::method2(x);
    assert(first == 3 * x);
}

Execution of trait provided default implementation

trait Default {
    fn default(x: Field, y: Field) -> Self;
    fn method2(x: Field) -> Field {
	x
    }
}

struct Foo {
    bar: Field,
    array: [Field; 2],
}

impl Default for Foo {
    fn default(x: Field,y: Field) -> Self {
        Self { bar: x, array: [x,y] }
    }
}

fn main(x: Field) {
    let first = Foo::method2(x);
    assert(first == x);
}

For more examples refer to tests added in this PR

@yordanmadzhunkov
Copy link
Contributor Author

@jfecher Have a nice weekend :)

@TomAFrench TomAFrench changed the title feat: Basic implementaion of traits feat: Basic implementation of traits Aug 18, 2023
crates/noirc_frontend/src/hir/def_collector/dc_crate.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/hir/def_collector/dc_mod.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/hir/def_collector/dc_mod.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/hir/def_collector/dc_mod.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/hir/def_collector/dc_mod.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/hir/def_collector/dc_mod.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/hir/def_collector/errors.rs Outdated Show resolved Hide resolved
crates/noirc_frontend/src/hir/def_collector/errors.rs Outdated Show resolved Hide resolved
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.

Just a few more small suggestions

@jfecher
Copy link
Contributor

jfecher commented Aug 25, 2023

Looks like you've got some import errors

@jfecher jfecher added this pull request to the merge queue Aug 30, 2023
Merged via the queue into noir-lang:master with commit df9f09e Aug 30, 2023
16 checks passed
TomAFrench added a commit that referenced this pull request Aug 30, 2023
* master:
  chore!: Remove keys from preprocessed artifacts (#2283)
  chore(noir): Release 0.10.5 (#2482)
  feat: Basic implementation of traits (#2368)
  fix: Implement constant folding during the mem2reg pass (#2464)
@Savio-Sou Savio-Sou mentioned this pull request Oct 17, 2023
@Savio-Sou
Copy link
Collaborator

Does this close #527?

@Savio-Sou
Copy link
Collaborator

@yordanmadzhunkov would you like to take a jab at documenting this PR? #3193

@yordanmadzhunkov
Copy link
Contributor Author

Sure, we will document traits, but for now I prefer to focus on testing/bug fixing, before documentation.

TomAFrench added a commit that referenced this pull request Aug 31, 2023
* master:
  chore: update ACIR artifacts (#2503)
  chore!: Update to `acvm-backend-barretenberg` v0.12.0 (#2377)
  fix: Bring back accidentally deleted double_verify_proof test. (#2501)
  chore(aztec_noir): import aztec library if not found yet (#2492)
  chore(abi)!: Replace struct name with fully qualified struct path (#2374)
  chore!: Remove keys from preprocessed artifacts (#2283)
  chore(noir): Release 0.10.5 (#2482)
  feat: Basic implementation of traits (#2368)
  fix: Implement constant folding during the mem2reg pass (#2464)
@jfecher
Copy link
Contributor

jfecher commented Aug 31, 2023

@Savio-Sou we're avoiding documenting traits until they're complete. We don't want users to be thinking they're in the language yet. There's also a compiler warning when using them to alert users they're still in development. This PR is just one in a series since traits are a large feature we need to break up into many PRs.

@Savio-Sou
Copy link
Collaborator

Savio-Sou commented Sep 1, 2023

Thanks for the clarification @jfecher!

Do we have a list of the intermediate Issues that'd get us to completion? Would be great if you could help add a tasklist on #527 if not. Feel free to edit the Issue directly!

@jfecher
Copy link
Contributor

jfecher commented Sep 1, 2023

@Savio-Sou I believe @yordanmadzhunkov and his team is currently working on such a list.

@signorecello
Copy link
Contributor

@yordanmadzhunkov I'm documenting traits in the Noir docs. Will add you as a reviewer if you don't mind!

@yordanmadzhunkov
Copy link
Contributor Author

yordanmadzhunkov commented Sep 7, 2023

@signorecello
Do it :)

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.

6 participants