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

chore: explicitly import CurveTrait for sub trait method and remove some default implementations #20

Merged
merged 2 commits into from
Jan 3, 2025

Conversation

asterite
Copy link
Contributor

@asterite asterite commented Dec 20, 2024

Description

Problem

Noir will soon require trait methods to be in scope to be used.

In this test this line:

    let result = point.dbl().dbl().sub(point);

uses the sub method for the CurveTrait. Because there's also sub from std::ops::Sub, Noir now doesn't know which one to use. This PR will fix it for the upcoming release.

Summary

In addition to using that import, I removed the default implementations of add and others because those are defined separately, and those default implementations will error once noir-lang/noir#6645 is merged.

Additional Context

For now this will produce a warning, but I guess it's fine (it will be gone when this feature is released)

PR Checklist

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

@asterite asterite requested a review from TomAFrench December 20, 2024 16:04
@asterite asterite requested review from TomAFrench and removed request for TomAFrench January 2, 2025 13:48
@asterite asterite changed the title chore: explicitly import CurveTrait for sub trait method chore: explicitly import CurveTrait for sub trait method and remove some default implementations Jan 2, 2025
@TomAFrench TomAFrench merged commit 42875fd into noir-lang:main Jan 3, 2025
8 checks passed
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