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

Bug with duplicate impl blocks defined for the same type with different type parameters. #3633

Closed
nfurfaro opened this issue Dec 18, 2022 · 7 comments · Fixed by #3676
Closed
Assignees
Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: ui Mostly compiler messages P: high Should be looked at if there are no critical issues left

Comments

@nfurfaro
Copy link
Contributor

The following code won't compile. The error is:
image

library bug;

pub trait Convert<T> {
    fn convert(t: T) -> Self;
}

pub struct A {
    a: u64,
}

pub struct B {
    b: u64,
}

pub struct C {
    c: u64,
}

impl Convert<B> for A {
    fn convert(t: B) -> Self {
        A {
            a: t.b
        }
    }
}

impl Convert<C> for A {
    fn convert(t: C) -> Self {
        A {
            a: t.c
        }
    }
}
@nfurfaro nfurfaro added bug Something isn't working compiler General compiler. Should eventually become more specific as the issue is triaged P: critical Should be looked at before anything else labels Dec 18, 2022
@nfurfaro nfurfaro moved this to Todo in Fuel Network Dec 18, 2022
@esdrubal esdrubal self-assigned this Jan 2, 2023
esdrubal added a commit to esdrubal/sway that referenced this issue Jan 2, 2023
When two generic trait implementation use the same method
implementation an error such as `Duplicate definition for method
"convert" for type "A"` would occur.

This fix does remove the error when a method with different type
parameters is added and also ensures that find_method_for_type returns a
method whose parameters types matches the given arguments expressions return types.

Closes FuelLabs#3633.
esdrubal added a commit to esdrubal/sway that referenced this issue Jan 3, 2023
When two generic trait implementation use the same method
implementation an error such as `Duplicate definition for method
"convert" for type "A"` would occur.

This fix does remove the error when a method with diferent type
parameters is added to another trait implementation with different type
parameters.
The fix also ensures that find_method_for_type returns a method whose
 parameters types matches the given arguments expressions return types.

Closes FuelLabs#3633.
esdrubal added a commit to esdrubal/sway that referenced this issue Jan 3, 2023
When two generic trait implementation use the same method
implementation an error such as `Duplicate definition for method
"convert" for type "A"` would occur.

This fix does remove the error when a method with diferent type
parameters is added to another trait implementation with different type
parameters.
The fix also ensures that find_method_for_type returns a method whose
 parameters types matches the given arguments expressions return types.

Closes FuelLabs#3633.
@emilyaherbert
Copy link
Contributor

This is not a bug---this case should not be allowed and it is true that these implementations are conflicting. This is because the generic type "B" is equivalent to the generic type "C", they are both "a generic type" without any additional information. i.e. when the user calls the method .convert::<bool>(..), how is the compiler to know which implementation of convert to use?

See here for this same case in Rust: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=293dbb6a1a51a2c06fba457340e430d6

We should make the error message more clear though---I will change this issue to reflect this.

@emilyaherbert emilyaherbert changed the title Bug with generic trait impls using same method names Improve error message with conflicting trait implementations with generics Jan 5, 2023
@emilyaherbert emilyaherbert added P: low compiler: ui Mostly compiler messages compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen and removed P: critical Should be looked at before anything else compiler General compiler. Should eventually become more specific as the issue is triaged labels Jan 5, 2023
@mohammadfawaz
Copy link
Contributor

@emilyaherbert I think in @nfurfaro's example, B and C are not meant to be generic parameters. This is the use case that he's thinking of: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=dad181001f80f61c5fd47994f3e6e067

The use case here is being able to implement From for ContractId from multiple types.

@nfurfaro
Copy link
Contributor Author

nfurfaro commented Jan 5, 2023

@mohammadfawaz is correct; sorry if my attempt at a simple repro is misleading and/or incorrect.

@emilyaherbert
Copy link
Contributor

Oh I see, apologies for my mistake! Thanks so much for clarifying @mohammadfawaz and @nfurfaro.

Yes in this case the original bug description is correct 😄

@emilyaherbert
Copy link
Contributor

sorry if my attempt at a simple repro is misleading and/or incorrect.

No no, just a brain fart on my end haha.

@emilyaherbert emilyaherbert changed the title Improve error message with conflicting trait implementations with generics Bug with duplicate impl blocks defined for the same type with different type parameters. Jan 5, 2023
esdrubal added a commit to esdrubal/sway that referenced this issue Jan 16, 2023
When two generic trait implementation use the same method
implementation an error such as `Duplicate definition for method
"convert" for type "A"` would occur.

This fix does remove the error when a method with diferent type
parameters is added to another trait implementation with different type
parameters.
The fix also ensures that find_method_for_type returns a method whose
 parameters types matches the given arguments expressions return types.

Closes FuelLabs#3633.
@nfurfaro
Copy link
Contributor Author

@esdrubal It looks like you may have a fix for this in the works?
I'm blocked by this again, and it will keep happening with more frequency until resolved. :)

@nfurfaro nfurfaro added P: high Should be looked at if there are no critical issues left and removed P: low labels Jan 26, 2023
esdrubal added a commit to esdrubal/sway that referenced this issue Jan 26, 2023
When two generic trait implementation use the same method
implementation an error such as `Duplicate definition for method
"convert" for type "A"` would occur.

This fix does remove the error when a method with diferent type
parameters is added to another trait implementation with different type
parameters.
The fix also ensures that find_method_for_type returns a method whose
 parameters types matches the given arguments expressions return types.

Closes FuelLabs#3633.
@esdrubal
Copy link
Contributor

@nfurfaro yes #3676 should fix this. Updated it now to remove merge conflicts. I hope it will be merged soon.

mohammadfawaz added a commit that referenced this issue Feb 3, 2023
)

When two generic trait implementation use the same method name with a
different type parameter an error such as `Duplicate definition for
method "convert" for type "A"` would occur.

This fix does remove the error when a method with diferent type
parameters is added to another trait implementation with different type
parameters.
The fix also ensures that when possible find_method_for_type returns a
method whose parameters types matches the given arguments expressions
return types.

Closes #3633.

---------

Co-authored-by: Mohammad Fawaz <mohammadfawaz89@gmail.com>
@github-project-automation github-project-automation bot moved this from Todo to Done in Fuel Network Feb 3, 2023
eightfilms pushed a commit that referenced this issue Feb 7, 2023
)

When two generic trait implementation use the same method name with a
different type parameter an error such as `Duplicate definition for
method "convert" for type "A"` would occur.

This fix does remove the error when a method with diferent type
parameters is added to another trait implementation with different type
parameters.
The fix also ensures that when possible find_method_for_type returns a
method whose parameters types matches the given arguments expressions
return types.

Closes #3633.

---------

Co-authored-by: Mohammad Fawaz <mohammadfawaz89@gmail.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 compiler: ui Mostly compiler messages P: high Should be looked at if there are no critical issues left
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants