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.

Sign up for GitHub

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

find_method_for_type improvements #6744

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

find_method_for_type improvements #6744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ebaac89
Fixes duplicate method error.
esdrubal Oct 28, 2024
c4f8f22
Adds insert_for_type in find_method_for_type.
esdrubal Nov 9, 2024
2c0a8f6
Changes find_method_for_type to check if constraints are satisfied.
esdrubal Nov 9, 2024
3ec2661
Adds tests and fixes find_method_for_type.
esdrubal Nov 20, 2024
0f5f033
Fixes cargo clippy.
esdrubal Nov 20, 2024
695f87f
Adds TypeFilter to TraitMap.
esdrubal Nov 25, 2024
e50e270
Some fixes, and adds get_method_safe_to_unify
esdrubal Jan 16, 2025
a401b54
Fixes clippy and tests not passing.
esdrubal Feb 17, 2025
0cd4843
Merge branch 'master' into esdrubal/find_method_for_type_improvements
JoshuaBatty Feb 17, 2025
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
15 changes: 15 additions & 0 deletions sway-core/src/engine_threading.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,21 @@ impl<T: OrdWithEngines> OrdWithEngines for Option<T> {
}
}

impl<T: OrdWithEngines> OrdWithEngines for Vec<T> {
fn cmp(&self, other: &Self, ctx: &OrdWithEnginesContext) -> Ordering {
match self.len().cmp(&other.len()) {
Ordering::Less => Ordering::Less,
Ordering::Equal => self
.iter()
.zip(other.iter())
.fold(Ordering::Equal, |accumulator, values| {
accumulator.then_with(|| values.0.cmp(values.1, ctx))
}),
Ordering::Greater => Ordering::Greater,
}
}
}

impl<T: OrdWithEngines> OrdWithEngines for Box<T> {
fn cmp(&self, other: &Self, ctx: &OrdWithEnginesContext) -> Ordering {
(**self).cmp(&(**other), ctx)
Expand Down
1 change: 1 addition & 0 deletions sway-core/src/semantic_analysis/ast_node/declaration/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@ impl ty::TyAbiDecl {
&Handler::default(),
CallPath::ident_to_fullpath(self.name.clone(), ctx.namespace()),
vec![],
vec![],
type_id,
&all_items,
&self.span,
Expand Down
1 change: 1 addition & 0 deletions sway-core/src/semantic_analysis/ast_node/declaration/auto_impl/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ where
&handler,
impl_trait.trait_name.clone(),
impl_trait.trait_type_arguments.clone(),
impl_trait.impl_type_parameters.clone(),
impl_trait.implementing_for.type_id,
&impl_trait.items,
&impl_trait.span,
Expand Down
2 changes: 2 additions & 0 deletions sway-core/src/semantic_analysis/ast_node/declaration/declaration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ impl TyDecl {
handler,
impl_trait.trait_name.clone(),
impl_trait.trait_type_arguments.clone(),
impl_trait.impl_type_parameters.clone(),
impl_trait.implementing_for.type_id,
&impl_trait.items,
&impl_trait.span,
Expand Down Expand Up @@ -309,6 +310,7 @@ impl TyDecl {
handler,
impl_trait.trait_name.clone(),
impl_trait.trait_type_arguments.clone(),
impl_trait.impl_type_parameters.clone(),
impl_trait.implementing_for.type_id,
impl_trait_items,
&impl_trait.span,
Expand Down
2 changes: 2 additions & 0 deletions sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ impl TyImplSelfOrTrait {
handler,
impl_trait.trait_name.clone(),
impl_trait.trait_type_arguments.clone(),
impl_trait.impl_type_parameters.clone(),
impl_trait.implementing_for.type_id,
&impl_trait.items,
&impl_trait.span,
Expand Down Expand Up @@ -819,6 +820,7 @@ fn type_check_trait_implementation(
&Handler::default(),
trait_name.clone(),
trait_type_arguments.to_vec(),
impl_type_parameters.to_vec(),
implementing_for,
&this_supertrait_impld_method_refs
.values()
Expand Down
3 changes: 3 additions & 0 deletions sway-core/src/semantic_analysis/ast_node/declaration/trait.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ impl TyTraitDecl {
handler,
CallPath::ident_to_fullpath(name.clone(), ctx.namespace),
new_type_parameters.iter().map(|x| x.into()).collect(),
new_type_parameters.clone(),
self_type,
&dummy_interface_surface,
&span,
Expand Down Expand Up @@ -234,6 +235,7 @@ impl TyTraitDecl {
handler,
CallPath::ident_to_fullpath(name.clone(), ctx.namespace()),
new_type_parameters.iter().map(|x| x.into()).collect(),
new_type_parameters.clone(),
self_type,
&dummy_interface_surface,
&span,
Expand Down Expand Up @@ -613,6 +615,7 @@ impl TyTraitDecl {
&Handler::default(),
trait_name.clone(),
type_arguments.to_vec(),
type_parameters.to_vec(),
type_id,
&all_items,
&trait_name.span(),
Expand Down
1 change: 1 addition & 0 deletions sway-core/src/semantic_analysis/ast_node/expression/typed_expression.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1982,6 +1982,7 @@ impl ty::TyExpression {
handler,
abi_name.clone(),
vec![],
vec![],
return_type,
&abi_items,
span,
Expand Down
127 changes: 117 additions & 10 deletions sway-core/src/semantic_analysis/namespace/trait_map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::{
},
type_system::{SubstTypes, TypeId},
IncludeSelf, SubstTypesContext, TraitConstraint, TypeArgument, TypeEngine, TypeInfo,
TypeSubstMap, UnifyCheck,
TypeParameter, TypeSubstMap, UnifyCheck,
};

use super::Module;
Expand Down Expand Up @@ -102,6 +102,7 @@ type TraitName = Arc<CallPath<TraitSuffix>>;
struct TraitKey {
name: TraitName,
type_id: TypeId,
type_id_type_parameters: Vec<TypeParameter>,
trait_decl_span: Option<Span>,
}

Expand All @@ -110,6 +111,10 @@ impl OrdWithEngines for TraitKey {
self.name
.cmp(&other.name, ctx)
.then_with(|| self.type_id.cmp(&other.type_id))
.then_with(|| {
self.type_id_type_parameters
.cmp(&other.type_id_type_parameters, ctx)
})
}
}

Expand Down Expand Up @@ -225,10 +230,11 @@ impl TraitMap {
/// declarations for the key `(trait_name, type_id)` whenever possible.
#[allow(clippy::too_many_arguments)]
pub(crate) fn insert(
&mut self,
handler: &Handler,
module: &mut Module,
trait_name: CallPath,
trait_type_args: Vec<TypeArgument>,
impl_type_parameters: Vec<TypeParameter>,
type_id: TypeId,
items: &[ResolvedTraitImplItem],
impl_span: &Span,
Expand All @@ -239,6 +245,25 @@ impl TraitMap {
) -> Result<(), ErrorEmitted> {
let type_id = engines.te().get_unaliased_type_id(type_id);

let mut type_id_type_parameters = match &*engines.te().get(type_id) {
TypeInfo::Enum(decl_id) => engines.de().get_enum(decl_id).type_parameters.clone(),
TypeInfo::Struct(decl_id) => engines.de().get_struct(decl_id).type_parameters.clone(),
_ => vec![],
};

// Copy impl type parameter trait constraint to type_id_type_parameters
for type_id_type_parameter in type_id_type_parameters.iter_mut() {
let impl_type_parameter = impl_type_parameters
.iter()
.filter(|t| t.type_id == type_id_type_parameter.type_id)
.last();
if let Some(impl_type_parameter) = impl_type_parameter {
type_id_type_parameter
.trait_constraints
.clone_from(&impl_type_parameter.trait_constraints);
}
}

handler.scope(|handler| {
let mut trait_items: TraitItems = HashMap::new();
for item in items.iter() {
Expand Down Expand Up @@ -267,14 +292,19 @@ impl TraitMap {
}
}

let trait_impls = self.get_impls_mut(engines, type_id);
let trait_impls = module
.current_items_mut()
.implemented_traits
.get_impls_mut(engines, type_id)
.clone();

// check to see if adding this trait will produce a conflicting definition
for TraitEntry {
key:
TraitKey {
name: map_trait_name,
type_id: map_type_id,
type_id_type_parameters: map_type_id_type_parameters,
trait_decl_span: _,
},
value:
Expand Down Expand Up @@ -360,6 +390,34 @@ impl TraitMap {
}
}

let mut trait_constraints_safified = true;
for (map_type_id_type_parameter, type_id_type_parameter) in
map_type_id_type_parameters
.iter()
.zip(type_id_type_parameters.iter())
{
// Check that type_id_type_parameter satisfies all trait constraints in map_type_id_type_parameter.
if type_id_type_parameter
.type_id
.is_concrete(engines, crate::TreatNumericAs::Abstract)
&& Self::check_if_trait_constraints_are_satisfied_for_type(
&Handler::default(),
module,
type_id_type_parameter.type_id,
&map_type_id_type_parameter.trait_constraints,
impl_span,
engines,
)
.is_err()
{
trait_constraints_safified = false;
}
}

if !trait_constraints_safified {
continue;
}

if matches!(is_extending_existing_impl, IsExtendingExistingImpl::No)
&& types_are_subset
&& traits_are_subset
Expand Down Expand Up @@ -438,11 +496,12 @@ impl TraitMap {
});

// even if there is a conflicting definition, add the trait anyway
self.insert_inner(
module.current_items_mut().implemented_traits.insert_inner(
trait_name,
impl_span.clone(),
trait_decl_span,
type_id,
type_id_type_parameters,
trait_items,
engines,
);
Expand All @@ -451,24 +510,28 @@ impl TraitMap {
})
}

#[allow(clippy::too_many_arguments)]
fn insert_inner(
&mut self,
trait_name: TraitName,
impl_span: Span,
trait_decl_span: Option<Span>,
type_id: TypeId,
type_id_type_parameters: Vec<TypeParameter>,
trait_methods: TraitItems,
engines: &Engines,
) {
let key = TraitKey {
name: trait_name,
name: trait_name.clone(),
type_id,
type_id_type_parameters,
trait_decl_span,
};
let value = TraitValue {
trait_items: trait_methods,
trait_items: trait_methods.clone(),
impl_span,
};

let entry = TraitEntry { key, value };
let mut trait_impls: TraitImpls = HashMap::<TypeRootFilter, Vec<TraitEntry>>::new();
let type_root_filter = Self::get_type_root_filter(engines, type_id);
Expand Down Expand Up @@ -504,10 +567,47 @@ impl TraitMap {
});

match pos {
Ok(pos) => self_vec[pos]
.value
.trait_items
.extend(oe.value.trait_items.clone()),
Ok(pos) => {
let mut skip_insert = false;

// If we have the same method in: impl<T> FromBytes for T
// and: impl FromBytes for DataPoint
// We keep the second implementation.
// We don't care for the order this is checked
#[allow(clippy::iter_over_hash_type)]
for (name, item) in oe.value.trait_items.iter() {
#[allow(clippy::iter_over_hash_type)]
for (existing_name, existing_item) in
self_vec[pos].value.trait_items.iter()
{
if name == existing_name {
if let (
TyTraitItem::Fn(fn_ref),
TyTraitItem::Fn(existing_fn_ref),
) = (
item.clone().expect_typed(),
existing_item.clone().expect_typed(),
) {
let method = engines.de().get_function(fn_ref.id());
let existing_method =
engines.de().get_function(existing_fn_ref.id());
if !existing_method.is_from_blanket_impl(engines)
&& method.is_from_blanket_impl(engines)
{
skip_insert = true;
}
}
}
}
}

if !skip_insert {
self_vec[pos]
.value
.trait_items
.extend(oe.value.trait_items.clone())
}
}
Err(pos) => self_vec.insert(pos, oe.clone()),
}
}
Expand Down Expand Up @@ -642,6 +742,7 @@ impl TraitMap {
TraitKey {
name: map_trait_name,
type_id: map_type_id,
type_id_type_parameters: map_type_id_constraints,
trait_decl_span: map_trait_decl_span,
},
value:
Expand All @@ -658,6 +759,7 @@ impl TraitMap {
impl_span.clone(),
map_trait_decl_span.clone(),
*type_id,
map_type_id_constraints.clone(),
map_trait_items.clone(),
engines,
);
Expand All @@ -667,6 +769,7 @@ impl TraitMap {
impl_span.clone(),
map_trait_decl_span.clone(),
*map_type_id,
map_type_id_constraints.clone(),
Self::filter_dummy_methods(
map_trait_items.clone(),
*type_id,
Expand Down Expand Up @@ -1190,6 +1293,10 @@ impl TraitMap {
)
})
.collect::<Vec<_>>(),
item_paths: candidates
.values()
.filter_map(|i| i.span(engines).to_string_path_with_line_col(engines.se()))
.collect::<Vec<String>>(),
span: symbol.span(),
},
)),
Expand Down
Loading
Loading