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

fix(experimental elaborator): Move code to declare trait impls earlier #5105

Merged
merged 10 commits into from
May 28, 2024

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented May 24, 2024

Description

Problem*

Resolves "No matching impl found for ..." trait solver issues in the elaborator.

Summary*

These issues were just because we'd add a trait impl to the interner after elaborating the impl. At that point, we'd already have elaborated all other functions & methods so it wasn't of much use. I've moved it to add the trait impl to the interner during collect_trait_impl instead.

Additional Context

Down to 6 errors after this PR. The errors are:

  • "Duplicate definitions of <generic> found" - looks to be only one case where there is a generic directly on a trait method. fn hash<H>(self, state: &mut H) where H: Hasher; (1 error)
  • "Expected type &mut _, found type H" - issues with auto-deref and trait methods possibly (5 errors)

(merging master has fixed the 2 "expression is ambiguous" errors from the turbofish operator changes)

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] 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 and others added 8 commits May 22, 2024 23:25
#5081)

# Description

## Problem\*

Closes #5035

## Summary\*

Alternative to #5035 which just deletes any test programs which don't
have a Nargo.toml file.

## Additional Context



## 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.
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR reactivates the gates report workflow now that we don't have
access to the number of constraints through `nargo info`

## Additional Context



## 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.
…ng identifiers (#5087)

# Description

## Problem\*

Resolves #5063 

## Summary\*

After the turbofish PRs we want the ability to instantiate a variable
from a generic trait. This was possible but it required a redundant type
annotiation like such (assume `H` has been specified with a `where`
clause):
```rust
let hasher: H = H::default()
```
Similarly to trait generics, we now add type bindings from a trait
constraint if the trait impl is assumed to exist.
We now can just do:
```rust
let hasher = H::default()
```

## Additional Context



## 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.
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

Moving #3542,
#5041, and
#5087 to the elaborator

## Additional Context



## Documentation\*

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

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>
…#5098)

# Description

## Problem\*

Currently, when creating a regular (not marked with unconstrained)
function, it can end up running in both a constrained and unconstrained
context.

## Summary\*

This intrinsic is resolved at compile time a provides a way for
developers to dispatch to different implementations in library code for
constrained and unconstrained context. This intrinsic has been used in
stdlib's field comparison code as demonstration.

## Additional Context

It's very typical for the constrained version of a calculation to take a
hint of a computed value and verify it. However, in the unconstrained
version of that calculation we just need to compute the value, there is
no point in verifying what we just did. This intrinsic provides a tool
for devs to optimize a function for unconstrained removing these
verifications.

## Documentation\*

Check one:
- [ ] No documentation needed.
- [x] 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.

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
# Description

## Problem\*

Related to  #4629

## Summary\*
Do not create a witness when multiplying a witness (or an affine
transformation of a witness) with an expression of degree 1.


## Additional Context
This PR does NOT fix the issue #4629, but it should help reducing the
overall gates number.
The arithmetic expressions resulting from the multiplication should not
explode in width since I only consider multiplication with a witness (or
'affine witness').

## 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.
@jfecher jfecher requested review from michaeljklein and vezenovm May 24, 2024 17:20
@github-actions github-actions bot added the documentation Improvements or additions to documentation label May 24, 2024
Copy link
Contributor

github-actions bot commented May 24, 2024

Base automatically changed from jf/elaborator-fixes5 to jf/elaborator-fixes4 May 28, 2024 18:42
@jfecher jfecher merged commit 0fed42c into jf/elaborator-fixes4 May 28, 2024
22 checks passed
@jfecher jfecher deleted the jf/elaborator-fixes6 branch May 28, 2024 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants