Fix some issues with ref mut parameters (FuelLabs#2563)

* Fix passing a pointer parameter to another function expecting a non-pointer.

Fixes Issue 1 from FuelLabs#2543.

* Add mutability check during function application sema checking.

Fixes Issue 2, 3 and 4 from
FuelLabs#2543.

* Check for ref mut parameters during contract ABI sema checking.

Fixes Issue 5 from FuelLabs#2543.

sway-core/src/error.rs

@@ -546,6 +546,10 @@ pub enum CompileError {
         "This parameter was declared as mutable, which is not supported yet, did you mean to use ref mut?"
     )]
     MutableParameterNotSupported { param_name: Ident },
+    #[error("Cannot pass immutable argument to mutable parameter.")]
+    ImmutableArgumentToMutableParameter { span: Span },
+    #[error("ref mut parameter is not allowed for contract ABI function.")]
+    RefMutableNotAllowedInContractAbi { param_name: Ident },
     #[error(
         "Cannot call associated function \"{fn_name}\" as a method. Use associated function \
         syntax instead."
@@ -1091,6 +1095,8 @@ impl Spanned for CompileError {
             ReassignmentToNonVariable { span, .. } => span.clone(),
             AssignmentToNonMutable { name } => name.span(),
             MutableParameterNotSupported { param_name } => param_name.span(),
+            ImmutableArgumentToMutableParameter { span } => span.clone(),
+            RefMutableNotAllowedInContractAbi { param_name } => param_name.span(),
             MethodRequiresMutableSelf { span, .. } => span.clone(),
             AssociatedFunctionCalledAsMethod { span, .. } => span.clone(),
             TypeParameterNotInTypeScope { span, .. } => span.clone(),

sway-core/src/ir_generation/function.rs

@@ -1187,7 +1187,16 @@ impl FnCompiler {
                     .add_metadatum(context, span_md_idx)
             })
         } else if let Some(val) = self.function.get_arg(context, name) {
-            Ok(val)
+            let is_ptr = val.get_type(context).filter(|f| f.is_ptr_type()).is_some();
+            if is_ptr {
+                Ok(self
+                    .current_block
+                    .ins(context)
+                    .load(val)
+                    .add_metadatum(context, span_md_idx))
+            } else {
+                Ok(val)
+            }
         } else if let Some(const_val) = self.module.get_global_constant(context, name) {
             Ok(const_val)
         } else {

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

@@ -8,7 +8,7 @@ use crate::{
         TypeCheckContext,
     },
     type_system::{insert_type, AbiName, TypeId},
-    AbiDeclaration, CompileResult, FunctionDeclaration, TypeInfo,
+    AbiDeclaration, CompileError, CompileResult, FunctionDeclaration, TypeInfo,
 };
 
 use super::{CreateTypeId, TypedTraitFn};
@@ -67,6 +67,16 @@ impl TypedAbiDeclaration {
             warnings,
             errors
         );
+        for typed_fn in &interface_surface {
+            for param in &typed_fn.parameters {
+                if param.is_reference && param.is_mutable {
+                    errors.push(CompileError::RefMutableNotAllowedInContractAbi {
+                        param_name: param.name.clone(),
+                    })
+                }
+            }
+        }
+
         // type check these for errors but don't actually use them yet -- the real
         // ones will be type checked with proper symbols when the ABI is implemented
         let _methods = check!(
@@ -75,6 +85,16 @@ impl TypedAbiDeclaration {
             warnings,
             errors
         );
+        for typed_fn in &methods {
+            for param in &typed_fn.parameters {
+                if param.is_reference && param.is_mutable {
+                    errors.push(CompileError::RefMutableNotAllowedInContractAbi {
+                        param_name: param.name.clone(),
+                    })
+                }
+            }
+        }
+
         let abi_decl = TypedAbiDeclaration {
             interface_surface,
             methods,

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

@@ -411,6 +411,14 @@ impl TypedExpression {
         }
     }
 
+    /// gathers the mutability of the expressions within
+    pub(crate) fn gather_mutability(&self) -> VariableMutability {
+        match &self.expression {
+            TypedExpressionVariant::VariableExpression { mutability, .. } => *mutability,
+            _ => VariableMutability::Immutable,
+        }
+    }
+
     pub(crate) fn type_check(mut ctx: TypeCheckContext, expr: Expression) -> CompileResult<Self> {
         let expr_span = expr.span();
         let span = expr_span.clone();
@@ -615,13 +623,15 @@ impl TypedExpression {
             Some(TypedDeclaration::VariableDeclaration(TypedVariableDeclaration {
                 name: decl_name,
                 body,
+                mutability,
                 ..
             })) => TypedExpression {
                 return_type: body.return_type,
                 is_constant: body.is_constant,
                 expression: TypedExpressionVariant::VariableExpression {
                     name: decl_name.clone(),
                     span: name.span(),
+                    mutability: *mutability,
                 },
                 span,
             },
@@ -637,6 +647,7 @@ impl TypedExpression {
                 expression: TypedExpressionVariant::VariableExpression {
                     name: decl_name.clone(),
                     span: name.span(),
+                    mutability: VariableMutability::Immutable,
                 },
                 span,
             },

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

@@ -52,6 +52,14 @@ pub(crate) fn instantiate_function_application(
                 warnings,
                 errors
             );
+
+            // check for matching mutability
+            let param_mutability =
+                convert_to_variable_immutability(param.is_reference, param.is_mutable);
+            if exp.gather_mutability().is_immutable() && param_mutability.is_mutable() {
+                errors.push(CompileError::ImmutableArgumentToMutableParameter { span: arg.span() });
+            }
+
             (param.name.clone(), exp)
         })
         .collect();

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

@@ -36,6 +36,7 @@ pub enum TypedExpressionVariant {
     VariableExpression {
         name: Ident,
         span: Span,
+        mutability: VariableMutability,
     },
     Tuple {
         fields: Vec<TypedExpression>,
@@ -160,12 +161,14 @@ impl PartialEq for TypedExpressionVariant {
                 Self::VariableExpression {
                     name: l_name,
                     span: l_span,
+                    mutability: l_mutability,
                 },
                 Self::VariableExpression {
                     name: r_name,
                     span: r_span,
+                    mutability: r_mutability,
                 },
-            ) => l_name == r_name && l_span == r_span,
+            ) => l_name == r_name && l_span == r_span && l_mutability == r_mutability,
             (Self::Tuple { fields: l_fields }, Self::Tuple { fields: r_fields }) => {
                 l_fields == r_fields
             }

sway-ir/src/error.rs

@@ -43,7 +43,6 @@ pub enum IrError {
     VerifyIntToPtrToCopyType(String),
     VerifyIntToPtrUnknownSourceType,
     VerifyLoadFromNonPointer,
-    VerifyLoadNonExistentPointer,
     VerifyMismatchedReturnTypes(String),
     VerifyPhiFromMissingBlock(String),
     VerifyPhiInconsistentTypes,
@@ -239,10 +238,6 @@ impl fmt::Display for IrError {
             IrError::VerifyLoadFromNonPointer => {
                 write!(f, "Verification failed: Load must be from a pointer.")
             }
-            IrError::VerifyLoadNonExistentPointer => write!(
-                f,
-                "Verification failed: Attempt to load from a pointer not found in function locals."
-            ),
             IrError::VerifyMismatchedReturnTypes(fn_str) => write!(
                 f,
                 "Verification failed: Function {fn_str} return type must match its RET \

sway-ir/src/instruction.rs

@@ -184,13 +184,13 @@ impl Instruction {
             Instruction::Gtf { .. } => Some(Type::Uint(64)),
             Instruction::InsertElement { array, .. } => array.get_type(context),
             Instruction::InsertValue { aggregate, .. } => aggregate.get_type(context),
-            Instruction::Load(ptr_val) => {
-                if let ValueDatum::Instruction(ins) = &context.values[ptr_val.0].value {
+            Instruction::Load(ptr_val) => match &context.values[ptr_val.0].value {
+                ValueDatum::Argument(ty) => Some(ty.strip_ptr_type(context)),
+                ValueDatum::Constant(cons) => Some(cons.ty.strip_ptr_type(context)),
+                ValueDatum::Instruction(ins) => {
                     ins.get_type(context).map(|f| f.strip_ptr_type(context))
-                } else {
-                    None
                 }
-            }
+            },
             Instruction::ReadRegister(_) => Some(Type::Uint(64)),
             Instruction::StateLoadWord(_) => Some(Type::Uint(64)),
             Instruction::Phi(alts) => {

sway-ir/src/verify.rs

@@ -562,8 +562,6 @@ impl<'a> InstructionVerifier<'a> {
             } else {
                 Ok(())
             }
-        } else if !self.is_local_pointer(src_ptr.as_ref().unwrap()) {
-            Err(IrError::VerifyLoadNonExistentPointer)
         } else {
             Ok(())
         }

sway-lsp/src/core/traverse_typed_tree.rs

@@ -278,7 +278,9 @@ fn handle_expression(expression: &TypedExpression, tokens: &TokenMap) {
             handle_expression(lhs, tokens);
             handle_expression(rhs, tokens);
         }
-        TypedExpressionVariant::VariableExpression { ref name, ref span } => {
+        TypedExpressionVariant::VariableExpression {
+            ref name, ref span, ..
+        } => {
             if let Some(mut token) = tokens.get_mut(&to_ident_key(&Ident::new(span.clone()))) {
                 token.typed = Some(TypedAstToken::TypedExpression(expression.clone()));
                 token.type_def = Some(TypeDefinition::Ident(name.clone()));

test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_contract_abi/Forc.lock

@@ -0,0 +1,4 @@
+[[package]]
+name = 'ref_mut_contract_abi'
+source = 'root'
+dependencies = []

test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_contract_abi/Forc.toml

@@ -0,0 +1,6 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+license = "Apache-2.0"
+name = "ref_mut_contract_abi"
+entry = "main.sw"
+implicit-std = false

test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_contract_abi/json_abi_oracle.json

@@ -0,0 +1,15 @@
+[
+  {
+    "inputs": [],
+    "name": "main",
+    "outputs": [
+      {
+        "components": null,
+        "name": "",
+        "type": "u32",
+        "typeArguments": null
+      }
+    ],
+    "type": "function"
+  }
+]

test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_contract_abi/src/main.sw

@@ -0,0 +1,11 @@
+contract;
+
+abi MyAbi {
+    fn foo(ref mut x: u64);
+}
+
+impl MyAbi for Contract {
+    fn foo(ref mut x: u64) {
+
+    }
+}

test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_contract_abi/test.toml

@@ -0,0 +1,4 @@
+category = "fail"
+
+# check: $()fn foo(ref mut x: u64);
+# nextln: $()ref mut parameter is not allowed for contract ABI function.

test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_mismatch/Forc.lock

@@ -0,0 +1,4 @@
+[[package]]
+name = 'ref_mut_mismatch'
+source = 'root'
+dependencies = []

test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_mismatch/Forc.toml

@@ -0,0 +1,6 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+license = "Apache-2.0"
+name = "ref_mut_mismatch"
+entry = "main.sw"
+implicit-std = false

test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_mismatch/json_abi_oracle.json

@@ -0,0 +1,15 @@
+[
+  {
+    "inputs": [],
+    "name": "main",
+    "outputs": [
+      {
+        "components": null,
+        "name": "",
+        "type": "u32",
+        "typeArguments": null
+      }
+    ],
+    "type": "function"
+  }
+]

test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_mismatch/src/main.sw

@@ -0,0 +1,12 @@
+script;
+
+fn foo(ref mut y: u64) {
+    y = 1;
+}
+
+fn main() {
+    let x = 1;
+    foo(x);
+
+    foo(0);
+}

test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_mismatch/test.toml

@@ -0,0 +1,8 @@
+category = "fail"
+
+# check: $()foo(x);
+# nextln: $()Cannot pass immutable argument to mutable parameter.
+# check: $()foo(0);
+
+# check: $()foo(0);
+# nextln: $()Cannot pass immutable argument to mutable parameter.

test/src/e2e_vm_tests/test_programs/should_pass/language/ref_mutable_fn_args_call/Forc.lock

@@ -0,0 +1,9 @@
+[[package]]
+name = 'core'
+source = 'path+from-root-1823A1D461605CE8'
+dependencies = []
+
+[[package]]
+name = 'ref_mutable_fn_args_struct_assign'
+source = 'root'
+dependencies = ['core']

test/src/e2e_vm_tests/test_programs/should_pass/language/ref_mutable_fn_args_call/Forc.toml

@@ -0,0 +1,9 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+license = "Apache-2.0"
+name = "ref_mutable_fn_args_struct_assign"
+entry = "main.sw"
+implicit-std = false
+
+[dependencies]
+core = { path = "../../../../../../../sway-lib-core" }

test/src/e2e_vm_tests/test_programs/should_pass/language/ref_mutable_fn_args_call/json_abi_oracle.json

@@ -0,0 +1,15 @@
+[
+  {
+    "inputs": [],
+    "name": "main",
+    "outputs": [
+      {
+        "components": null,
+        "name": "",
+        "type": "u64",
+        "typeArguments": null
+      }
+    ],
+    "type": "function"
+  }
+]

test/src/e2e_vm_tests/test_programs/should_pass/language/ref_mutable_fn_args_call/src/main.sw

@@ -0,0 +1,16 @@
+script;
+
+// x += 1 below passes x which is a pointer in the IR
+// directly to core::ops::add, so this tests that we can pass
+// the pointer parameter to a call that doesn't expect a pointer,
+// requiring us to issue an IR load first.
+
+fn foo(ref mut x: u64) {
+    x += 1;
+}
+
+fn main() -> u64 {
+    let mut x = 1;
+    foo(x);
+    x
+}

test/src/e2e_vm_tests/test_programs/should_pass/language/ref_mutable_fn_args_call/test.toml

@@ -0,0 +1,3 @@
+category = "run"
+expected_result = { action = "return", value = 2 }
+validate_abi = true