Fixing the implementation of supertraits to match Rust (FuelLabs#835)

sway-core/src/error.rs

@@ -724,8 +724,8 @@ pub enum CompileError {
     MoreThanOneEnumInstantiator { span: Span, ty: String },
     #[error("This enum variant represents the unit type, so it should not be instantiated with any value.")]
     UnnecessaryEnumInstantiator { span: Span },
-    #[error("Trait \"{name}\" does not exist in this scope.")]
-    TraitNotFound { name: Ident, span: Span },
+    #[error("Cannot find trait \"{name}\" in this scope.")]
+    TraitNotFound { name: String, span: Span },
     #[error("This expression is not valid on the left hand side of a reassignment.")]
     InvalidExpressionOnLhs { span: Span },
     #[error(
@@ -835,6 +835,20 @@ pub enum CompileError {
         trait_name: String,
         span: Span,
     },
+    #[error("The trait \"{supertrait_name}\" is not implemented for type \"{type_name}\"")]
+    SupertraitImplMissing {
+        supertrait_name: String,
+        type_name: String,
+        span: Span,
+    },
+    #[error(
+        "Implementation of trait \"{supertrait_name}\" is required by this bound in \"{trait_name}\""
+    )]
+    SupertraitImplRequired {
+        supertrait_name: String,
+        trait_name: String,
+        span: Span,
+    },
 }
 
 impl std::convert::From<TypeError> for CompileError {
@@ -1039,6 +1053,8 @@ impl CompileError {
             AsteriskWithAlias { span, .. } => span,
             AbiAsSupertrait { span, .. } => span,
             NameDefinedMultipleTimesForTrait { span, .. } => span,
+            SupertraitImplMissing { span, .. } => span,
+            SupertraitImplRequired { span, .. } => span,
         }
     }
 

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

