Skip to content

Commit

Permalink
Fixes issue with generic trait impls using the same method names. (Fu…
Browse files Browse the repository at this point in the history
…elLabs#3676)

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 FuelLabs#3633.

---------

Co-authored-by: Mohammad Fawaz <[email protected]>
  • Loading branch information
esdrubal and mohammadfawaz authored Feb 3, 2023
1 parent 880c22f commit 5e67357
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 6 deletions.
52 changes: 49 additions & 3 deletions sway-core/src/semantic_analysis/namespace/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ use crate::{
CompileResult, Ident,
};

use super::{module::Module, root::Root, submodule_namespace::SubmoduleNamespace, Path, PathBuf};
use super::{
module::Module, root::Root, submodule_namespace::SubmoduleNamespace,
trait_map::are_equal_minus_dynamic_types, Path, PathBuf,
};

use sway_error::error::CompileError;
use sway_types::{span::Span, Spanned};

use std::collections::VecDeque;
use std::{cmp::Ordering, collections::VecDeque};

/// The set of items that represent the namespace context passed throughout type checking.
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -217,6 +220,8 @@ impl Namespace {
let mut methods = local_methods;
methods.append(&mut type_methods);

let mut matching_method_decl_ids: Vec<DeclId> = vec![];

for decl_id in methods.into_iter() {
let method = check!(
CompileResult::from(decl_engine.get_function(decl_id.clone(), &decl_id.span())),
Expand All @@ -225,8 +230,49 @@ impl Namespace {
errors
);
if &method.name == method_name {
return ok(decl_id, warnings, errors);
matching_method_decl_ids.push(decl_id);
}
}

let matching_method_decl_id = match matching_method_decl_ids.len().cmp(&1) {
Ordering::Equal => matching_method_decl_ids.get(0).cloned(),
Ordering::Greater => {
// Case where multiple methods exist with the same name
// This is the case of https://github.com/FuelLabs/sway/issues/3633
// where multiple generic trait impls use the same method name but with different parameter types
let mut maybe_method_decl_id: Option<DeclId> = Option::None;
for decl_id in matching_method_decl_ids.clone().into_iter() {
let method = check!(
CompileResult::from(
decl_engine.get_function(decl_id.clone(), &decl_id.span())
),
return err(warnings, errors),
warnings,
errors
);
if method.parameters.len() == args_buf.len()
&& !method.parameters.iter().zip(args_buf.iter()).any(|(p, a)| {
!are_equal_minus_dynamic_types(engines, p.type_id, a.return_type)
})
{
maybe_method_decl_id = Some(decl_id);
break;
}
}
if let Some(matching_method_decl_id) = maybe_method_decl_id {
// In case one or more methods match the parameter types we return the first match.
Some(matching_method_decl_id)
} else {
// When we can't match any method with parameter types we still return the first method found
// This was the behavior before introducing the parameter type matching
matching_method_decl_ids.get(0).cloned()
}
}
Ordering::Less => None,
};

if let Some(method_decl_id) = matching_method_decl_id {
return ok(method_decl_id, warnings, errors);
}

if !args_buf
Expand Down
8 changes: 6 additions & 2 deletions sway-core/src/semantic_analysis/namespace/trait_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl TraitMap {
type_implementing_for: engines.help_out(type_id).to_string(),
second_impl_span: impl_span.clone(),
});
} else if types_are_subset {
} else if types_are_subset && (traits_are_subset || is_impl_self) {
for (name, decl_id) in trait_methods.iter() {
if map_trait_methods.get(name).is_some() {
let method = check!(
Expand Down Expand Up @@ -793,7 +793,11 @@ impl TraitMap {
}
}

fn are_equal_minus_dynamic_types(engines: Engines<'_>, left: TypeId, right: TypeId) -> bool {
pub(crate) fn are_equal_minus_dynamic_types(
engines: Engines<'_>,
left: TypeId,
right: TypeId,
) -> bool {
if left.index() == right.index() {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,38 @@ impl MyTriple<u64> for MyU64 {
}
}

pub struct A {
a: u64,
}

pub struct B {
b: u64,
}

pub struct C {
c: u64,
}

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

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

fn main() -> u64 {
let a = FooBarData {
value: 1u8
Expand Down Expand Up @@ -170,6 +202,9 @@ fn main() -> u64 {
};
let q = p.my_triple(1);

let r_b = B { b: 42 };
let r_c = C { c: 42 };

if c == 42u8
&& d
&& e == 9u64
Expand All @@ -179,7 +214,9 @@ fn main() -> u64 {
&& l == 50
&& n == 240
&& o == 360
&& q == 93 {
&& q == 93
&& A::convert(r_b).a == 42
&& A::convert(r_c).a == 42 {
42
} else {
7
Expand Down

0 comments on commit 5e67357

Please sign in to comment.