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

Trait constraints issues with Eq for custom types #3326

Closed
mohammadfawaz opened this issue Nov 8, 2022 · 1 comment · Fixed by #3391
Closed

Trait constraints issues with Eq for custom types #3326

mohammadfawaz opened this issue Nov 8, 2022 · 1 comment · Fixed by #3391
Assignees
Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen P: critical Should be looked at before anything else

Comments

@mohammadfawaz
Copy link
Contributor

mohammadfawaz commented Nov 8, 2022

Code:

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),
        }
    }
}

enum Error {
    BoolError: bool,      
    U8Error: u8,      
}

impl Eq for Error {
    fn eq(self, other: Self) -> bool {
        match (self, other) {
            (Error::BoolError(val1), Error::BoolError(val2)) => val2 == val1,
            (Error::U8Error(val1), Error::U8Error(val2)) => val2 == val1,
            _ => false,
        }
    }
}

fn test_none_ok_or<T, E>(_val: T, default: E) where E: Eq {
    match Option::None::<T>().ok_or(default) {
        Result::Ok(_) => revert(0),
        Result::Err(e) => assert(default == e),
    }
}

fn main() {
    test_none_ok_or(true, Error::BoolError(true));
}

This compiles but reverts. When I change test_none_ok_or(true, Error::BoolError(true)); to test_none_ok_or(true, Error::BoolError(0)); for example, the test does not revert.

In test_none_ok_or, if I log default and e, I get the same value so the issue is with the comparison default == e which evaluates to false for some reason, even though Eq is implemented correctly. In fact, even if I change the implementation of Eq for Error to just be return true, the test still reverts 🤔

In fact, I don't think we're using the custom definition of Eq for Error at all! If I log some value in eq, I don't get any Log receipts back

@mohammadfawaz mohammadfawaz added bug Something isn't working P: critical Should be looked at before anything else compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen labels Nov 8, 2022
@esdrubal
Copy link
Contributor

Seems the same issue as #3351

When I disable inlining it is also using an empty implementation of eq.

pub fn eq_8(self !78: { u64, ( bool | u64 ) }, other !79: { u64, ( bool | u64 ) }) -> bool, !82 {
      entry(self: { u64, ( bool | u64 ) }, other: { u64, ( bool | u64 ) }):
      v0 = const bool undef
      ret bool v0
  }

esdrubal added a commit to esdrubal/sway that referenced this issue Nov 18, 2022
When constraint is used in generic functions the trait name does not
contain any prefixes this was causing
get_methods_for_type_and_trait_name to return no methods to be used.

Closes FuelLabs#3326
esdrubal added a commit to esdrubal/sway that referenced this issue Nov 18, 2022
When constraint is used in generic functions the trait name does not
contain any prefixes this was causing
get_methods_for_type_and_trait_name to return no methods to be used.

Closes FuelLabs#3326
esdrubal added a commit to esdrubal/sway that referenced this issue Nov 24, 2022
When constraint is used in generic functions the trait name does not
contain any prefixes this was causing
get_methods_for_type_and_trait_name to return no methods to be used.

Closes FuelLabs#3326
tritao pushed a commit that referenced this issue Dec 8, 2022
The issue was caused by constraints used in generic functions having
trait name that does not contain any prefixes. This was causing
`get_methods_for_type_and_trait_name` to return no methods to be used.

This PR changes `get_methods_for_type_and_trait_name` so when a
`trait_name` without prefixes is provided it still tries to match using
the suffixes.

Closes #3326

Co-authored-by: Mohammad Fawaz <mohammadfawaz89@gmail.com>
Co-authored-by: Emily Herbert <17410721+emilyaherbert@users.noreply.github.com>
sdankel pushed a commit that referenced this issue Dec 14, 2022
The issue was caused by constraints used in generic functions having
trait name that does not contain any prefixes. This was causing
`get_methods_for_type_and_trait_name` to return no methods to be used.

This PR changes `get_methods_for_type_and_trait_name` so when a
`trait_name` without prefixes is provided it still tries to match using
the suffixes.

Closes #3326

Co-authored-by: Mohammad Fawaz <mohammadfawaz89@gmail.com>
Co-authored-by: Emily Herbert <17410721+emilyaherbert@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen P: critical Should be looked at before anything else
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants