Panic when using trait static method references to generic impls #3964
Assignees
Labels
bug
Something isn't working
Comments
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Jan 8, 2024
…to generic impls (#3967) # Description ## Problem\* Resolves #3964 ## Summary\* The core problem in #3964 stems from a trait method's generics being different from the corresponding impl's generics. So when the former was instantiated, and later tried to apply to the latter during monomorphization after an impl was selected, the latter would be unchanged. This was fixed by remembering the `TraitMethodId` during monomorphization and calling `try_unify` between the trait method's type and the impl method's type before monomorphizing the impl function. This gives type bindings to translate between the two. ## Additional Context - In creating the above type bindings, we need to bind to the original type variables on the trait without instantiating them. This meant any further impl searches would see that trait as having its monomorphized type, rather than its generic type. This would break impl search since that trait/impl would no longer apply to all the types it used to. I've fixed this by allowing `Type::substitute` to substitute to already-bound type variables so that when the impl search later occurs and the type is instantiated, the instantiation automatically sheds the bindings after its (currently bound) type variables are swapped to fresh type variables. Allowing substitution on already bound type variables is a bit concerning though. It's unclear if this could cause unsoundness later on. ~~I'll open an issue to limit the scope of this by only allowing it during `instantiate` calls instead of all `substitute` calls.~~ I've just implemented the change directly in this PR instead. I split `substitute` into `substitute` and `force_substitute`. - There's also a mostly unrelated change in this PR. `function_meta` now returns a reference instead of cloning the entire `FunctionMeta` which was quite wasteful. I'll work on breaking this out into a separate PR. (Edit: #3968) ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Aim
Using static method call syntax for a trait
Eq::eq(a, b)or operator overloadinga == bwhere the object type refers to a generic impl likeimpl<A, B> Eq for (A, B) where A: Eq, B: Eq.Expected Behavior
The code to compile and execute correctly
Bug
A panic occurs during inlining since the compiler uses type bindings from the trait signature rather than the impl signature. This leads to it instantiating the impl to
(_, _)but keeping the unbound type variables. The inner trait constraints are now_: Eqand_: Eqwhich will match any impl, and the compiler chooses the first available instead of the correct one. This leads to a panic during function inlining if the sizes of the types are not equal, and leads to panics elsewhere otherwise.To Reproduce
Installation Method
None
Nargo Version
No response
Additional Context
No response
Would you like to submit a PR for this Issue?
No
Support Needs
No response
The text was updated successfully, but these errors were encountered: