-
Notifications
You must be signed in to change notification settings - Fork 204
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
Panic when monomorphizing HashMap
code
#4663
Labels
bug
Something isn't working
Comments
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Mar 29, 2024
…#4662) # Description ## Problem\* Resolves #4653 ## Summary\* If the object type is an unbound type variable, impl search currently just chooses the first available matching impl instead of erroring that type annotations are needed or similar. This can lead to confusing situations where the type chosen is chosen just because it was the first impl in the stdlib to be defined. This PR prevents that. ## Additional Context This PR depends on #4648 and should be merged after. ~~The `hashmap` test is currently failing because the `H: Hasher` constraint on its `Eq` implementation ~~is actually unsolvable since `H` isn't mentioned in the hashmap type at all~~. It is solvable since it is meantioned in `B`, although solving it so far has lead to errors during monomorphization. Previously it was fine since H was unbound and the first/only impl was just always chosen. Now I'll need to change or remove H to fix it.~~ This PR is now ready to review. I've been debugging a bug with monomorphizing some HashMap code for a little while but it turns out the bug is also present in master. So I'm considering it a separate issue: #4663 ## 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.
TomAFrench
pushed a commit
that referenced
this issue
Apr 3, 2024
…#4662) # Description ## Problem\* Resolves #4653 ## Summary\* If the object type is an unbound type variable, impl search currently just chooses the first available matching impl instead of erroring that type annotations are needed or similar. This can lead to confusing situations where the type chosen is chosen just because it was the first impl in the stdlib to be defined. This PR prevents that. ## Additional Context This PR depends on #4648 and should be merged after. ~~The `hashmap` test is currently failing because the `H: Hasher` constraint on its `Eq` implementation ~~is actually unsolvable since `H` isn't mentioned in the hashmap type at all~~. It is solvable since it is meantioned in `B`, although solving it so far has lead to errors during monomorphization. Previously it was fine since H was unbound and the first/only impl was just always chosen. Now I'll need to change or remove H to fix it.~~ This PR is now ready to review. I've been debugging a bug with monomorphizing some HashMap code for a little while but it turns out the bug is also present in master. So I'm considering it a separate issue: #4663 ## 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.
New repo
|
This issue still exists on master. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Aim
Expected Behavior
The code to execute and print an empty hashmap.
Bug
To Reproduce
Project Impact
None
Impact Context
No response
Workaround
None
Workaround Description
No response
Additional Context
No response
Installation Method
None
Nargo Version
No response
NoirJS Version
No response
Would you like to submit a PR for this Issue?
None
Support Needs
No response
The text was updated successfully, but these errors were encountered: