Fixes method Eq not found when used with where clause. #3348
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mohammadfawaz
added
bug
Centril
reviewed
Nov 21, 2022
emilyaherbert
previously approved these changes
Nov 22, 2022
test/src/e2e_vm_tests/test_programs/should_pass/stdlib/eq_generic/src/main.sw
Outdated
Show resolved
Hide resolved
emilyaherbert
requested review from
Centril and
a team
and removed request for
Centril
mohammadfawaz
previously approved these changes
Nov 22, 2022
|
Oh, conflicts 😐 |
The problem was that although two `TypeInfo::UnknownGeneric` were unified they were not regarded as equals by `are_equal_minus_dynamic_types`. Solution was to change how `TypeInfo::UnknownGeneric` are compared in `are_equal_minus_dynamic_types`. Which can now check if two `TypeInfo::UnknownGeneric` were previously unified. This was accomplished by adding two methods to the type engine `insert_unified_type` and `get_unified_types`. Closes FuelLabs#3324
Adds a comment as requested in review. Adds more insert_unified_type to unify and unify_right which covers one case that does not currently pass in CI until FuelLabs#3391 is merged.
esdrubal
dismissed stale reviews from mohammadfawaz and emilyaherbert
via
9877d4b
|
Conflicts resolved. |
mohammadfawaz
approved these changes
Nov 22, 2022
tritao
approved these changes
Nov 22, 2022
sdankel
pushed a commit
that referenced
this pull request
Dec 14, 2022
The problem was that although two `TypeInfo::UnknownGeneric` were unified they were not regarded as equals by `are_equal_minus_dynamic_types`. Solution was to change how `TypeInfo::UnknownGeneric` are compared in `are_equal_minus_dynamic_types`. Which can now check if two `TypeInfo::UnknownGeneric` were previously unified. This was accomplished by adding two methods to the type engine `insert_unified_type` and `get_unified_types`. Closes #3324
emilyaherbert
added a commit
that referenced
this pull request
Jan 23, 2023
This PR fixes #3864 by changing the way that trait impls are re-exported to the greater namespace. Previously they we reexported somewhat at random, with one place being at function declaration sites. They _were not_ reexported at function call sites and method call sites. This PR changes this so to standardize the re-export locations to include function call sites and method call sites and to exclude function declaration sites. This PR serves as an alternate fix to #3324, as the bug with this issue involved re-exporting traits. Take this Sway example: ```rust script; use core::ops::*; impl<T> Option<T> { pub fn ok_or<E>(self, err: E) -> Result<T, E> { match self { Option::Some(v) => Result::Ok(v), Option::None => Result::Err(err), } } } fn test_ok_or<T, E>(val: T, default: E) where T: Eq { match Option::Some(val).ok_or(default) { Result::Ok(inner) => assert(inner == val), Result::Err(_) => revert(0), } } fn main() {} ``` This PR better serves to re-export the trait constraint `T: Eq` to the usable namespace inside of the function body for `ok_or`. So, this PR also removes the `unify_map` abstraction which was added to the `TypeEngine` in #3348. Closes #3864 Co-authored-by: emilyaherbert <emily.herbert@fuel.sh>
sdankel
pushed a commit
that referenced
this pull request
Jan 25, 2023
This PR fixes #3864 by changing the way that trait impls are re-exported to the greater namespace. Previously they we reexported somewhat at random, with one place being at function declaration sites. They _were not_ reexported at function call sites and method call sites. This PR changes this so to standardize the re-export locations to include function call sites and method call sites and to exclude function declaration sites. This PR serves as an alternate fix to #3324, as the bug with this issue involved re-exporting traits. Take this Sway example: ```rust script; use core::ops::*; impl<T> Option<T> { pub fn ok_or<E>(self, err: E) -> Result<T, E> { match self { Option::Some(v) => Result::Ok(v), Option::None => Result::Err(err), } } } fn test_ok_or<T, E>(val: T, default: E) where T: Eq { match Option::Some(val).ok_or(default) { Result::Ok(inner) => assert(inner == val), Result::Err(_) => revert(0), } } fn main() {} ``` This PR better serves to re-export the trait constraint `T: Eq` to the usable namespace inside of the function body for `ok_or`. So, this PR also removes the `unify_map` abstraction which was added to the `TypeEngine` in #3348. Closes #3864 Co-authored-by: emilyaherbert <emily.herbert@fuel.sh>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem was that although two
TypeInfo::UnknownGenericwere unified they were not regarded as equals byare_equal_minus_dynamic_types.Solution was to change how
TypeInfo::UnknownGenericare compared inare_equal_minus_dynamic_types. Which can now check if twoTypeInfo::UnknownGenericwere previously unified. This was accomplished by adding two methods to the type engineinsert_unified_typeandget_unified_types.Closes #3324