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(traits): Add default and override of methods #2585

Merged

Conversation

yordanmadzhunkov
Copy link
Contributor

This is the first part from split #2574

#Description #2568
In this PR we introduce the following progress of implementation of traits:

  • Trait can have an default implementation that will execute if not override by the struct
  • Trait can have an empty implementation which forces the struct to provide one
  • Structs can implement traits and override the default implementation

Remove unnecessary checks:

  • Remove the premature checks (from def collector), that should be done at a later phase:

Name resolution/Type checking:

  • Check for duplicate implementations for the same Trait and Type (this check should work even for empty traits!)
  • Error when implementing non-struct type on trait

Successful examples:

trait_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);
}

trait_override_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 method2(x: Field) -> Field {	
        x * 3    
    }
}

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

PR Checklist*

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

This commit removes some checks on UnresolvedTypes.

It adds the following validity checks and tests for them:

    Duplicate trait blocks should not compile
    Duplicate trait impl blocks should not compile
    Duplicate method definitions in impl blocks should not compile
    Impl blocks invalid types should not compile
@yordanmadzhunkov
Copy link
Contributor Author

@jfecher :)

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.

There's a few items here that can be cleaned up (see comments), but we can leave those to a future PR. I'll be on vacation until Monday the 18th starting tomorrow so I'd like to approve this now to get it merged.

While I'm gone, feel free to keep making PRs. You can set the merge target of each successive PR to the branch in the previous PR so that you can keep building on the code even if the prior PR(s) aren't yet merged. Might have some merge conflicts when changes are made to an earlier PR later but I think this would be better than halting all progress for a week. Apologies, I know this is suboptimal!

@jfecher jfecher added this pull request to the merge queue Sep 7, 2023
Merged via the queue into noir-lang:master with commit 98c3ba9 Sep 7, 2023
14 checks passed
@nickysn nickysn mentioned this pull request Sep 7, 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.

4 participants