From 5fc17500699f548ddcd90cda3cefe5b54b66265a Mon Sep 17 00:00:00 2001 From: Vaivaswatha N Date: Mon, 12 Dec 2022 16:14:26 +0530 Subject: [PATCH] Checks around `ref` and `mut` params of ABIs and Traits (#3573) Fixes #3570 --- .../ast_node/declaration/abi.rs | 4 +- .../ast_node/declaration/impl_trait.rs | 17 +++--- sway-error/src/error.rs | 11 ++-- .../should_fail/abi_ref_mut/src/main.sw | 58 ++++++++++++++++++- .../should_fail/abi_ref_mut/test.toml | 45 +++++++++++++- .../ref_mut_contract_abi/test.toml | 2 +- .../trait_method_signature_mismatch/test.toml | 2 +- 7 files changed, 117 insertions(+), 22 deletions(-) diff --git a/sway-core/src/semantic_analysis/ast_node/declaration/abi.rs b/sway-core/src/semantic_analysis/ast_node/declaration/abi.rs index 51030cf868a..f0bc12aa3ff 100644 --- a/sway-core/src/semantic_analysis/ast_node/declaration/abi.rs +++ b/sway-core/src/semantic_analysis/ast_node/declaration/abi.rs @@ -43,7 +43,7 @@ impl ty::TyAbiDeclaration { errors ); for param in &method.parameters { - if param.is_reference && param.is_mutable { + if param.is_reference || param.is_mutable { errors.push(CompileError::RefMutableNotAllowedInContractAbi { param_name: param.name.clone(), }) @@ -62,7 +62,7 @@ impl ty::TyAbiDeclaration { errors ); for param in &method.parameters { - if param.is_reference && param.is_mutable { + if param.is_reference || param.is_mutable { errors.push(CompileError::RefMutableNotAllowedInContractAbi { param_name: param.name.clone(), }) diff --git a/sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs b/sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs index 5d87cef2392..672848c22e4 100644 --- a/sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs +++ b/sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs @@ -700,17 +700,18 @@ fn type_check_trait_implementation( { // TODO use trait constraints as part of the type here to // implement trait constraint solver */ - // check if the mutability of the parameters is incompatible - if impl_method_param.is_mutable != impl_method_signature_param.is_mutable { - errors.push(CompileError::ParameterMutabilityMismatch { - span: impl_method_param.mutability_span.clone(), - }); + // Check if we have a non-ref mutable argument. That's not allowed. + if impl_method_signature_param.is_mutable && !impl_method_signature_param.is_reference { + errors.push(CompileError::MutableParameterNotSupported { + param_name: impl_method_signature.name.clone(), + }) } - if (impl_method_param.is_reference || impl_method_signature_param.is_reference) - && is_contract + // check if reference / mutability of the parameters is incompatible + if impl_method_param.is_mutable != impl_method_signature_param.is_mutable + || impl_method_param.is_reference != impl_method_signature_param.is_reference { - errors.push(CompileError::RefMutParameterInContract { + errors.push(CompileError::ParameterRefMutabilityMismatch { span: impl_method_param.mutability_span.clone(), }); } diff --git a/sway-error/src/error.rs b/sway-error/src/error.rs index 92b7e832549..7bd98715932 100644 --- a/sway-error/src/error.rs +++ b/sway-error/src/error.rs @@ -131,7 +131,7 @@ pub enum CompileError { 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.")] + #[error("ref mut or 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 \ @@ -583,11 +583,9 @@ pub enum CompileError { span: Span, }, #[error( - "Parameter mutability mismatch between the trait function declaration and its implementation." + "Parameter reference type or mutability mismatch between the trait function declaration and its implementation." )] - ParameterMutabilityMismatch { span: Span }, - #[error("Ref mutable parameter is not supported for contract ABI function.")] - RefMutParameterInContract { span: Span }, + ParameterRefMutabilityMismatch { span: Span }, #[error("Literal value is too large for type {ty}.")] IntegerTooLarge { span: Span, ty: String }, #[error("Literal value underflows type {ty}.")] @@ -841,8 +839,7 @@ impl Spanned for CompileError { DeclIsNotAConstant { span, .. } => span.clone(), ImpureInNonContract { span, .. } => span.clone(), ImpureInPureContext { span, .. } => span.clone(), - ParameterMutabilityMismatch { span, .. } => span.clone(), - RefMutParameterInContract { span, .. } => span.clone(), + ParameterRefMutabilityMismatch { span, .. } => span.clone(), IntegerTooLarge { span, .. } => span.clone(), IntegerTooSmall { span, .. } => span.clone(), IntegerContainsInvalidDigit { span, .. } => span.clone(), diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/abi_ref_mut/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_fail/abi_ref_mut/src/main.sw index 84ec42f6ea8..cb3507a6a11 100644 --- a/test/src/e2e_vm_tests/test_programs/should_fail/abi_ref_mut/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_fail/abi_ref_mut/src/main.sw @@ -1,11 +1,65 @@ contract; abi MyContract { - fn test_function(p: u64); + fn test_function1(p1: u64); + fn test_function2(mut p2: u64); + fn test_function3(ref mut p3: u64); + fn test_function4(ref p4: u64); + fn test_function5(p5: u64); + fn test_function6(p6: u64); } impl MyContract for Contract { - fn test_function(ref mut p: u64) { + fn test_function1(ref mut p1: u64) { + + } + fn test_function2(mut p2: u64) { + + } + fn test_function3(ref mut p3: u64) { + + } + fn test_function4(ref p4: u64) { + + } + fn test_function5(ref p5: u64) { + + } + fn test_function6(mut p6: u64) { + + } +} + +trait MyTrait { + fn check_function1(q1: u64); + fn check_function2(mut q2: u64); + fn check_function3(ref mut q3: u64); + fn check_function4(ref q4: u64); + fn check_function5(q5: u64); + fn check_function6(q6: u64); +} + +struct S { + +} + +impl MyTrait for S { + fn check_function1(ref mut q1: u64) { + + } + fn check_function2(mut q2: u64) { + + } + fn check_function3(ref mut q3: u64) { + + } + fn check_function4(ref q4: u64) { + + } + fn check_function5(ref q5: u64) { + + } + fn check_function6(mut q6: u64) { } } diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/abi_ref_mut/test.toml b/test/src/e2e_vm_tests/test_programs/should_fail/abi_ref_mut/test.toml index 2fec989b686..7dd28c9b472 100644 --- a/test/src/e2e_vm_tests/test_programs/should_fail/abi_ref_mut/test.toml +++ b/test/src/e2e_vm_tests/test_programs/should_fail/abi_ref_mut/test.toml @@ -1,3 +1,46 @@ category = "fail" -# check: $()Ref mutable parameter is not supported for contract ABI function. \ No newline at end of file +# ABI + +# check: $()error +# check: $()fn test_function2(mut p2: u64); +# nextln: $()ref mut or mut parameter is not allowed for contract ABI function + +# check: $()error +# check: $()fn test_function3(ref mut p3: u64); +# nextln: $()ref mut or mut parameter is not allowed for contract ABI function + +# check: $()error +# check: $()fn test_function4(ref p4: u64); +# nextln: $()ref mut or mut parameter is not allowed for contract ABI function + +# check: $()error +# check: $()fn test_function1(ref mut p1: u64) { +# nextln: $()Parameter reference type or mutability mismatch between the trait function declaration and its implementation + +# check: $()error +# check: $()fn test_function5(ref p5: u64) { +# nextln: $()Parameter reference type or mutability mismatch between the trait function declaration and its implementation + +# check: $()error +# check: $()fn test_function6(mut p6: u64) { +# nextln: $()Parameter reference type or mutability mismatch between the trait function declaration and its implementation + +#### Trait + +# check: $()error +# check: $()fn check_function1(ref mut q1: u64) { +# nextln: $()Parameter reference type or mutability mismatch between the trait function declaration and its implementation + +# check: $()error +# check: $()fn check_function2(mut q2: u64); +# nextln: $()This parameter was declared as mutable, which is not supported yet, did you mean to use ref mut? + +# check: $()error +# check: $()fn check_function5(ref q5: u64) { +# nextln: $()Parameter reference type or mutability mismatch between the trait function declaration and its implementation + +# check: $()error +# check: $()fn check_function6(mut q6: u64) { +# nextln: $()Parameter reference type or mutability mismatch between the trait function declaration and its implementation + diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_contract_abi/test.toml b/test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_contract_abi/test.toml index adcecd4182b..ebddfbbc18e 100644 --- a/test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_contract_abi/test.toml +++ b/test/src/e2e_vm_tests/test_programs/should_fail/ref_mut_contract_abi/test.toml @@ -1,4 +1,4 @@ category = "fail" # check: $()fn foo(ref mut x: u64); -# nextln: $()ref mut parameter is not allowed for contract ABI function. \ No newline at end of file +# nextln: $()ref mut or mut parameter is not allowed for contract ABI function. \ No newline at end of file diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/trait_method_signature_mismatch/test.toml b/test/src/e2e_vm_tests/test_programs/should_fail/trait_method_signature_mismatch/test.toml index 0065d286ee3..0ce3ccb442a 100644 --- a/test/src/e2e_vm_tests/test_programs/should_fail/trait_method_signature_mismatch/test.toml +++ b/test/src/e2e_vm_tests/test_programs/should_fail/trait_method_signature_mismatch/test.toml @@ -11,7 +11,7 @@ category = "fail" # nextln: $()The definition of this function must match the one in the trait "Foo" declaration. # check: fn bar(ref mut variable: u64) -> bool { -# check: $()Parameter mutability mismatch between the trait function declaration and its implementation. +# check: $()Parameter reference type or mutability mismatch between the trait function declaration and its implementation. # check: fn baz() -> u64 { # nextln: $()expected: u32