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

Enable turbofish for impls #5584

Closed
signorecello opened this issue Jul 23, 2024 · 0 comments · Fixed by #6306
Closed

Enable turbofish for impls #5584

signorecello opened this issue Jul 23, 2024 · 0 comments · Fixed by #6306
Assignees
Labels
enhancement New feature or request

Comments

@signorecello
Copy link
Contributor

signorecello commented Jul 23, 2024

Problem

Some code like this should compile:

struct MerkleTree<let N: u32> {
    root: Field,
    hasher: fn([Field; N]) -> Field,
}

impl<let N: u32> MerkleTree<N> {
    fn new(root: Field, hasher: fn([Field; N]) -> Field) -> Self {
        Self { root, hasher }
    }
}

#[test]
fn test_merkle_tree_membership() {
    let root = 0x1c59022dba1d97f63021cc5a23e4fe80f019465e0ccb54de9aa91935495354a3;
    let mt = MerkleTree::<2>::new(root, dep::std::hash::pedersen_hash);
}

But it doesn't, because MerkleTree doesn't accept turbofish

Happy Case

The above code would compile and work :)

Project Impact

None

Impact Context

No response

Workaround

None

Workaround Description

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@signorecello signorecello added the enhancement New feature or request label Jul 23, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jul 23, 2024
github-merge-queue bot pushed a commit that referenced this issue Jul 25, 2024
# Description

## Problem

Part of #5584

## Summary

This PR adds support for parsing turbofishes in any path segments.
Turbofish is only still checked when elaborating variables, but only for
the last turbofish (like before). For now any other placement of a
turbofish will give a type error, but at least they can be parsed now.
In later PRs we can make them work in the middle of variables, in
constructors, etc.

## Additional Context

Right now turbofishes aren't parsed in named typed (for example in a
struct pattern). The reason is that if I use `path()` instead of
`path_no_turbofish()` then the parser gives a stack overflow and I'm not
sure why. Maybe it's because `parse_type()` is used recursively, I'm not
sure... but these stack overflow errors are kind of hard to diagnose. In
any case we can parse those later on once we decide to support turbofish
in struct patterns.

Also, because a `Path`'s segments changed from `Vec<Ident>` to
`Vec<PathSegment>` some functions, like `Path::last_segment()` made more
sense if they returned `PathSegment` instead of `Ident`. Then I
introduced a bunch of helper functions to reduce some call chains, but
also introduced helpers like `last_name()` which is nice because it
returns a `&str` where in some cases an unnecessary clone was made (so
some refactors/cleanups are included in this PR as part of this
feature).

## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [x] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# 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.
github-merge-queue bot pushed a commit that referenced this issue Jul 26, 2024
# Description

## Problem

Part of #5584

## Summary

This adds support for things like `let foo = Foo::<i32> { x }`.

## Additional Context

None.

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# 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.
github-merge-queue bot pushed a commit that referenced this issue Jul 29, 2024
# Description

## Problem

Part of #5584

## Summary

Allows parsing turbofish in a struct pattern path segments, then uses
those turbofish generics for type binding/inference.

## Additional Context

None.

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# 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.
@asterite asterite self-assigned this Oct 21, 2024
@asterite asterite moved this from 📋 Backlog to 🚧 Blocked in Noir Oct 24, 2024
@github-project-automation github-project-automation bot moved this from 🚧 Blocked to ✅ Done in Noir Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants