Skip to content

Commit

Permalink
Fixes issue with generic trait impls using the same method names.
Browse files Browse the repository at this point in the history
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 #3633.
  • Loading branch information
esdrubal committed Jan 3, 2023
1 parent 3bfc15d commit 2cfa494
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 10 deletions.
64 changes: 57 additions & 7 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 @@ -210,6 +213,8 @@ impl Namespace {
let mut methods = local_methods;
methods.append(&mut type_methods);

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

for decl_id in methods.into_iter() {
let method = check!(
CompileResult::from(
Expand All @@ -220,12 +225,57 @@ impl Namespace {
errors
);
if &method.name == method_name {
// if we find the method that we are looking for, we also need
// to retrieve the impl definitions for the return type so that
// the user can string together method calls
self.insert_trait_implementation_for_type(engines, method.return_type);
return ok(decl_id, warnings, errors);
matching_method_decl_ids.push(decl_id);
}
}

let mut matching_method_decl_id = None;
match matching_method_decl_ids.len().cmp(&1) {
Ordering::Equal => {
matching_method_decl_id = Some(matching_method_decl_ids[0].clone());
}
Ordering::Greater => {
let mut matching_method_decl_ids2: Vec<DeclarationId> = vec![];
for decl_id in matching_method_decl_ids.clone().into_iter() {
let method = check!(
CompileResult::from(
declaration_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(type_engine, p.type_id, a.return_type)
})
{
matching_method_decl_ids2.push(decl_id);
}
}
if matching_method_decl_ids2.is_empty() {
matching_method_decl_id = Some(matching_method_decl_ids[0].clone());
} else {
matching_method_decl_id = Some(matching_method_decl_ids2[0].clone());
}
}
Ordering::Less => {}
}

if let Some(method_decl_id) = matching_method_decl_id {
let method = check!(
CompileResult::from(
declaration_engine.get_function(method_decl_id.clone(), &method_decl_id.span())
),
return err(warnings, errors),
warnings,
errors
);
// if we find the method that we are looking for, we also need
// to retrieve the impl definitions for the return type so that
// the user can string together method calls
self.insert_trait_implementation_for_type(engines, method.return_type);
return ok(method_decl_id, warnings, errors);
}

if !args_buf
Expand Down
39 changes: 37 additions & 2 deletions sway-core/src/semantic_analysis/namespace/trait_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,14 @@ impl TraitMap {
second_impl_span: impl_span.clone(),
});
} else if types_are_subset {
let type_args_are_equal = trait_type_args.len() == map_trait_type_args.len()
&& !trait_type_args
.iter()
.zip(map_trait_type_args.iter())
.any(|(a1, a2)| !a1.eq(a2, engines));

for (name, decl_id) in trait_methods.iter() {
if map_trait_methods.get(name).is_some() {
if let Some(map_trait_method_decl_id) = map_trait_methods.get(name) {
let method = check!(
CompileResult::from(
declaration_engine.get_function(decl_id.clone(), impl_span)
Expand All @@ -201,6 +207,31 @@ impl TraitMap {
warnings,
errors
);
let map_trait_method = check!(
CompileResult::from(
declaration_engine
.get_function(map_trait_method_decl_id.clone(), impl_span)
),
return err(warnings, errors),
warnings,
errors
);
if !type_args_are_equal
&& (method.parameters.len() != map_trait_method.parameters.len()
|| method
.parameters
.iter()
.zip(map_trait_method.parameters.iter())
.any(|(p1, p2)| {
!are_equal_minus_dynamic_types(
type_engine,
p1.type_id,
p2.type_id,
)
}))
{
continue;
}
errors.push(CompileError::DuplicateMethodsDefinedForType {
func_name: method.name.to_string(),
type_implementing_for: engines.help_out(type_id).to_string(),
Expand Down Expand Up @@ -792,7 +823,11 @@ impl TraitMap {
}
}

fn are_equal_minus_dynamic_types(type_engine: &TypeEngine, left: TypeId, right: TypeId) -> bool {
pub fn are_equal_minus_dynamic_types(
type_engine: &TypeEngine,
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 @@ -62,6 +62,33 @@ impl Data<u8> {
}
}

pub struct A {
a: u64,
}

pub struct B {
b: u64,
}

pub struct C {
c: u64,
}

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

impl Convert<B> for A {
fn convert(t: u64) {
}
}

impl Convert<C> for A {
// duplicate definition
fn convert(t: u64) {
}
}

fn main() {

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@ category = "fail"

# check: $()fn my_add(self, other: Self) -> Self {
# check: $()Duplicate definitions for the method "my_add" for type "Data<u32>".

# check: $()fn convert(t: u64) {
# check: $()Duplicate definitions for the method "convert" for type "A".
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 2cfa494

Please sign in to comment.