Improved trait coherence checking

sway-core/src/language/ty/program.rs

@@ -423,6 +423,31 @@ impl TyProgram {
             }
         }
 
+        // check trait overlap
+        let mut unified_trait_map = root
+            .namespace
+            .root_module()
+            .root_lexical_scope()
+            .items
+            .implemented_traits
+            .clone();
+        unified_trait_map.check_overlap_and_extend(handler, unified_trait_map.clone(), engines)?;
+
+        for (_, submodule) in root.submodules.iter() {
+            unified_trait_map.check_overlap_and_extend(
+                handler,
+                submodule
+                    .module
+                    .namespace
+                    .module(engines)
+                    .root_lexical_scope()
+                    .items
+                    .implemented_traits
+                    .clone(),
+                engines,
+            )?;
+        }
+
         Ok((typed_program_kind, declarations, configurables))
     }
 

sway-core/src/semantic_analysis/ast_node/declaration/abi.rs

@@ -429,6 +429,7 @@ impl ty::TyAbiDecl {
                 CallPath::ident_to_fullpath(self.name.clone(), ctx.namespace()),
                 vec![],
                 type_id,
+                vec![],
                 &all_items,
                 &self.span,
                 Some(self.span()),

sway-core/src/semantic_analysis/ast_node/declaration/auto_impl.rs

@@ -416,6 +416,7 @@ where
                 impl_trait.trait_name.clone(),
                 impl_trait.trait_type_arguments.clone(),
                 impl_trait.implementing_for.type_id,
+                impl_trait.impl_type_parameters.clone(),
                 &impl_trait.items,
                 &impl_trait.span,
                 impl_trait

sway-core/src/semantic_analysis/ast_node/declaration/declaration.rs

@@ -243,6 +243,7 @@ impl TyDecl {
                         impl_trait.trait_name.clone(),
                         impl_trait.trait_type_arguments.clone(),
                         impl_trait.implementing_for.type_id,
+                        impl_trait.impl_type_parameters.clone(),
                         &impl_trait.items,
                         &impl_trait.span,
                         impl_trait
@@ -310,6 +311,7 @@ impl TyDecl {
                     impl_trait.trait_name.clone(),
                     impl_trait.trait_type_arguments.clone(),
                     impl_trait.implementing_for.type_id,
+                    impl_trait.impl_type_parameters.clone(),
                     impl_trait_items,
                     &impl_trait.span,
                     impl_trait

sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs

@@ -504,6 +504,7 @@ impl TyImplSelfOrTrait {
                         impl_trait.trait_name.clone(),
                         impl_trait.trait_type_arguments.clone(),
                         impl_trait.implementing_for.type_id,
+                        impl_trait.impl_type_parameters.clone(),
                         &impl_trait.items,
                         &impl_trait.span,
                         impl_trait
@@ -764,6 +765,7 @@ fn type_check_trait_implementation(
             trait_name.clone(),
             trait_type_arguments.to_vec(),
             implementing_for,
+            impl_type_parameters.to_vec(),
             &this_supertrait_impld_method_refs
                 .values()
                 .cloned()

sway-core/src/semantic_analysis/ast_node/declaration/trait.rs

@@ -167,6 +167,7 @@ impl TyTraitDecl {
                     CallPath::ident_to_fullpath(name.clone(), ctx.namespace),
                     new_type_parameters.iter().map(|x| x.into()).collect(),
                     self_type,
+                    vec![],
                     &dummy_interface_surface,
                     &span,
                     None,
@@ -235,6 +236,7 @@ impl TyTraitDecl {
                     CallPath::ident_to_fullpath(name.clone(), ctx.namespace()),
                     new_type_parameters.iter().map(|x| x.into()).collect(),
                     self_type,
+                    vec![],
                     &dummy_interface_surface,
                     &span,
                     None,
@@ -614,6 +616,7 @@ impl TyTraitDecl {
             trait_name.clone(),
             type_arguments.to_vec(),
             type_id,
+            vec![],
             &all_items,
             &trait_name.span(),
             Some(self.span()),

sway-core/src/semantic_analysis/ast_node/expression/typed_expression.rs

@@ -1987,6 +1987,7 @@ impl ty::TyExpression {
             abi_name.clone(),
             vec![],
             return_type,
+            vec![],
             &abi_items,
             span,
             Some(span.clone()),

sway-core/src/semantic_analysis/namespace/trait_map.rs

@@ -24,7 +24,7 @@ use crate::{
     },
     type_system::{SubstTypes, TypeId},
     IncludeSelf, SubstTypesContext, TraitConstraint, TypeArgument, TypeEngine, TypeInfo,
-    TypeSubstMap, UnifyCheck,
+    TypeParameter, TypeSubstMap, UnifyCheck,
 };
 
 use super::Module;
@@ -102,14 +102,18 @@ type TraitName = Arc<CallPath<TraitSuffix>>;
 struct TraitKey {
     name: TraitName,
     type_id: TypeId,
+    impl_type_parameters: Vec<TypeParameter>,
     trait_decl_span: Option<Span>,
 }
 
 impl OrdWithEngines for TraitKey {
     fn cmp(&self, other: &Self, ctx: &OrdWithEnginesContext) -> std::cmp::Ordering {
-        self.name
-            .cmp(&other.name, ctx)
-            .then_with(|| self.type_id.cmp(&other.type_id))
+        self.name.cmp(&other.name, ctx).then_with(|| {
+            self.type_id.cmp(&other.type_id).then_with(|| {
+                self.impl_type_parameters
+                    .cmp(&other.impl_type_parameters, ctx)
+            })
+        })
     }
 }
 
@@ -221,6 +225,7 @@ impl TraitMap {
         trait_name: CallPath,
         trait_type_args: Vec<TypeArgument>,
         type_id: TypeId,
+        impl_type_parameters: Vec<TypeParameter>,
         items: &[ResolvedTraitImplItem],
         impl_span: &Span,
         trait_decl_span: Option<Span>,
@@ -265,6 +270,7 @@ impl TraitMap {
                         name: map_trait_name,
                         type_id: map_type_id,
                         trait_decl_span: _,
+                        impl_type_parameters: _,
                     },
                 value:
                     TraitValue {
@@ -448,6 +454,7 @@ impl TraitMap {
                 impl_span.clone(),
                 trait_decl_span,
                 type_id,
+                impl_type_parameters,
                 trait_items,
                 engines,
             );
@@ -462,13 +469,15 @@ impl TraitMap {
         impl_span: Span,
         trait_decl_span: Option<Span>,
         type_id: TypeId,
+        impl_type_parameters: Vec<TypeParameter>,
         trait_methods: TraitItems,
         engines: &Engines,
     ) {
         let key = TraitKey {
             name: trait_name,
             type_id,
             trait_decl_span,
+            impl_type_parameters,
         };
         let value = TraitValue {
             trait_items: trait_methods,
@@ -631,6 +640,141 @@ impl TraitMap {
         }
     }
 
+    fn get_traits_types(
+        &self,
+        traits_types: &mut HashMap<CallPath, Vec<TypeId>>,
+    ) -> Result<(), ErrorEmitted> {
+        let mut keys = self.trait_impls.keys().clone().collect::<Vec<_>>();
+        keys.sort();
+        for key in keys {
+            for self_entry in self.trait_impls[key].iter() {
+                let callpath = CallPath {
+                    prefixes: self_entry.key.name.prefixes.clone(),
+                    suffix: self_entry.key.name.suffix.name.clone(),
+                    is_absolute: self_entry.key.name.is_absolute,
+                };
+                println!(
+                    "get_traits_types {:?} {}",
+                    callpath,
+                    self_entry.key.type_id.index()
+                );
+                if let Some(vec) = traits_types.get_mut(&callpath) {
+                    vec.push(self_entry.key.type_id);
+                } else {
+                    traits_types.insert(callpath, vec![self_entry.key.type_id]);
+                }
+            }
+        }
+        Ok(())
+    }
+
+    /// Given [TraitMap]s `self` and `other`, checks for overlaps between `self` and `other`.
+    /// If no overlaps are found extends `self` with `other`.
+    pub(crate) fn check_overlap_and_extend(
+        &mut self,
+        handler: &Handler,
+        other: TraitMap,
+        engines: &Engines,
+    ) -> Result<(), ErrorEmitted> {
+        let mut overlap_err = None;
+        let unify_check = UnifyCheck::coercion(engines);
+        let mut keys = self.trait_impls.keys().clone().collect::<Vec<_>>();
+        keys.sort();
+        let mut traits_types = HashMap::<CallPath, Vec<TypeId>>::new();
+        self.get_traits_types(&mut traits_types)?;
+        other.get_traits_types(&mut traits_types)?;
+
+        for key in keys {
+            for self_entry in self.trait_impls[key].iter() {
+                let self_tcs = self_entry
+                    .key
+                    .impl_type_parameters
+                    .iter()
+                    .map(|tp| tp.trait_constraints.iter().map(|tc| (tc, tp.clone())))
+                    .flatten()
+                    .collect::<Vec<_>>();
+
+                for other_entry in other.get_impls(engines, self_entry.key.type_id, true) {
+                    if self_entry.key.name.eq(
+                        &*other_entry.key.name,
+                        &PartialEqWithEnginesContext::new(engines),
+                    ) && self_entry.value.impl_span != other_entry.value.impl_span
+                        && unify_check.check(self_entry.key.type_id, other_entry.key.type_id)
+                    {
+                        let other_tcs = other_entry
+                            .key
+                            .impl_type_parameters
+                            .iter()
+                            .map(|tp| tp.trait_constraints.iter().map(|tc| (tc, tp.clone())))
+                            .flatten()
+                            .collect::<Vec<_>>();
+                        let other_tcs_satisfied = other_tcs.iter().all(|(tc, tp)| {
+                            if let Some(tc_type_ids) = traits_types.get(&tc.trait_name) {
+                                tc_type_ids.iter().any(|tc_type_id| {
+                                    let mut type_mapping = TypeSubstMap::new();
+                                    type_mapping.insert(tp.type_id, *tc_type_id);
+                                    let mut type_id = other_entry.key.type_id;
+                                    type_id.subst(&SubstTypesContext::new(
+                                        engines,
+                                        &type_mapping,
+                                        false,
+                                    ));
+                                    unify_check.check(self_entry.key.type_id, type_id)
+                                })
+                            } else {
+                                false
+                            }
+                        });
+
+                        let self_tcs_satisfied = self_tcs.iter().all(|(tc, tp)| {
+                            if let Some(tc_type_ids) = traits_types.get(&tc.trait_name) {
+                                tc_type_ids.iter().any(|tc_type_id| {
+                                    let mut type_mapping = TypeSubstMap::new();
+                                    type_mapping.insert(tp.type_id, *tc_type_id);
+                                    let mut type_id = self_entry.key.type_id;
+                                    type_id.subst(&SubstTypesContext::new(
+                                        engines,
+                                        &type_mapping,
+                                        false,
+                                    ));
+                                    unify_check.check(other_entry.key.type_id, type_id)
+                                })
+                            } else {
+                                false
+                            }
+                        });
+
+                        if other_tcs_satisfied && self_tcs_satisfied {
+                            for (trait_item_name1, _) in self_entry.value.trait_items.clone() {
+                                for (trait_item_name2, _) in other_entry.value.trait_items.clone() {
+                                    if trait_item_name1 == trait_item_name2 {
+                                        handler.emit_err(CompileError::InternalOwned(
+                                            format!("Overlapped item {}", trait_item_name1),
+                                            self_entry.value.impl_span.clone(),
+                                        ));
+                                        overlap_err =
+                                            Some(handler.emit_err(CompileError::InternalOwned(
+                                                format!("Overlapped item {}", trait_item_name1),
+                                                other_entry.value.impl_span.clone(),
+                                            )));
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        if let Some(overlap_err) = overlap_err {
+            return Err(overlap_err);
+        }
+
+        self.extend(other, engines);
+
+        Ok(())
+    }
+
     /// Filters the entries in `self` and return a new [TraitMap] with all of
     /// the entries from `self` that implement a trait from the declaration with that span.
     pub(crate) fn filter_by_trait_decl_span(&self, trait_decl_span: Span) -> TraitMap {
@@ -896,6 +1040,7 @@ impl TraitMap {
                         name: map_trait_name,
                         type_id: map_type_id,
                         trait_decl_span: map_trait_decl_span,
+                        impl_type_parameters: map_impl_type_parameters,
                     },
                 value:
                     TraitValue {
@@ -911,6 +1056,7 @@ impl TraitMap {
                         impl_span.clone(),
                         map_trait_decl_span.clone(),
                         *type_id,
+                        map_impl_type_parameters.clone(),
                         map_trait_items.clone(),
                         engines,
                     );
@@ -1009,6 +1155,7 @@ impl TraitMap {
                         impl_span.clone(),
                         map_trait_decl_span.clone(),
                         *type_id,
+                        map_impl_type_parameters.clone(),
                         trait_items,
                         engines,
                     );

sway-core/src/semantic_analysis/type_check_context.rs

@@ -19,7 +19,8 @@ use crate::{
         Namespace,
     },
     type_system::{SubstTypes, TypeArgument, TypeId, TypeInfo},
-    EnforceTypeArguments, SubstTypesContext, TraitConstraint, TypeSubstMap, UnifyCheck,
+    EnforceTypeArguments, SubstTypesContext, TraitConstraint, TypeParameter, TypeSubstMap,
+    UnifyCheck,
 };
 use sway_error::{
     error::CompileError,
@@ -1292,13 +1293,24 @@ impl<'a> TypeCheckContext<'a> {
         trait_name: CallPath,
         trait_type_args: Vec<TypeArgument>,
         type_id: TypeId,
+        mut impl_type_parameters: Vec<TypeParameter>,
         items: &[ty::TyImplItem],
         impl_span: &Span,
         trait_decl_span: Option<Span>,
         is_impl_self: IsImplSelf,
         is_extending_existing_impl: IsExtendingExistingImpl,
     ) -> Result<(), ErrorEmitted> {
         let engines = self.engines;
+
+        // Use trait name with full path, improves consistency between
+        // this inserting and getting in `get_methods_for_type_and_trait_name`.
+        let full_trait_name = trait_name.to_fullpath(self.engines(), self.namespace());
+        impl_type_parameters.iter_mut().for_each(|tp| {
+            tp.trait_constraints.iter_mut().for_each(|tc| {
+                tc.trait_name = tc.trait_name.to_fullpath(self.engines(), self.namespace())
+            })
+        });
+
         // CallPath::to_fullpath gives a resolvable path, but is not guaranteed to provide the path
         // to the actual trait declaration. Since the path of the trait declaration is used as a key
         // in the trait map, we need to find the actual declaration path.
@@ -1317,6 +1329,7 @@ impl<'a> TypeCheckContext<'a> {
                 canonical_trait_path,
                 trait_type_args,
                 type_id,
+                impl_type_parameters,
                 &items,
                 impl_span,
                 trait_decl_span,