Fixes trait item method DCA warning. (FuelLabs#4229)

## Description ```rust pub trait MyTrait { fn trait_method(self) -> bool; } { fn item_method(self) -> bool { true } } ``` When a trait implements an item method in a library, the compiler should not throw a warning when the method is not used as it may be used by any Sway program that imports the library. Closes FuelLabs#3674. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: Mohammad Fawaz <[email protected]>
OxArmin · Mar 15, 2023 · 707bf2e · 707bf2e
1 parent 5f452e5
commit 707bf2e
Show file tree

Hide file tree

Showing 15 changed files with 191 additions and 14 deletions.
diff --git a/sway-core/src/control_flow_analysis/dead_code_analysis.rs b/sway-core/src/control_flow_analysis/dead_code_analysis.rs
@@ -379,12 +379,12 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
         }
         TraitDeclaration { decl_id, .. } => {
             let trait_decl = decl_engine.get_trait(decl_id);
-            connect_trait_declaration(&trait_decl, graph, entry_node);
+            connect_trait_declaration(&trait_decl, graph, entry_node, tree_type);
             Ok(leaves.to_vec())
         }
         AbiDeclaration { decl_id, .. } => {
             let abi_decl = decl_engine.get_abi(decl_id);
-            connect_abi_declaration(engines, &abi_decl, graph, entry_node)?;
+            connect_abi_declaration(engines, &abi_decl, graph, entry_node, tree_type)?;
             Ok(leaves.to_vec())
         }
         StructDeclaration { decl_id, .. } => {
@@ -399,7 +399,10 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
         }
         ImplTrait { decl_id, .. } => {
             let ty::TyImplTrait {
-                trait_name, items, ..
+                trait_name,
+                items,
+                trait_decl_ref,
+                ..
             } = decl_engine.get_impl_trait(decl_id);
 
             connect_impl_trait(
@@ -409,6 +412,7 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
                 &items,
                 entry_node,
                 tree_type,
+                trait_decl_ref,
                 options,
             )?;
             Ok(leaves.to_vec())
@@ -478,13 +482,15 @@ fn connect_struct_declaration<'eng: 'cfg, 'cfg>(
 /// that the declaration was indeed at some point implemented.
 /// Additionally, we insert the trait's methods into the method namespace in order to
 /// track which exact methods are dead code.
+#[allow(clippy::too_many_arguments)]
 fn connect_impl_trait<'eng: 'cfg, 'cfg>(
     engines: Engines<'eng>,
     trait_name: &CallPath,
     graph: &mut ControlFlowGraph<'cfg>,
     items: &[TyImplItem],
     entry_node: NodeIndex,
     tree_type: &TreeType,
+    trait_decl_ref: Option<DeclRef<InterfaceDeclId>>,
     options: NodeConnectionOptions,
 ) -> Result<(), CompileError> {
     let decl_engine = engines.de();
@@ -496,9 +502,23 @@ fn connect_impl_trait<'eng: 'cfg, 'cfg>(
         }
         Some(trait_decl_node) => {
             graph.add_edge_from_entry(entry_node, "".into());
-            graph.add_edge(entry_node, trait_decl_node, "".into());
+            graph.add_edge(entry_node, trait_decl_node.trait_idx, "".into());
         }
     };
+    let trait_entry = graph.namespace.find_trait(trait_name).cloned();
+    // Collect the methods that are directly implemented in the trait.
+    let mut trait_items_method_names = Vec::new();
+    if let Some(trait_decl_ref) = trait_decl_ref {
+        if let InterfaceDeclId::Trait(trait_decl_id) = &trait_decl_ref.id() {
+            let trait_decl = decl_engine.get_trait(trait_decl_id);
+            for trait_item in trait_decl.items {
+                if let ty::TyTraitItem::Fn(func_decl_ref) = trait_item {
+                    let functional_decl_id = decl_engine.get_function(&func_decl_ref);
+                    trait_items_method_names.push(functional_decl_id.name.as_str().to_string());
+                }
+            }
+        }
+    }
     let mut methods_and_indexes = vec![];
     // insert method declarations into the graph
     for item in items {
@@ -511,7 +531,23 @@ fn connect_impl_trait<'eng: 'cfg, 'cfg>(
                     method_decl_ref: method_decl_ref.clone(),
                     engines,
                 });
