Split processing of impl methods in two phases (part 1). (#4890)

## Description

This PR mainly refactors existing code around type checking and
processing of functions.

[Split type checking and namespace insertion for parameters and type
parameters](327deace24)

[Split processing of functions in two
phases.](e3cf148f3f)

I've split this up from the rest of the PR for fixing impl methods
calling to make it easier to review (also made it easier for me to find
and fix some regressions).

I also threw in another round of clippy fixes that my Rust toolchain was
complaining about.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] 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.

forc-pkg/src/pkg.rs

@@ -1872,7 +1872,7 @@ pub fn compile(
                 metrics
             );
 
-            ops.extend(abi.into_iter());
+            ops.extend(abi);
 
             ProgramABI::Evm(ops)
         }

forc-plugins/forc-tx/src/lib.rs

@@ -397,7 +397,7 @@ pub enum ConvertInputError {
     WitnessPredicateMismatch,
 }
 
-const EXAMPLES: &str = r#"EXAMPLES:
+const EXAMPLES: &str = r"EXAMPLES:
     # An example constructing a `create` transaction.
     forc tx create \
         --bytecode ./my-contract/out/debug/my-contract.bin \
@@ -453,7 +453,7 @@ const EXAMPLES: &str = r#"EXAMPLES:
         output contract-created \
             --contract-id 0xCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC \
             --state-root 0x0000000000000000000000000000000000000000000000000000000000000000
-"#;
+";
 
 impl ParseError {
     /// Print the error with clap's fancy formatting.

sway-core/src/decl_engine/template.rs

@@ -7,7 +7,7 @@
 /// [SubstList](crate::type_system::SubstList) contained in this field is simply
 /// a template for usages of the declaration declared in that particular
 /// [TyDecl](crate::language::ty::TyDecl) node.
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Default)]
 pub struct Template<T>(T)
 where
     T: Clone;

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

@@ -7,7 +7,7 @@ use crate::{
     type_system::*, types::DeterministicallyAborts,
 };
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Default)]
 pub struct TyCodeBlock {
     pub contents: Vec<TyAstNode>,
 }

sway-core/src/lib.rs

