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

Fixes multiple trait constraints for same method. #3818

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions sway-core/src/decl_engine/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use std::{collections::BTreeMap, fmt};

use sway_types::Ident;

use crate::language::ty::TyFunctionSig;

use super::DeclId;

type SourceDecl = DeclId;
Expand Down Expand Up @@ -46,6 +48,14 @@ impl DeclMapping {
self.mapping.is_empty()
}

pub(crate) fn mapping(&self) -> Vec<(SourceDecl, DestinationDecl)> {
self.mapping.clone()
}

pub(crate) fn from_mapping(mapping: Vec<(SourceDecl, DestinationDecl)>) -> DeclMapping {
DeclMapping { mapping }
}

pub(crate) fn from_stub_and_impld_decl_ids(
stub_decl_ids: BTreeMap<Ident, DeclId>,
impld_decl_ids: BTreeMap<Ident, DeclId>,
Expand All @@ -59,6 +69,22 @@ impl DeclMapping {
DeclMapping { mapping }
}

pub(crate) fn from_stub_and_impld_decl_ids_with_function_sig(
stub_decl_ids: BTreeMap<Ident, DeclId>,
new_decl_ids: BTreeMap<(Ident, TyFunctionSig), DeclId>,
) -> DeclMapping {
let mut mapping = vec![];
for (stub_decl_name, stub_decl_id) in stub_decl_ids.into_iter() {
for ((new_decl_name, _), new_decl_id) in new_decl_ids.iter() {
if new_decl_name.as_str() != stub_decl_name.as_str() {
continue;
}
mapping.push((stub_decl_id.clone(), new_decl_id.clone()));
}
}
DeclMapping { mapping }
}

pub(crate) fn find_match(&self, decl_id: &SourceDecl) -> Option<DestinationDecl> {
for (source_decl_id, dest_decl_id) in self.mapping.iter() {
if **source_decl_id == **decl_id {
Expand Down
19 changes: 19 additions & 0 deletions sway-core/src/language/ty/declaration/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,3 +448,22 @@ impl TyFunctionParameter {
self.name.as_str() == "self"
}
}

#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct TyFunctionSig {
pub return_type: TypeId,
pub parameters: Vec<TypeId>,
}

impl TyFunctionSig {
pub fn from_fn_decl(fn_decl: &TyFunctionDeclaration) -> Self {
Self {
return_type: fn_decl.return_type,
parameters: fn_decl
.parameters
.iter()
.map(|p| p.type_id)
.collect::<Vec<_>>(),
}
}
}
55 changes: 54 additions & 1 deletion sway-core/src/language/ty/expression/expression_variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
decl_engine::*,
engine_threading::*,
language::{ty::*, *},
namespace::are_equal_minus_dynamic_types,
type_system::*,
};

Expand Down Expand Up @@ -616,7 +617,59 @@ impl ReplaceDecls for TyExpressionVariant {
ref mut arguments,
..
} => {
function_decl_id.replace_decls(decl_mapping, engines);
// Filter decl mapping so function declaration is replaced by another
// function declaration that matches argument types
// This is needed for when multiple trait constraints are used that replace the same trait function
// This is required when we have a function such as:
// ```
// fn function<T, E>(v1: T, v2: E) where T: Eq, E: Eq {
// v1 == v1
// v2 == v2
// }
//```
// if T and E are different types then v1 == v1 should call one trait
// implementation while v2 == v2 should call another one. In this case
// decl_mapping will have v1 == v1 function declarations replaced by
// the T implementation and v2 == v2 function declarations replaced by
// the E implementation, decl_mapping will have an original function
// declaration to be replaced by two different implemented function
// declaration. The code below filters the correct implementation to use.
let mut decl_mapping_filtered = vec![];
'decls: for (orig_decl_id, dest_decl_id) in decl_mapping.mapping() {
let all_parents = engines
.de()
.find_all_parents(engines, function_decl_id.clone());
if **function_decl_id != *orig_decl_id
&& !all_parents.iter().any(|f| **f == *orig_decl_id)
{
continue;
}

let decl = engines.de().get(dest_decl_id.clone());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding these changes here, does it work to instead copy the code from lines 645-660 and add it to the definition of replace_decls on DeclId?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inside a for loop, I could move the whole block from 620 to 669, but I think it looks better on the context it is now.

if let DeclWrapper::Function(fn_decl) = decl.clone() {
if fn_decl.parameters.len() != arguments.len() {
continue;
}
for (i, argument) in arguments.iter().enumerate() {
if !are_equal_minus_dynamic_types(
engines,
argument.1.return_type,
fn_decl.parameters[i].type_id,
) {
continue 'decls;
}
}
decl_mapping_filtered.push((orig_decl_id, dest_decl_id))
}
}

// No function with same argument type were found so we try to replace decls as usual
if decl_mapping_filtered.is_empty() {
decl_mapping_filtered = decl_mapping.mapping();
}

function_decl_id
.replace_decls(&DeclMapping::from_mapping(decl_mapping_filtered), engines);
let new_decl_id = function_decl_id
.clone()
.replace_decls_and_insert_new(decl_mapping, engines);
Expand Down
1 change: 1 addition & 0 deletions sway-core/src/semantic_analysis/namespace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub use items::Items;
pub use module::Module;
pub use namespace::Namespace;
pub use root::Root;
pub(crate) use trait_map::are_equal_minus_dynamic_types;
pub(super) use trait_map::TraitMap;

use sway_types::Ident;
Expand Down
6 changes: 5 additions & 1 deletion sway-core/src/semantic_analysis/namespace/trait_map.rs
Original file line number Diff line number Diff line change
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
20 changes: 15 additions & 5 deletions sway-core/src/type_system/type_parameter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ use crate::{
decl_engine::*,
engine_threading::*,
error::*,
language::{ty, CallPath},
language::{
ty::{self, TyFunctionSig},
CallPath,
},
semantic_analysis::*,
type_system::*,
};
Expand Down Expand Up @@ -191,7 +194,7 @@ impl TypeParameter {
let mut errors = vec![];

let mut original_method_ids: BTreeMap<Ident, DeclId> = BTreeMap::new();
let mut impld_method_ids: BTreeMap<Ident, DeclId> = BTreeMap::new();
let mut impld_method_ids: BTreeMap<(Ident, TyFunctionSig), DeclId> = BTreeMap::new();

for type_param in type_parameters.iter() {
let TypeParameter {
Expand Down Expand Up @@ -228,13 +231,20 @@ impl TypeParameter {
errors
);
original_method_ids.extend(trait_original_method_ids);
impld_method_ids.extend(trait_impld_method_ids);
for (key, value) in trait_impld_method_ids {
if let Ok(fn_decl) = ctx.decl_engine.get_function(value.clone(), access_span) {
impld_method_ids
.insert((key, TyFunctionSig::from_fn_decl(&fn_decl)), value);
}
}
}
}

if errors.is_empty() {
let decl_mapping =
DeclMapping::from_stub_and_impld_decl_ids(original_method_ids, impld_method_ids);
let decl_mapping = DeclMapping::from_stub_and_impld_decl_ids_with_function_sig(
original_method_ids,
impld_method_ids,
);
ok(decl_mapping, warnings, errors)
} else {
err(warnings, errors)
Expand Down
8 changes: 8 additions & 0 deletions sway-types/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,14 @@ impl fmt::Display for Ident {
#[derive(Debug, Clone)]
pub struct IdentUnique(BaseIdent);

impl IdentUnique {
esdrubal marked this conversation as resolved.
Show resolved Hide resolved
pub fn as_str(&self) -> &str {
self.0
.name_override_opt
.unwrap_or_else(|| self.0.span.as_str())
}
}

impl From<Ident> for IdentUnique {
fn from(item: Ident) -> Self {
IdentUnique(item)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ fn main() -> u64 {
assert(m.x == 12u64);
assert(m.y == 20u64);

// TODO(Esdrubal): reactivate this once fix in #3621 is merged
//test_ok_or(true, 0);
test_ok_or(true, 0);
test_ok_or(0, true);

42
}