Refactor and fix collect_types_metadata for TyProgram (FuelLabs#3839

)
cold-briu · Jan 24, 2023 · 9a980ac · 9a980ac
1 parent 5c4e3b4
commit 9a980ac
Show file tree

Hide file tree

Showing 27 changed files with 338 additions and 113 deletions.
diff --git a/sway-core/src/language/ty/ast_node.rs b/sway-core/src/language/ty/ast_node.rs
@@ -3,13 +3,8 @@ use std::fmt::{self, Debug};
 use sway_types::{Ident, Span};
 
 use crate::{
-    decl_engine::*,
-    engine_threading::*,
-    error::*,
-    language::{parsed, ty::*},
-    transform::AttributeKind,
-    type_system::*,
-    types::DeterministicallyAborts,
+    decl_engine::*, engine_threading::*, error::*, language::ty::*, transform::AttributeKind,
+    type_system::*, types::DeterministicallyAborts,
 };
 
 pub trait GetDeclIdent {
@@ -150,13 +145,8 @@ impl TyAstNode {
         ok(public, warnings, errors)
     }
 
-    /// Naive check to see if this node is a function declaration of a function called `main` if
-    /// the [TreeType] is Script or Predicate.
-    pub(crate) fn is_main_function(
-        &self,
-        decl_engine: &DeclEngine,
-        tree_type: parsed::TreeType,
-    ) -> CompileResult<bool> {
+    /// Check to see if this node is a function declaration with generic type parameters.
+    pub(crate) fn is_generic_function(&self, decl_engine: &DeclEngine) -> CompileResult<bool> {
         let mut warnings = vec![];
         let mut errors = vec![];
         match &self {
@@ -165,18 +155,15 @@ impl TyAstNode {
                 content: TyAstNodeContent::Declaration(TyDeclaration::FunctionDeclaration(decl_id)),
                 ..
             } => {
-                let TyFunctionDeclaration { name, .. } = check!(
+                let TyFunctionDeclaration {
+                    type_parameters, ..
+                } = check!(
                     CompileResult::from(decl_engine.get_function(decl_id.clone(), span)),
                     return err(warnings, errors),
                     warnings,
                     errors
                 );
-                let is_main = name.as_str() == sway_types::constants::DEFAULT_ENTRY_POINT_FN_NAME
-                    && matches!(
-                        tree_type,
-                        parsed::TreeType::Script | parsed::TreeType::Predicate
-                    );
-                ok(is_main, warnings, errors)
+                ok(!type_parameters.is_empty(), warnings, errors)
             }
             _ => ok(false, warnings, errors),
         }

diff --git a/sway-core/src/language/ty/program.rs b/sway-core/src/language/ty/program.rs
@@ -91,7 +91,7 @@ impl TyProgram {
                 TyAstNodeContent::Declaration(TyDeclaration::ConstantDeclaration(decl_id)) => {
                     match decl_engine.get_constant(decl_id.clone(), &node.span) {
                         Ok(config_decl) if config_decl.is_configurable => {
-                            configurables.push(config_decl);
+                            configurables.push(config_decl)
                         }
                         _ => {}
                     }
@@ -304,124 +304,115 @@ impl TyProgram {
         )
     }
 
-    /// Ensures there are no unresolved types or types awaiting resolution in the AST.
+    /// Collect various type information such as unresolved types and types of logged data
     pub(crate) fn collect_types_metadata(
         &mut self,
         ctx: &mut CollectTypesMetadataContext,
     ) -> CompileResult<Vec<TypeMetadata>> {
         let mut warnings = vec![];
         let mut errors = vec![];
         let decl_engine = ctx.decl_engine;
-        // Get all of the entry points for this tree type. For libraries, that's everything
-        // public. For contracts, ABI entries. For scripts and predicates, any function named `main`.
-        let metadata = match &self.kind {
-            TyProgramKind::Library { .. } => {
-                let mut ret = vec![];
-                for node in self.root.all_nodes.iter() {
-                    let public = check!(
-                        node.is_public(decl_engine),
-                        return err(warnings, errors),
-                        warnings,
-                        errors
-                    );
-                    let is_test = check!(
-                        node.is_test_function(decl_engine),
-                        return err(warnings, errors),
-                        warnings,
-                        errors
-                    );
-                    if public || is_test {
-                        ret.append(&mut check!(
-                            node.collect_types_metadata(ctx),
-                            return err(warnings, errors),
-                            warnings,
-                            errors
-                        ));
-                    }
-                }
-                ret
+        let mut metadata = vec![];
+
+        // First, look into all entry points that are not unit tests.
+        match &self.kind {
+            // For scripts and predicates, collect metadata for all the types starting with
+            // `main()` as the only entry point
+            TyProgramKind::Script { main_function, .. }
+            | TyProgramKind::Predicate { main_function, .. } => {
+                metadata.append(&mut check!(
+                    main_function.collect_types_metadata(ctx),
+                    return err(warnings, errors),
+                    warnings,
+                    errors
+                ));
             }
-            TyProgramKind::Script { .. } => {
-                let mut data = vec![];
-                for node in self.root.all_nodes.iter() {
-                    let is_main = check!(
-                        node.is_main_function(decl_engine, parsed::TreeType::Script),
-                        return err(warnings, errors),
-                        warnings,
-                        errors
-                    );
-                    let is_test = check!(
-                        node.is_test_function(decl_engine),
+            // For contracts, collect metadata for all the types starting with each ABI method as
+            // an entry point.
+            TyProgramKind::Contract { abi_entries, .. } => {
+                for entry in abi_entries.iter() {
+                    metadata.append(&mut check!(
+                        entry.collect_types_metadata(ctx),
                         return err(warnings, errors),
                         warnings,
                         errors
-                    );
-                    if is_main || is_test {
-                        data.append(&mut check!(
-                            node.collect_types_metadata(ctx),
-                            return err(warnings, errors),
-                            warnings,
-                            errors
-                        ));
-                    }
+                    ));
                 }
-                data
             }
-            TyProgramKind::Predicate { .. } => {
-                let mut data = vec![];
-                for node in self.root.all_nodes.iter() {
-                    let is_main = check!(
-                        node.is_main_function(decl_engine, parsed::TreeType::Predicate),
-                        return err(warnings, errors),
-                        warnings,
-                        errors
-                    );
-                    let is_test = check!(
-                        node.is_test_function(decl_engine),
-                        return err(warnings, errors),
-                        warnings,
-                        errors
-                    );
-                    if is_main || is_test {
-                        data.append(&mut check!(
-                            node.collect_types_metadata(ctx),
+            // For libraries, collect metadata for all the types starting with each `pub` node as
+            // an entry point. Also dig into all the submodules of a library because nodes in those
+            // submodules can also be entry points.
+            TyProgramKind::Library { .. } => {
+                for module in std::iter::once(&self.root).chain(
+                    self.root
+                        .submodules_recursive()
+                        .into_iter()
+                        .map(|(_, submod)| &submod.module),
+                ) {
+                    for node in module.all_nodes.iter() {
+                        let is_public = check!(
+                            node.is_public(decl_engine),
                             return err(warnings, errors),
                             warnings,
                             errors
-                        ));
-                    }
-                }
-                data
-            }
-            TyProgramKind::Contract { abi_entries, .. } => {
-                let mut data = vec![];
-                for node in self.root.all_nodes.iter() {
-                    let is_test = check!(
-                        node.is_test_function(decl_engine),
-                        return err(warnings, errors),
-                        warnings,
-                        errors
-                    );
-                    if is_test {
-                        data.append(&mut check!(
-                            node.collect_types_metadata(ctx),
+                        );
+                        let is_generic_function = check!(
+                            node.is_generic_function(decl_engine),
                             return err(warnings, errors),
                             warnings,
                             errors
-                        ));
+                        );
+                        if is_public {
+                            let node_metadata = check!(
+                                node.collect_types_metadata(ctx),
+                                return err(warnings, errors),
+                                warnings,
+                                errors
+                            );
+                            metadata.append(
+                                &mut node_metadata
+                                    .iter()
+                                    .filter(|m| {
+                                        // Generic functions are allowed to have unresolved types
+                                        // so filter those
+                                        !(is_generic_function
+                                            && matches!(m, TypeMetadata::UnresolvedType(..)))
+                                    })
+                                    .cloned()
+                                    .collect::<Vec<TypeMetadata>>(),
+                            );
+                        }
                     }
                 }
-                for entry in abi_entries.iter() {
-                    data.append(&mut check!(
-                        entry.collect_types_metadata(ctx),
+            }
+        }
+
+        // Now consider unit tests: all unit test are considered entry points regardless of the
+        // program type
+        for module in std::iter::once(&self.root).chain(
+            self.root
+                .submodules_recursive()
+                .into_iter()
+                .map(|(_, submod)| &submod.module),
+        ) {
+            for node in module.all_nodes.iter() {
+                let is_test_function = check!(
+                    node.is_test_function(decl_engine),
+                    return err(warnings, errors),
+                    warnings,
+                    errors
+                );
+                if is_test_function {
+                    metadata.append(&mut check!(
+                        node.collect_types_metadata(ctx),
                         return err(warnings, errors),
                         warnings,
                         errors
                     ));
                 }
-                data
             }
-        };
+        }
+
         if errors.is_empty() {
             ok(metadata, warnings, errors)
         } else {

diff --git a/sway-core/src/type_system/collect_types_metadata.rs b/sway-core/src/type_system/collect_types_metadata.rs
@@ -47,6 +47,7 @@ impl MessageId {
 }
 
 #[allow(clippy::enum_variant_names)]
+#[derive(Debug, Clone)]
 pub enum TypeMetadata {
     // UnresolvedType receives the Ident of the type and a call site span.
     UnresolvedType(Ident, Option<Span>),

diff --git a/...src/e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/Forc.lock b/...src/e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/Forc.lock
@@ -0,0 +1,13 @@
+[[package]]
+name = 'core'
+source = 'path+from-root-53DBC6786CED201E'
+
+[[package]]
+name = 'script_with_nested_libs'
+source = 'member'
+dependencies = ['std']
+
+[[package]]
+name = 'std'
+source = 'path+from-root-53DBC6786CED201E'
+dependencies = ['core']
diff --git a/...src/e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/Forc.toml b/...src/e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/Forc.toml
@@ -0,0 +1,8 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "script_with_nested_libs"
+
+[dependencies]
+std = { path = "../../../../../../../sway-lib-std" }
diff --git a/.../e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/src/inner.sw b/.../e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/src/inner.sw
@@ -0,0 +1,14 @@
+library inner;
+
+dep inner2;
+
+#[test]
+fn test_meaning_of_life_inner() {
+    let meaning = 6 * 7;
+    assert(meaning == 42);
+}
+
+#[test]
+fn log_test_inner() {
+    std::logging::log(1u16);
+}
diff --git a/...e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/src/inner2.sw b/...e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/src/inner2.sw
@@ -0,0 +1,12 @@
+library inner2;
+
+#[test]
+fn test_meaning_of_life_inner2() {
+    let meaning = 6 * 7;
+    assert(meaning == 42);
+}
+
+#[test]
+fn log_test_inner2() {
+    std::logging::log(1u8);
+}
diff --git a/...c/e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/src/main.sw b/...c/e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/src/main.sw
@@ -0,0 +1,23 @@
+contract;
+
+dep inner;
+
+abi MyContract {
+    fn foo();
+}
+
+impl MyContract for Contract {
+    fn foo() { }
+}
+
+
+#[test]
+fn test_meaning_of_life() {
+    let meaning = 6 * 7;
+    assert(meaning == 42);
+}
+
+#[test]
+fn log_test() {
+    std::logging::log(1u32);
+}
diff --git a/...src/e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/test.toml b/...src/e2e_vm_tests/test_programs/should_pass/unit_tests/contract_with_nested_libs/test.toml
@@ -0,0 +1 @@
+category = "unit_tests_pass"
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/unit_tests/nested_libs/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/unit_tests/nested_libs/Forc.lock
@@ -0,0 +1,13 @@
+[[package]]
+name = 'core'
+source = 'path+from-root-6E8361428B3F8FCA'
+
+[[package]]
+name = 'nested_libs'
+source = 'member'
+dependencies = ['std']
+
+[[package]]
+name = 'std'
+source = 'path+from-root-6E8361428B3F8FCA'
+dependencies = ['core']
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/unit_tests/nested_libs/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/unit_tests/nested_libs/Forc.toml
@@ -0,0 +1,8 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+entry = "lib.sw"
+license = "Apache-2.0"
+name = "nested_libs"
+
+[dependencies]
+std = { path = "../../../../../../../sway-lib-std" }
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/unit_tests/nested_libs/src/inner.sw b/test/src/e2e_vm_tests/test_programs/should_pass/unit_tests/nested_libs/src/inner.sw
@@ -0,0 +1,14 @@
+library inner;
+
+dep inner2;
+
+#[test]
+fn test_meaning_of_life_inner() {
+    let meaning = 6 * 7;
+    assert(meaning == 42);
+}
+
+#[test]
+fn log_test_inner() {
+    std::logging::log(1u16);
+}