-                if matches!(tree_type, TreeType::Library { .. } | TreeType::Contract) {
+                let add_edge_to_fn_decl =
+                    if trait_items_method_names.contains(&fn_decl.name.as_str().to_string()) {
+                        if let Some(trait_entry) = trait_entry.clone() {
+                            matches!(
+                                trait_entry.module_tree_type,
+                                TreeType::Library { .. } | TreeType::Contract
+                            )
+                        } else {
+                            // trait_entry not found which means it is an external trait.
+                            // As the trait is external we assume it is within a library
+                            // thus we can return true directly.
+                            true
+                        }
+                    } else {
+                        matches!(tree_type, TreeType::Library { .. } | TreeType::Contract)
+                    };
+                if add_edge_to_fn_decl {
                     graph.add_edge(entry_node, fn_decl_entry_node, "".into());
                 }
                 // connect the impl declaration node to the functions themselves, as all trait functions are
@@ -558,14 +594,18 @@ fn connect_trait_declaration(
     decl: &ty::TyTraitDeclaration,
     graph: &mut ControlFlowGraph,
     entry_node: NodeIndex,
+    tree_type: &TreeType,
 ) {
     graph.namespace.add_trait(
         CallPath {
             prefixes: vec![],
             suffix: decl.name.clone(),
             is_absolute: false,
         },
-        entry_node,
+        TraitNamespaceEntry {
+            trait_idx: entry_node,
+            module_tree_type: tree_type.clone(),
+        },
     );
 }
 
@@ -575,6 +615,7 @@ fn connect_abi_declaration(
     decl: &ty::TyAbiDeclaration,
     graph: &mut ControlFlowGraph,
     entry_node: NodeIndex,
+    tree_type: &TreeType,
 ) -> Result<(), CompileError> {
     let type_engine = engines.te();
     let decl_engine = engines.de();
@@ -585,7 +626,10 @@ fn connect_abi_declaration(
             suffix: decl.name.clone(),
             is_absolute: false,
         },
-        entry_node,
+        TraitNamespaceEntry {
+            trait_idx: entry_node,
+            module_tree_type: tree_type.clone(),
+        },
     );
 
     // If a struct type is used as a return type in the interface surface
@@ -1377,10 +1421,10 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
         AbiName(abi_name) => {
             if let crate::type_system::AbiName::Known(abi_name) = abi_name {
                 // abis are treated as traits here
-                let decl = graph.namespace.find_trait(abi_name).cloned();
-                if let Some(decl_node) = decl {
+                let entry = graph.namespace.find_trait(abi_name).cloned();
+                if let Some(entry) = entry {
                     for leaf in leaves {
-                        graph.add_edge(*leaf, decl_node, "".into());
+                        graph.add_edge(*leaf, entry.trait_idx, "".into());
                     }
                 }
             }

diff --git a/sway-core/src/control_flow_analysis/flow_graph/mod.rs b/sway-core/src/control_flow_analysis/flow_graph/mod.rs
@@ -17,6 +17,7 @@ use petgraph::{graph::EdgeIndex, prelude::NodeIndex};
 mod namespace;
 use namespace::ControlFlowNamespace;
 pub(crate) use namespace::FunctionNamespaceEntry;
+pub(crate) use namespace::TraitNamespaceEntry;
 
 pub type EntryPoint = NodeIndex;
 pub type ExitPoint = NodeIndex;

diff --git a/sway-core/src/control_flow_analysis/flow_graph/namespace.rs b/sway-core/src/control_flow_analysis/flow_graph/namespace.rs
@@ -1,6 +1,7 @@
 use super::{EntryPoint, ExitPoint};
 use crate::{
     language::{
+        parsed::TreeType,
         ty::{self, TyFunctionDeclaration, TyFunctionSig},
         CallPath,
     },
@@ -27,6 +28,12 @@ pub(crate) struct StructNamespaceEntry {
     pub(crate) fields: HashMap<String, NodeIndex>,
 }
 
+#[derive(Clone, Debug)]
+pub(crate) struct TraitNamespaceEntry {
+    pub(crate) trait_idx: NodeIndex,
+    pub(crate) module_tree_type: TreeType,
+}
+
 #[derive(Default, Clone)]
 /// This namespace holds mappings from various declarations to their indexes in the graph. This is
 /// used for connecting those vertices when the declarations are instantiated.
@@ -37,7 +44,7 @@ pub(crate) struct StructNamespaceEntry {
 pub struct ControlFlowNamespace {
     pub(crate) function_namespace: HashMap<(IdentUnique, TyFunctionSig), FunctionNamespaceEntry>,
     pub(crate) enum_namespace: HashMap<IdentUnique, (NodeIndex, HashMap<Ident, NodeIndex>)>,
-    pub(crate) trait_namespace: HashMap<CallPath, NodeIndex>,
+    pub(crate) trait_namespace: HashMap<CallPath, TraitNamespaceEntry>,
     /// This is a mapping from trait name to method names and their node indexes
     pub(crate) trait_method_namespace: HashMap<CallPath, HashMap<Ident, NodeIndex>>,
     /// This is a mapping from struct name to field names and their node indexes
@@ -110,11 +117,11 @@ impl ControlFlowNamespace {
         Some((*enum_ix, *enum_decl.get(variant_name)?))
     }
 
-    pub(crate) fn add_trait(&mut self, trait_name: CallPath, trait_idx: NodeIndex) {
-        self.trait_namespace.insert(trait_name, trait_idx);
+    pub(crate) fn add_trait(&mut self, trait_name: CallPath, trait_entry: TraitNamespaceEntry) {
+        self.trait_namespace.insert(trait_name, trait_entry);
     }
 
-    pub(crate) fn find_trait(&self, name: &CallPath) -> Option<&NodeIndex> {
+    pub(crate) fn find_trait(&self, name: &CallPath) -> Option<&TraitNamespaceEntry> {
         self.trait_namespace.get(name)
     }
 

diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method/Forc.lock
@@ -0,0 +1,8 @@
+[[package]]
+name = 'trait_method'
+source = 'member'
+dependencies = ['trait_method_lib']
+
+[[package]]
+name = 'trait_method_lib'
+source = 'path+from-root-BB9C49E98039D004'
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method/Forc.toml
@@ -0,0 +1,9 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "trait_method"
+implicit-std = false
+
+[dependencies]
+trait_method_lib = { path = "../trait_method_lib" }
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method/src/main.sw
@@ -0,0 +1,29 @@
+script;
+
+use trait_method_lib::*;
+
+pub trait MyTrait2 {
+    fn trait_method2(self) -> bool;
+} {
+    fn method2(self) -> MyStruct {
+        MyStruct {}
+    }
+}
+
+impl MyTrait for MyStruct {
+    fn trait_method(self) -> bool {
+        true
+    }
+}
+
+impl MyTrait2 for MyStruct {
+    fn trait_method2(self) -> bool {
+        true
+    }
+}
+
+fn main() {
+    let s = MyStruct {};
+    let b = s.trait_method();
+    let b = s.trait_method2();
+}
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method/test.toml b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method/test.toml
@@ -0,0 +1,7 @@
+category = "compile"
+
+# check: $()fn method2(self) -> MyStruct {
+# check: $()This method is never called.
+
+expected_warnings = 1
+
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_lib/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_lib/Forc.lock
@@ -0,0 +1,3 @@
+[[package]]
+name = 'trait_method_lib'
+source = 'member'
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_lib/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_lib/Forc.toml
@@ -0,0 +1,6 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "trait_method_lib"
+implicit-std = false
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_lib/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_lib/src/main.sw
@@ -0,0 +1,13 @@
+library;
+
+pub struct MyStruct {
+
+}
+
+pub trait MyTrait {
+    fn trait_method(self) -> bool;
+} {
+    fn method(self) -> MyStruct {
+        MyStruct {}
+    }
+}
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_lib/test.toml b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_lib/test.toml
@@ -0,0 +1,3 @@
+category = "compile"
+expected_warnings = 0
+
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_neq/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_neq/Forc.lock
@@ -0,0 +1,13 @@
+[[package]]
+name = 'core'
+source = 'path+from-root-A6EAB8F92CD55881'
+
+[[package]]
+name = 'std'
+source = 'path+from-root-A6EAB8F92CD55881'
+dependencies = ['core']
+
+[[package]]
+name = 'trait_method_neq'
+source = 'member'
+dependencies = ['std']
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_neq/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_neq/Forc.toml
@@ -0,0 +1,8 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "trait_method_neq"
+
+[dependencies]
+std = { path = "../../../../../../../sway-lib-std" }
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_neq/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_neq/src/main.sw
@@ -0,0 +1,23 @@
+script;
+fn main() -> u64 {
+    let a = 255;
+
+    enum X {
+        Y: bool,
+    }
+
+    impl core::ops::Eq for X {
+        fn eq(self, other: Self) -> bool {
+            asm(r1: self, r2: other, r3) {
+                eq r3 r2 r1;
+                r3: bool
+            }
+        }
+    }
+
+    if X::Y(true) == X::Y(true) {
+        a
+    } else {
+        a
+    }
+}
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_neq/test.toml b/test/src/e2e_vm_tests/test_programs/should_pass/dca/trait_method_neq/test.toml
@@ -0,0 +1,3 @@
+category = "compile"
+expected_warnings = 0
+