@@ -383,6 +383,7 @@ pub struct TypedTraitDeclaration {
     pub(crate) interface_surface: Vec<TypedTraitFn>,
     pub(crate) methods: Vec<FunctionDeclaration>,
     pub(crate) type_parameters: Vec<TypeParameter>,
+    pub(crate) supertraits: Vec<Supertrait>,
     pub(crate) visibility: Visibility,
 }
 impl TypedTraitDeclaration {

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

@@ -12,7 +12,7 @@ use crate::{
 
 use sway_types::span::{join_spans, Span};
 
-use std::{collections::HashSet, sync::Arc};
+use std::sync::Arc;
 
 pub(crate) use crate::semantic_analysis::ast_node::declaration::ReassignmentLhs;
 
@@ -321,90 +321,36 @@ impl TypedAstNode {
                         Declaration::TraitDeclaration(TraitDeclaration {
                             name,
                             interface_surface,
-                            mut methods,
+                            methods,
                             type_parameters,
                             supertraits,
                             visibility,
                         }) => {
                             // type check the interface surface
-                            let mut interface_surface = check!(
+                            let interface_surface = check!(
                                 type_check_interface_surface(interface_surface, namespace),
                                 return err(warnings, errors),
                                 warnings,
                                 errors
                             );
 
-                            // A HashSet to keep track of the function names available to the
-                            // trait. Mainly used for error checking currently.
-                            let mut trait_method_names = HashSet::new();
-                            for interface in &interface_surface.clone() {
-                                let name = interface.name.span().span.as_str().to_string();
-                                trait_method_names.insert(name);
-                            }
-                            for method in &methods.clone() {
-                                let name = method.name.span().span.as_str().to_string();
-                                trait_method_names.insert(name);
-                            }
-                            for supertrait in supertraits {
+                            // Error checking. Make sure that each supertrait exists and that none
+                            // of the supertraits are actually an ABI declaration
+                            for supertrait in &supertraits {
                                 match namespace
                                     .get_call_path(&supertrait.name)
                                     .ok(&mut warnings, &mut errors)
                                 {
-                                    Some(TypedDeclaration::TraitDeclaration(supertrait_decl)) => {
-                                        // Augment the interface of the trait with the interface of
-                                        // each supertrait
-                                        let mut supertrait_surface =
-                                            supertrait_decl.interface_surface;
-                                        for supertrait_interface in &supertrait_surface {
-                                            let supertrait_interface_span =
-                                                supertrait_interface.name.span();
-                                            let supertrait_interface_name =
-                                                supertrait_interface_span.span.as_str().to_string();
-                                            if trait_method_names
-                                                .contains(&supertrait_interface_name)
-                                            {
-                                                errors.push(
-                                                    CompileError::NameDefinedMultipleTimesForTrait {
-                                                        fn_name: supertrait_interface_name,
-                                                        trait_name: name.span().span.as_str().to_string(),
-                                                        span: supertrait_interface_span.clone(),
-                                                    },
-                                                );
-                                            } else {
-                                                trait_method_names
-                                                    .insert(supertrait_interface_name);
-                                            }
-                                        }
-                                        interface_surface.append(&mut supertrait_surface);
-
-                                        // Augment the set of methods of the trait with the set of
-                                        // methods of each supertrait
-                                        let mut supertrait_methods = supertrait_decl.methods;
-                                        for supertrait_method in &supertrait_methods {
-                                            let supertrait_method_span =
-                                                supertrait_method.name.span();
-                                            let supertrait_method_name =
-                                                supertrait_method_span.span.as_str().to_string();
-                                            if trait_method_names.contains(&supertrait_method_name)
-                                            {
-                                                errors.push(
-                                                    CompileError::NameDefinedMultipleTimesForTrait {
-                                                        fn_name: supertrait_method_name,
-                                                        trait_name: name.span().span.as_str().to_string(),
-                                                        span: supertrait_method_span.clone(),
-                                                    },
-                                                );
-                                            } else {
-                                                trait_method_names.insert(supertrait_method_name);
-                                            }
-                                        }
-                                        methods.append(&mut supertrait_methods);
-                                    }
-                                    _ => {
+                                    Some(TypedDeclaration::TraitDeclaration(_)) => (),
+                                    Some(TypedDeclaration::AbiDeclaration(_)) => {
                                         errors.push(CompileError::AbiAsSupertrait {
                                             span: name.span().clone(),
-                                        });
+                                        })
                                     }
+                                    _ => errors.push(CompileError::TraitNotFound {
+                                        name: supertrait.name.span().as_str().to_string(),
+                                        span: name.span().clone(),
+                                    }),
                                 }
                             }
 
@@ -443,6 +389,7 @@ impl TypedAstNode {
                                     interface_surface,
                                     methods,
                                     type_parameters,
+                                    supertraits,
                                     visibility,
                                 });
                             namespace.insert(name, trait_decl.clone());

sway-core/src/semantic_analysis/syntax_tree.rs

@@ -9,7 +9,8 @@ use crate::{
     error::*,
     parse_tree::Purity,
     semantic_analysis::{
-        ast_node::Mode, retrieve_module, Namespace, NamespaceRef, TypeCheckArguments,
+        ast_node::Mode, namespace::arena::NamespaceWrapper, retrieve_module, Namespace,
+        NamespaceRef, TypeCheckArguments,
     },
     type_engine::*,
     AstNode, ParseTree,
@@ -172,6 +173,10 @@ impl TypedParseTree {
         // Keep a copy of the nodes as they are.
         let all_nodes = typed_tree_nodes.clone();
 
+        // Check that if trait B is a supertrait of trait A, and if A is implemented for type T,
+        // then B is also implemented for type T
+        errors.append(&mut check_supertraits(&all_nodes, &namespace));
+
         // Extract other interesting properties from the list.
         let mut mains = Vec::new();
         let mut declarations = Vec::new();
@@ -262,6 +267,81 @@ impl TypedParseTree {
     }
 }
 
+/// Given a list of typed AST nodes and a namespace, check whether all supertrait constraints are
+/// satisfied. We're basically checking the following condition:
+///    if trait B is implemented for type T, then trait A_i is also implemented for type T for
+///    every A_i such that A_i is a supertrait of B.
+///
+/// This nicely works for transitive supertraits as well.
+///
+fn check_supertraits(
+    typed_tree_nodes: &[TypedAstNode],
+    namespace: &NamespaceRef,
+) -> Vec<CompileError> {
+    let mut errors = vec![];
+    for node in typed_tree_nodes {
+        if let TypedAstNodeContent::Declaration(TypedDeclaration::ImplTrait {
+            trait_name,
+            span,
+            type_implementing_for,
+            ..
+        }) = &node.content
+        {
+            if let CompileResult {
+                value: Some(TypedDeclaration::TraitDeclaration(tr)),
+                ..
+            } = namespace.get_call_path(trait_name)
+            {
+                let supertraits = tr.supertraits;
+                for supertrait in &supertraits {
+                    if !typed_tree_nodes.iter().any(|search_node| {
+                        if let TypedAstNodeContent::Declaration(TypedDeclaration::ImplTrait {
+                            trait_name: search_node_trait_name,
+                            type_implementing_for: search_node_type_implementing_for,
+                            ..
+                        }) = &search_node.content
+                        {
+                            if let (
+                                CompileResult {
+                                    value: Some(TypedDeclaration::TraitDeclaration(tr1)),
+                                    ..
+                                },
+                                CompileResult {
+                                    value: Some(TypedDeclaration::TraitDeclaration(tr2)),
+                                    ..
+                                },
+                            ) = (
+                                namespace.get_call_path(search_node_trait_name),
+                                namespace.get_call_path(&supertrait.name),
+                            ) {
+                                return (tr1.name == tr2.name)
+                                    && (type_implementing_for
+                                        == search_node_type_implementing_for);
+                            }
+                        }
+                        false
+                    }) {
+                        // The two errors below should really be a single error (and a "note"),
+                        // but we don't have a way today to point to two separate locations in the
+                        // user code with a single error.
+                        errors.push(CompileError::SupertraitImplMissing {
+                            supertrait_name: supertrait.name.to_string(),
+                            type_name: type_implementing_for.friendly_type_str(),
+                            span: span.clone(),
+                        });
+                        errors.push(CompileError::SupertraitImplRequired {
+                            supertrait_name: supertrait.name.to_string(),
+                            trait_name: tr.name.to_string(),
+                            span: tr.name.span().clone(),
+                        });
+                    }
+                }
+            }
+        }
+    }
+    errors
+}
+
 fn disallow_impure_functions(
     declarations: &[TypedDeclaration],
     mains: &[TypedFunctionDeclaration],

test/src/e2e_vm_tests/mod.rs

@@ -83,8 +83,7 @@ pub fn run(filter_regex: Option<regex::Regex>) {
         ("tuple_indexing", ProgramState::Return(1)),
         ("tuple_access", ProgramState::Return(42)),
         ("funcs_with_generic_types", ProgramState::Return(1)), // true
-        ("supertraits_1", ProgramState::Return(1)),
-        ("supertraits_2", ProgramState::Return(1)),
+        ("supertraits", ProgramState::Return(1)),
         ("new_allocator_test", ProgramState::Return(42)), // true
     ];
 
@@ -128,10 +127,9 @@ pub fn run(filter_regex: Option<regex::Regex>) {
         "star_import_alias",
         "item_used_without_import",
         "shadow_import",
-        "supertrait_dup_methods_1",
-        "supertrait_dup_methods_2",
-        "missing_supertrait",
         "missing_supertrait_impl",
+        "missing_func_from_supertrait_impl",
+        "supertrait_does_not_exist",
     ];
     number_of_tests_run += negative_project_names.iter().fold(0, |acc, name| {
         if filter(name) {

test/src/e2e_vm_tests/test_programs/supertraits_dup_methods_2/Forc.toml → test/src/e2e_vm_tests/test_programs/missing_func_from_supertrait_impl/Forc.toml

@@ -2,4 +2,4 @@
 author = "Fuel Labs <[email protected]>"
 entry = "main.sw"
 license = "Apache-2.0"
-name = "supertraits_dup_methods_2"
+name = "missing_func_from_supertrait_impl"

test/src/e2e_vm_tests/test_programs/missing_func_from_supertrait_impl/src/main.sw

@@ -0,0 +1,50 @@
+script;
+
+trait A {
+    fn a1();
+    fn a2();
+}
+
+trait B: A {
+    fn b();
+}
+
+trait C {
+    fn c();
+}
+
+trait D: C + B  {
+    fn d();
+}
+
+struct X { x: u64 }
+impl A for X {
+    fn a1() { }
+    fn a2() { }
+}
+impl B for X {
+    fn b() { }
+}
+impl C for X {
+    fn c() { }
+}
+impl D for X {
+    fn d() { }
+}
+
+struct Y { y: u64 }
+// This code shouldn't compile because the implementation of `A` below is missing `fn a2()`:
+impl A for Y {
+    fn a1() { }
+}
+impl B for Y {
+    fn b() { }
+}
+impl C for Y {
+    fn c() { }
+}
+impl D for Y {
+    fn d() { }
+}
+
+fn main() { }