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: implement From on EC structs #4148

Closed
wants to merge 2 commits into from
Closed

Conversation

TomAFrench
Copy link
Member

Description

Problem*

Resolves

Summary*

I'm experimenting with implementing From and Into now as generic traits are being built out.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [Exceptional Case] Documentation to be submitted in a separate PR.

PR Checklist*

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

@TomAFrench TomAFrench force-pushed the tf/implement-from-on-ec branch from 71bbc59 to 517077b Compare January 24, 2024 16:22
@jfecher
Copy link
Contributor

jfecher commented Jan 24, 2024

Can we just quietly not include From and Into in Noir? 😅
Unlike other traits where the intent is communicated more clearly, From and Into just read as "cast this thing" and Into doesn't even answer the question of "into what type?" Code using these tends to be more difficult to read than code using normal functions/methods for conversion just to save a few characters at the call site.
I'd rather if we encouraged users to write explicit methods like into_curve or from_field

@TomAFrench
Copy link
Member Author

Interesting, I personally feel the opposite and feel that usage of From results in clearer code compared to a hodge-podge of ad hoc methods all jumbled together in a single impl. Discuss on standup tomorrow?

@TomAFrench
Copy link
Member Author

There's also the fact that we're probably going to end up with an implementation of this trait out in the wild anyway.

@TomAFrench TomAFrench closed this Jan 25, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jan 25, 2024
# Description

## Problem\*

Supercedes #4148 

## Summary\*

This PR adds a definition of `From` and `Into` traits into the Noir
prelude. I've provided implementations for the primitive types along
with the trivial general implementations of the traits.

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [x] **[Exceptional Case]** Documentation to be submitted in a separate
PR.
Will document once codegen is up.
# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
@TomAFrench TomAFrench deleted the tf/implement-from-on-ec branch November 20, 2024 12:03
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