@@ -738,14 +738,10 @@ fn module_dead_code_analysis<'eng: 'cfg, 'cfg>(
     tree_type: &parsed::TreeType,
     graph: &mut ControlFlowGraph<'cfg>,
 ) -> Result<(), ErrorEmitted> {
-    module
+    module.submodules.iter().try_fold((), |_, (_, submodule)| {
-        .submodules
+        let tree_type = parsed::TreeType::Library;
-        .iter()
+        module_dead_code_analysis(handler, engines, &submodule.module, &tree_type, graph)
-        .fold(Ok(()), |res, (_, submodule)| {
+    })?;
-            let tree_type = parsed::TreeType::Library;
-            res?;
-            module_dead_code_analysis(handler, engines, &submodule.module, &tree_type, graph)
-        })?;
     let res = {
         ControlFlowGraph::append_module_to_dead_code_graph(
             engines,

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

@@ -26,11 +26,15 @@ impl ty::TyEnumDecl {
         let mut decl_namespace = ctx.namespace.clone();
         let mut ctx = ctx.scoped(&mut decl_namespace);
 
-        // Type check the type parameters. This will also insert them into the
+        // Type check the type parameters.
-        // current namespace.
         let new_type_parameters =
             TypeParameter::type_check_type_params(handler, ctx.by_ref(), type_parameters)?;
 
+        // Insert them into the current namespace.
+        for p in &new_type_parameters {
+            p.insert_into_namespace(handler, ctx.by_ref())?;
+        }
+
         // type check the variants
         let mut variants_buf = vec![];
         for variant in variants {

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

@@ -8,7 +8,11 @@ use sway_error::{
 };
 
 use crate::{
-    language::{parsed::*, ty, Visibility},
+    language::{
+        parsed::*,
+        ty::{self, TyCodeBlock},
+        Visibility,
+    },
     semantic_analysis::*,
     type_system::*,
 };
@@ -21,10 +25,27 @@ impl ty::TyFunctionDecl {
         fn_decl: FunctionDeclaration,
         is_method: bool,
         is_in_impl_self: bool,
+    ) -> Result<Self, ErrorEmitted> {
+        let mut ty_fn_decl = Self::type_check_signature(
+            handler,
+            ctx.by_ref(),
+            fn_decl.clone(),
+            is_method,
+            is_in_impl_self,
+        )?;
+        Self::type_check_body(handler, ctx, fn_decl, &mut ty_fn_decl)
+    }
+
+    pub fn type_check_signature(
+        handler: &Handler,
+        mut ctx: TypeCheckContext,
+        fn_decl: FunctionDeclaration,
+        is_method: bool,
+        is_in_impl_self: bool,
     ) -> Result<Self, ErrorEmitted> {
         let FunctionDeclaration {
             name,
-            body,
+            body: _,
             parameters,
             span,
             attributes,
@@ -68,16 +89,25 @@ impl ty::TyFunctionDecl {
         let new_type_parameters =
             TypeParameter::type_check_type_params(handler, ctx.by_ref(), type_parameters)?;
 
+        // Insert them into the current namespace.
+        for p in &new_type_parameters {
+            p.insert_into_namespace(handler, ctx.by_ref())?;
+        }
+
         // type check the function parameters, which will also insert them into the namespace
         let mut new_parameters = vec![];
         handler.scope(|handler| {
             for parameter in parameters.into_iter() {
-                new_parameters.push(
+                new_parameters.push({
-                    match ty::TyFunctionParameter::type_check(handler, ctx.by_ref(), parameter) {
+                    let param =
-                        Ok(val) => val,
+                        match ty::TyFunctionParameter::type_check(handler, ctx.by_ref(), parameter)
-                        Err(_) => continue,
+                        {
-                    },
+                            Ok(val) => val,
-                );
+                            Err(_) => continue,
+                        };
+                    param.insert_into_namespace(handler, ctx.by_ref());
+                    param
+                });
             }
             Ok(())
         })?;
@@ -93,6 +123,72 @@ impl ty::TyFunctionDecl {
             )
             .unwrap_or_else(|err| type_engine.insert(engines, TypeInfo::ErrorRecovery(err)));
 
+        let (visibility, is_contract_call) = if is_method {
+            if is_in_impl_self {
+                (visibility, false)
+            } else {
+                (Visibility::Public, false)
+            }
+        } else {
+            (visibility, matches!(ctx.abi_mode(), AbiMode::ImplAbiFn(..)))
+        };
+
+        let function_decl = ty::TyFunctionDecl {
+            name,
+            body: TyCodeBlock::default(),
+            parameters: new_parameters,
+            implementing_type: None,
+            span,
+            attributes,
+            return_type,
+            type_parameters: new_type_parameters,
+            visibility,
+            is_contract_call,
+            purity,
+            where_clause,
+        };
+
+        Ok(function_decl)
+    }
+
+    pub fn type_check_body(
+        handler: &Handler,
+        mut ctx: TypeCheckContext,
+        fn_decl: FunctionDeclaration,
+        ty_fn_decl: &mut Self,
+    ) -> Result<Self, ErrorEmitted> {
+        let FunctionDeclaration { body, .. } = fn_decl;
+
+        let ty::TyFunctionDecl {
+            parameters,
+            purity,
+            return_type,
+            type_parameters,
+            ..
+        } = ty_fn_decl;
+
+        let type_engine = ctx.engines.te();
+        let engines = ctx.engines();
+
+        // create a namespace for the function
+        let mut fn_namespace = ctx.namespace.clone();
+        let mut ctx = ctx
+            .by_ref()
+            .scoped(&mut fn_namespace)
+            .with_purity(*purity)
+            .with_const_shadowing_mode(ConstShadowingMode::Sequential)
+            .disallow_functions();
+
+        // Insert the previously type checked type parameters into the current namespace.
+        for p in type_parameters {
+            p.insert_into_namespace(handler, ctx.by_ref())?;
+        }
+
+        // Insert the previously type checked function parameters into the namespace.
+        for p in parameters {
+            p.insert_into_namespace(handler, ctx.by_ref());
+        }
+
         // type check the function body
         //
         // If there are no implicit block returns, then we do not want to type check them, so we
@@ -100,7 +196,7 @@ impl ty::TyFunctionDecl {
         let (body, _implicit_block_return) = {
             let ctx = ctx
                 .by_ref()
-                .with_purity(purity)
+                .with_purity(*purity)
                 .with_help_text("Function body's return type does not match up with its return type annotation.")
                 .with_type_annotation(return_type.type_id);
             ty::TyCodeBlock::type_check(handler, ctx, body).unwrap_or_else(|err| {
@@ -125,16 +221,6 @@ impl ty::TyFunctionDecl {
             return_type.type_id,
         )?;
 
-        let (visibility, is_contract_call) = if is_method {
-            if is_in_impl_self {
-                (visibility, false)
-            } else {
-                (Visibility::Public, false)
-            }
-        } else {
-            (visibility, matches!(ctx.abi_mode(), AbiMode::ImplAbiFn(..)))
-        };
-
         return_type.type_id.check_type_parameter_bounds(
             handler,
             &ctx,
@@ -142,22 +228,8 @@ impl ty::TyFunctionDecl {
             vec![],
         )?;
 
-        let function_decl = ty::TyFunctionDecl {
+        ty_fn_decl.body = body;
-            name,
+        Ok(ty_fn_decl.clone())
-            body,
-            parameters: new_parameters,
-            implementing_type: None,
-            span,
-            attributes,
-            return_type,
-            type_parameters: new_type_parameters,
-            visibility,
-            is_contract_call,
-            purity,
-            where_clause,
-        };
-
-        Ok(function_decl)
     }
 }
 

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

@@ -62,8 +62,6 @@ impl ty::TyFunctionParameter {
             type_argument,
         };
 
-        insert_into_namespace(handler, ctx, &typed_parameter);
-
         Ok(typed_parameter)
     }
 
@@ -103,31 +101,27 @@ impl ty::TyFunctionParameter {
 
         Ok(typed_parameter)
     }
-}
 
-fn insert_into_namespace(
+    pub fn insert_into_namespace(&self, handler: &Handler, ctx: TypeCheckContext) {
-    handler: &Handler,
+        let const_shadowing_mode = ctx.const_shadowing_mode();
-    ctx: TypeCheckContext,
+        let _ = ctx.namespace.insert_symbol(
-    typed_parameter: &ty::TyFunctionParameter,
+            handler,
-) {
+            self.name.clone(),
-    let const_shadowing_mode = ctx.const_shadowing_mode();
+            ty::TyDecl::VariableDecl(Box::new(ty::TyVariableDecl {
-    let _ = ctx.namespace.insert_symbol(
+                name: self.name.clone(),
-        handler,
+                body: ty::TyExpression {
-        typed_parameter.name.clone(),
+                    expression: ty::TyExpressionVariant::FunctionParameter,
-        ty::TyDecl::VariableDecl(Box::new(ty::TyVariableDecl {
+                    return_type: self.type_argument.type_id,
-            name: typed_parameter.name.clone(),
+                    span: self.name.span(),
-            body: ty::TyExpression {
+                },
-                expression: ty::TyExpressionVariant::FunctionParameter,
+                mutability: ty::VariableMutability::new_from_ref_mut(
-                return_type: typed_parameter.type_argument.type_id,
+                    self.is_reference,
-                span: typed_parameter.name.span(),
+                    self.is_mutable,
-            },
+                ),
-            mutability: ty::VariableMutability::new_from_ref_mut(
+                return_type: self.type_argument.type_id,
-                typed_parameter.is_reference,
+                type_ascription: self.type_argument.clone(),
-                typed_parameter.is_mutable,
+            })),
-            ),
+            const_shadowing_mode,
-            return_type: typed_parameter.type_argument.type_id,
+        );
-            type_ascription: typed_parameter.type_argument.clone(),
+    }
-        })),
-        const_shadowing_mode,
-    );
 }

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

@@ -45,11 +45,15 @@ impl ty::TyImplTrait {
             .with_const_shadowing_mode(ConstShadowingMode::ItemStyle)
             .allow_functions();
 
-        // Type check the type parameters. This will also insert them into the
+        // Type check the type parameters.
-        // current namespace.
         let new_impl_type_parameters =
             TypeParameter::type_check_type_params(handler, ctx.by_ref(), impl_type_parameters)?;
 
+        // Insert them into the current namespace.
+        for p in &new_impl_type_parameters {
+            p.insert_into_namespace(handler, ctx.by_ref())?;
+        }
+
         // resolve the types of the trait type arguments
         for type_arg in trait_type_arguments.iter_mut() {
             type_arg.type_id =
@@ -240,11 +244,15 @@ impl ty::TyImplTrait {
             is_absolute: false,
         };
 
-        // Type check the type parameters. This will also insert them into the
+        // Type check the type parameters.
-        // current namespace.
         let new_impl_type_parameters =
             TypeParameter::type_check_type_params(handler, ctx.by_ref(), impl_type_parameters)?;
 
+        // Insert them into the current namespace.
+        for p in &new_impl_type_parameters {
+            p.insert_into_namespace(handler, ctx.by_ref())?;
+        }
+
         // type check the type that we are implementing for
         implementing_for.type_id = ctx.resolve_type_without_self(
             handler,

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

@@ -26,11 +26,15 @@ impl ty::TyStructDecl {
         let mut decl_namespace = ctx.namespace.clone();
         let mut ctx = ctx.scoped(&mut decl_namespace);
 
-        // Type check the type parameters. This will also insert them into the
+        // Type check the type parameters.
-        // current namespace.
         let new_type_parameters =
             TypeParameter::type_check_type_params(handler, ctx.by_ref(), type_parameters)?;
 
+        // Insert them into the current namespace.
+        for p in &new_type_parameters {
+            p.insert_into_namespace(handler, ctx.by_ref())?;
+        }
+
         // type check the fields
         let mut new_fields = vec![];
         for field in fields.into_iter() {

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

@@ -54,11 +54,15 @@ impl ty::TyTraitDecl {
         let mut trait_namespace = ctx.namespace.clone();
         let mut ctx = ctx.scoped(&mut trait_namespace).with_self_type(self_type);
 
-        // Type check the type parameters. This will also insert them into the
+        // Type check the type parameters.
-        // current namespace.
         let new_type_parameters =
             TypeParameter::type_check_type_params(handler, ctx.by_ref(), type_parameters)?;
 
+        // Insert them into the current namespace.
+        for p in &new_type_parameters {
+            p.insert_into_namespace(handler, ctx.by_ref())?;
+        }
+
         // Recursively make the interface surfaces and methods of the
         // supertraits available to this trait.
         insert_supertraits_into_namespace(

sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs

@@ -2361,13 +2361,13 @@ fn statement_to_ast_nodes(
         }
         Statement::Item(item) => {
             let nodes = item_to_ast_nodes(context, handler, engines, item, false, None)?;
-            nodes.iter().fold(Ok(()), |res, node| {
+            nodes.iter().try_fold((), |res, node| {
                 if ast_node_is_test_fn(node) {
                     let span = node.span.clone();
                     let error = ConvertParseTreeError::TestFnOnlyAllowedAtModuleLevel { span };
                     Err(handler.emit_err(error.into()))
                 } else {
-                    res
+                    Ok(res)
                 }
             })?;
             nodes

sway-core/src/type_system/ast_elements/type_parameter.rs

@@ -188,16 +188,6 @@ impl TypeParameter {
             },
         );
 
-        // Insert the trait constraints into the namespace.
-        for trait_constraint in trait_constraints.iter() {
-            TraitConstraint::insert_into_namespace(
-                handler,
-                ctx.by_ref(),
-                type_id,
-                trait_constraint,
-            )?;
-        }
-
         // When type parameter is from parent then it was already inserted.
         // Instead of inserting a type with same name we unify them.
         if is_from_parent {
@@ -225,16 +215,6 @@ impl TypeParameter {
                     }
                 }
             }
-        } else {
-            // Insert the type parameter into the namespace as a dummy type
-            // declaration.
-            let type_parameter_decl =
-                ty::TyDecl::GenericTypeForFunctionScope(ty::GenericTypeForFunctionScope {
-                    name: name_ident.clone(),
-                    type_id,
-                });
-            ctx.insert_symbol(handler, name_ident.clone(), type_parameter_decl)
-                .ok();
         }
 
         let type_parameter = TypeParameter {
@@ -248,6 +228,43 @@ impl TypeParameter {
         Ok(type_parameter)
     }
 
+    pub fn insert_into_namespace(
+        &self,
+        handler: &Handler,
+        mut ctx: TypeCheckContext,
+    ) -> Result<(), ErrorEmitted> {
+        let Self {
+            is_from_parent,
+            name_ident,
+            type_id,
+            ..
+        } = self;
+
+        // Insert the trait constraints into the namespace.
+        for trait_constraint in self.trait_constraints.iter() {
+            TraitConstraint::insert_into_namespace(
+                handler,
+                ctx.by_ref(),
+                *type_id,
+                trait_constraint,
+            )?;
+        }
+
+        if !is_from_parent {
+            // Insert the type parameter into the namespace as a dummy type
+            // declaration.
+            let type_parameter_decl =
+                ty::TyDecl::GenericTypeForFunctionScope(ty::GenericTypeForFunctionScope {
+                    name: name_ident.clone(),
+                    type_id: *type_id,
+                });
+            ctx.insert_symbol(handler, name_ident.clone(), type_parameter_decl)
+                .ok();
+        }
+
+        Ok(())
+    }
+
     /// Creates a [DeclMapping] from a list of [TypeParameter]s.
     pub(crate) fn gather_decl_mapping_from_trait_constraints(
         handler: &Handler,

sway-ir/src/optimize/misc_demotion.rs

@@ -367,9 +367,10 @@ fn wide_binary_op_demotion(context: &mut Context, function: Function) -> Result<
         let Instruction::BinaryOp { op, arg1, arg2 } = binary_op_instr_val
             .get_instruction(context)
             .cloned()
-            .unwrap() else {
+            .unwrap()
-                continue;
+        else {
-            };
+            continue;
+        };
 
         let binary_op_metadata = binary_op_instr_val.get_metadata(context);
 

test/src/e2e_vm_tests/harness.rs

@@ -30,13 +30,11 @@ where
     let mut output = String::new();
 
     // Capture both stdout and stderr to buffers, run the code and save to a string.
-    let buf_stdout = Some(gag::BufferRedirect::stdout().unwrap());
+    let mut buf_stdout = gag::BufferRedirect::stdout().unwrap();
-    let buf_stderr = Some(gag::BufferRedirect::stderr().unwrap());
+    let mut buf_stderr = gag::BufferRedirect::stderr().unwrap();
 
     let result = func().await;
 
-    let mut buf_stdout = buf_stdout.unwrap();
-    let mut buf_stderr = buf_stderr.unwrap();
     buf_stdout.read_to_string(&mut output).unwrap();
     buf_stderr.read_to_string(&mut output).unwrap();
     drop(buf_stdout);