Implement __contract_ret as diverging and retd as terminator (#6486)

## Description This PR implements the `__contract_ret` as a diverging (returning `!`) and accordingly the FuelVM `retd` instruction as being a terminator. This strict semantics is needed in the #6351 which introduces strict(er) verification of IR invariants. ## 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). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [ ] 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.
FuelLabs · Sep 4, 2024 · e220eca · e220eca
1 parent a81c42e
commit e220eca
Show file tree

Hide file tree

Showing 28 changed files with 187 additions and 58 deletions.
diff --git a/forc-plugins/forc-client/test/data/standalone_contract/Forc.lock b/forc-plugins/forc-client/test/data/standalone_contract/Forc.lock
@@ -0,0 +1,13 @@
+[[package]]
+name = "core"
+source = "path+from-root-79BB3EA8498403DE"
+
+[[package]]
+name = "standalone_contract"
+source = "member"
+dependencies = ["std"]
+
+[[package]]
+name = "std"
+source = "path+from-root-79BB3EA8498403DE"
+dependencies = ["core"]
diff --git a/forc-plugins/forc-client/tests/deploy.rs b/forc-plugins/forc-client/tests/deploy.rs
@@ -341,7 +341,7 @@ async fn test_simple_deploy() {
     node.kill().unwrap();
     let expected = vec![DeployedContract {
         id: ContractId::from_str(
-            "ad0bba17e0838ef859abe2693d8a5e3bc4e7cfb901601e30f4dc34999fda6335",
+            "50fe882cbef5f3da6da82509a66b7e5e0a64a40d70164861c01c908a332198ae",
         )
         .unwrap(),
         proxy: None,
@@ -383,7 +383,7 @@ async fn test_deploy_submit_only() {
     node.kill().unwrap();
     let expected = vec![DeployedContract {
         id: ContractId::from_str(
-            "ad0bba17e0838ef859abe2693d8a5e3bc4e7cfb901601e30f4dc34999fda6335",
+            "50fe882cbef5f3da6da82509a66b7e5e0a64a40d70164861c01c908a332198ae",
         )
         .unwrap(),
         proxy: None,
@@ -428,12 +428,12 @@ async fn test_deploy_fresh_proxy() {
     node.kill().unwrap();
     let impl_contract = DeployedContract {
         id: ContractId::from_str(
-            "ad0bba17e0838ef859abe2693d8a5e3bc4e7cfb901601e30f4dc34999fda6335",
+            "50fe882cbef5f3da6da82509a66b7e5e0a64a40d70164861c01c908a332198ae",
         )
         .unwrap(),
         proxy: Some(
             ContractId::from_str(
-                "f2d67efbd6038c85ddaffdcdc859770d8bd20eeec8e3909911f1446b2ec9f764",
+                "9c50c6837ba29508ad1b0fb01953892031218b5a08be73925ca5c0148e00a186",
             )
             .unwrap(),
         ),

diff --git a/sway-core/src/ir_generation/compile.rs b/sway-core/src/ir_generation/compile.rs
@@ -635,7 +635,7 @@ fn compile_fn(
     // recent instruction was a RET.
     let already_returns = compiler
         .current_block
-        .is_terminated_by_ret_or_revert(context);
+        .is_terminated_by_return_or_revert(context);
     if !already_returns
         && (compiler.current_block.num_instructions(context) > 0
             || compiler.current_block == compiler.function.get_entry_block(context)

diff --git a/sway-core/src/semantic_analysis/ast_node/expression/intrinsic_function.rs b/sway-core/src/semantic_analysis/ast_node/expression/intrinsic_function.rs
@@ -1855,14 +1855,7 @@ fn type_check_smo(
     );
     let data = ty::TyExpression::type_check(handler, ctx.by_ref(), &arguments[1])?;
 
-    // Type check the third argument which is the output index, so it has to be a `u64`.
-    let mut ctx = ctx.by_ref().with_type_annotation(type_engine.insert(
-        engines,
-        TypeInfo::UnsignedInteger(IntegerBits::SixtyFour),
-        None,
-    ));
-
-    // Type check the fourth argument which is the amount of coins to send, so it has to be a `u64`.
+    // Type check the third argument which is the amount of coins to send, so it has to be a `u64`.
     let mut ctx = ctx.by_ref().with_type_annotation(type_engine.insert(
         engines,
         TypeInfo::UnsignedInteger(IntegerBits::SixtyFour),
@@ -1881,20 +1874,37 @@ fn type_check_smo(
     ))
 }
 
-/// Signature: `__contract_call<T>()`
-/// Description: Calls another contract
+/// Signature: `__contract_ret(ptr: raw_ptr, len: u64) -> !`
+/// Description: Returns from contract. The returned data is located at the memory location `ptr` and has
+/// the length of `len` bytes.
 /// Constraints: None.
 fn type_check_contract_ret(
     handler: &Handler,
     mut ctx: TypeCheckContext,
-    _kind: sway_ast::Intrinsic,
+    kind: sway_ast::Intrinsic,
     arguments: &[Expression],
-    _type_arguments: &[TypeArgument],
-    _span: Span,
+    type_arguments: &[TypeArgument],
+    span: Span,
 ) -> Result<(ty::TyIntrinsicFunctionKind, TypeId), ErrorEmitted> {
     let type_engine = ctx.engines.te();
     let engines = ctx.engines();
 
+    if arguments.len() != 2 {
+        return Err(handler.emit_err(CompileError::IntrinsicIncorrectNumArgs {
+            name: kind.to_string(),
+            expected: 2,
+            span,
+        }));
+    }
+
+    if !type_arguments.is_empty() {
+        return Err(handler.emit_err(CompileError::IntrinsicIncorrectNumTArgs {
+            name: kind.to_string(),
+            expected: 0,
+            span,
+        }));
+    }
+
     let arguments: Vec<ty::TyExpression> = arguments
         .iter()
         .map(|x| {
@@ -1906,17 +1916,14 @@ fn type_check_contract_ret(
         })
         .collect::<Result<Vec<_>, _>>()?;
 
-    let t = ctx
-        .engines
-        .te()
-        .insert(ctx.engines, TypeInfo::Tuple(vec![]), None);
+    let t = ctx.engines.te().insert(ctx.engines, TypeInfo::Never, None);
 
     Ok((
         ty::TyIntrinsicFunctionKind {
             kind: Intrinsic::ContractRet,
             arguments,
             type_arguments: vec![],
-            span: Span::dummy(),
+            span,
         },
         t,
     ))

diff --git a/sway-ir/src/block.rs b/sway-ir/src/block.rs
@@ -369,14 +369,21 @@ impl Block {
         }
     }
 
-    /// Return whether this block is already terminated specifically by a Ret instruction.
-    pub fn is_terminated_by_ret_or_revert(&self, context: &Context) -> bool {
+    /// Return whether this block is already terminated by non-branching instructions,
+    /// means with instructions that cause either revert, or local or context returns.
+    /// Those instructions are: [InstOp::Ret], [FuelVmInstruction::Retd],
+    /// [FuelVmInstruction::JmpMem], and [FuelVmInstruction::Revert]).
+    pub fn is_terminated_by_return_or_revert(&self, context: &Context) -> bool {
         self.get_terminator(context).map_or(false, |i| {
             matches!(
                 i,
                 Instruction {
                     op: InstOp::Ret(..)
-                        | InstOp::FuelVm(FuelVmInstruction::Revert(..) | FuelVmInstruction::JmpMem),
+                        | InstOp::FuelVm(
+                            FuelVmInstruction::Revert(..)
+                                | FuelVmInstruction::JmpMem
+                                | FuelVmInstruction::Retd { .. }
+                        ),
                     ..
                 }
             )

diff --git a/sway-ir/src/instruction.rs b/sway-ir/src/instruction.rs
@@ -284,7 +284,6 @@ impl InstOp {
             InstOp::FuelVm(FuelVmInstruction::Log { .. }) => Some(Type::get_unit(context)),
             InstOp::FuelVm(FuelVmInstruction::ReadRegister(_)) => Some(Type::get_uint64(context)),
             InstOp::FuelVm(FuelVmInstruction::Smo { .. }) => Some(Type::get_unit(context)),
-            InstOp::FuelVm(FuelVmInstruction::Retd { .. }) => None,
 
             // Load needs to strip the pointer from the source type.
             InstOp::Load(ptr_val) => match &context.values[ptr_val.0].value {
@@ -310,7 +309,11 @@ impl InstOp {
             // These are all terminators which don't return, essentially.  No type.
             InstOp::Branch(_)
             | InstOp::ConditionalBranch { .. }
-            | InstOp::FuelVm(FuelVmInstruction::Revert(..) | FuelVmInstruction::JmpMem)
+            | InstOp::FuelVm(
+                FuelVmInstruction::Revert(..)
+                | FuelVmInstruction::JmpMem
+                | FuelVmInstruction::Retd { .. },
+            )
             | InstOp::Ret(..) => None,
 
             // No-op is also no-type.
@@ -692,7 +695,11 @@ impl InstOp {
             InstOp::Branch(_)
                 | InstOp::ConditionalBranch { .. }
                 | InstOp::Ret(..)
-                | InstOp::FuelVm(FuelVmInstruction::Revert(..) | FuelVmInstruction::JmpMem)
+                | InstOp::FuelVm(
+                    FuelVmInstruction::Revert(..)
+                        | FuelVmInstruction::JmpMem
+                        | FuelVmInstruction::Retd { .. }
+                )
         )
     }
 }

diff --git a/sway-ir/src/value.rs b/sway-ir/src/value.rs
@@ -12,7 +12,7 @@ use crate::{
     block::BlockArgument,
     constant::Constant,
     context::Context,
-    instruction::{FuelVmInstruction, InstOp},
+    instruction::InstOp,
     irtype::Type,
     metadata::{combine, MetadataIndex},
     pretty::DebugWithContext,
@@ -102,13 +102,7 @@ impl Value {
     /// and is either a branch or return.
     pub fn is_terminator(&self, context: &Context) -> bool {
         match &context.values[self.0].value {
-            ValueDatum::Instruction(Instruction { op, .. }) => matches!(
-                op,
-                InstOp::Branch(_)
-                    | InstOp::ConditionalBranch { .. }
-                    | InstOp::Ret(_, _)
-                    | InstOp::FuelVm(FuelVmInstruction::Revert(_) | FuelVmInstruction::JmpMem)
-            ),
+            ValueDatum::Instruction(Instruction { op, .. }) => op.is_terminator(),
             ValueDatum::Argument(..) | ValueDatum::Constant(..) => false,
         }
     }

diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/contract_ret_intrinsic/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/language/contract_ret_intrinsic/Forc.lock
@@ -0,0 +1,13 @@
+[[package]]
+name = "contract_ret_intrinsic"
+source = "member"
+dependencies = ["std"]
+
+[[package]]
+name = "core"
+source = "path+from-root-793618C6A3B48D0B"
+
+[[package]]
+name = "std"
+source = "path+from-root-793618C6A3B48D0B"
+dependencies = ["core"]
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/contract_ret_intrinsic/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/language/contract_ret_intrinsic/Forc.toml
@@ -0,0 +1,9 @@
+[project]
+authors = ["Fuel Labs <[email protected]>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "contract_ret_intrinsic"
+implicit-std = false
+
+[dependencies]
+std = { path = "../../../../reduced_std_libs/sway-lib-std-assert" }
diff --git a/...e_vm_tests/test_programs/should_pass/language/contract_ret_intrinsic/json_abi_oracle.json b/...e_vm_tests/test_programs/should_pass/language/contract_ret_intrinsic/json_abi_oracle.json
@@ -0,0 +1,25 @@
+{
+  "configurables": [],
+  "functions": [
+    {
+      "attributes": null,
+      "inputs": [],
+      "name": "main",
+      "output": {
+        "name": "",
+        "type": 0,
+        "typeArguments": null
+      }
+    }
+  ],
+  "loggedTypes": [],
+  "messagesTypes": [],
+  "types": [
+    {
+      "components": null,
+      "type": "u64",
+      "typeId": 0,
+      "typeParameters": null
+    }
+  ]
+}
diff --git a/...st_programs/should_pass/language/contract_ret_intrinsic/json_abi_oracle_new_encoding.json b/...st_programs/should_pass/language/contract_ret_intrinsic/json_abi_oracle_new_encoding.json
@@ -0,0 +1,23 @@
+{
+  "concreteTypes": [
+    {
+      "concreteTypeId": "1506e6f44c1d6291cdf46395a8e573276a4fa79e8ace3fc891e092ef32d1b0a0",
+      "type": "u64"
+    }
+  ],
+  "configurables": [],
+  "encodingVersion": "1",
+  "functions": [
+    {
+      "attributes": null,
+      "inputs": [],
+      "name": "main",
+      "output": "1506e6f44c1d6291cdf46395a8e573276a4fa79e8ace3fc891e092ef32d1b0a0"
+    }
+  ],
+  "loggedTypes": [],
+  "messagesTypes": [],
+  "metadataTypes": [],
+  "programType": "script",
+  "specVersion": "1"
+}
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/contract_ret_intrinsic/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/language/contract_ret_intrinsic/src/main.sw
@@ -0,0 +1,34 @@
+contract;
+
+abi Abi {
+    fn return_via_contract_ret(x: u8) -> u64;
+}
+
+impl Abi for Contract {
+    fn return_via_contract_ret(x: u8) -> u64 {
+        match x {
+            1 => {
+                let ret: raw_slice = encode::<u64>(100);
+                __contract_ret(ret.ptr(), ret.len::<u8>());
+                __revert(100);
+            },
+            2 => {
+                let ret: raw_slice = encode::<u64>(200);
+                __contract_ret(ret.ptr(), ret.len::<u8>());
+                __revert(200);
+            },
+            _ => __revert(0xaaa),
+        }
+    }
+}
+
+#[test]
+fn test() {
+    let caller = abi(Abi, CONTRACT_ID);
+
+    let res = caller.return_via_contract_ret(1);
+    assert_eq(100, res);
+
+    let res = caller.return_via_contract_ret(2);
+    assert_eq(200, res);
+}
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/contract_ret_intrinsic/test.toml b/test/src/e2e_vm_tests/test_programs/should_pass/language/contract_ret_intrinsic/test.toml
@@ -0,0 +1 @@
+category = "unit_tests_pass"
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/slice/slice_contract/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/language/slice/slice_contract/src/main.sw
@@ -12,8 +12,7 @@ impl MyContract for Contract {
 
 #[test]
 fn test_success() {
-    let contract_id = 0x4269c55d899c258109d170019aa45b1fd3acf6eb8c50e6139887a07ea6df1ae7; // AUTO-CONTRACT-ID .
-    let caller = abi(MyContract, contract_id);
+    let caller = abi(MyContract, CONTRACT_ID);
 
     let data = 1u64;
     let slice = raw_slice::from_parts::<u64>(__addr_of(&data), 1);

diff --git a/...m_tests/test_programs/should_pass/language/string_slice/string_slice_contract/src/main.sw b/...m_tests/test_programs/should_pass/language/string_slice/string_slice_contract/src/main.sw
@@ -12,8 +12,7 @@ impl MyContract for Contract {
 
 #[test]
 fn test_success() {
-    let contract_id = 0x573a7901ce5a722d0b80a4ad49296f8e6a23e4f6282555a561ed5118e5890ec2; // AUTO-CONTRACT-ID .
-    let caller = abi(MyContract, contract_id); 
+    let caller = abi(MyContract, CONTRACT_ID); 
     let result = caller.test_function("a");
     assert(result == "a")
 }
diff --git a/...test_programs/should_pass/require_contract_deployment/array_of_structs_caller/src/main.sw b/...test_programs/should_pass/require_contract_deployment/array_of_structs_caller/src/main.sw
@@ -6,7 +6,7 @@ use std::hash::*;
 #[cfg(experimental_new_encoding = false)]
 const CONTRACT_ID = 0x14ed3cd06c2947248f69d54bfa681fe40d26267be84df7e19e253622b7921bbe;
 #[cfg(experimental_new_encoding = true)]
-const CONTRACT_ID = 0xb7fd078d247144fb0b1505caf58ba37e1cb7a44495e135e8626fec02790b0ad4; // AUTO-CONTRACT-ID ../../test_contracts/array_of_structs_contract --release
+const CONTRACT_ID = 0xf9f1fec713b977865880637fc24e58cda9e69f6e711ed8e5efe7de9ce51c88ec; // AUTO-CONTRACT-ID ../../test_contracts/array_of_structs_contract --release
 
 fn main() -> u64 {
     let addr = abi(TestContract, CONTRACT_ID);

diff --git a/...vm_tests/test_programs/should_pass/require_contract_deployment/asset_ops_test/src/main.sw b/...vm_tests/test_programs/should_pass/require_contract_deployment/asset_ops_test/src/main.sw
@@ -9,12 +9,12 @@ use test_fuel_coin_abi::*;
 #[cfg(experimental_new_encoding = false)]
 const FUEL_COIN_CONTRACT_ID = 0xec2277ebe007ade87e3d797c3b1e070dcd542d5ef8f038b471f262ef9cebc87c;
 #[cfg(experimental_new_encoding = true)]
-const FUEL_COIN_CONTRACT_ID = 0xf5d8dc006c7686b126c6a8d4e8aa5ec53e73f23c367d2427f930cb3783c4dbb2;
+const FUEL_COIN_CONTRACT_ID = 0xf8c7b4d09f9964ab4c437ac7f5cbd6dbad7e1f218fce452c5807aac3c67afa6f;
 
 #[cfg(experimental_new_encoding = false)]
 const BALANCE_CONTRACT_ID = 0xf6cd545152ac83225e8e7df2efb5c6fa6e37bc9b9e977b5ea8103d28668925df;
 #[cfg(experimental_new_encoding = true)]
-const BALANCE_CONTRACT_ID = 0x51088b17e33a9fbbcac387cd3e462571dfce54e340579d7130c5b6fe08793ea9; // AUTO-CONTRACT-ID ../../test_contracts/balance_test_contract --release
+const BALANCE_CONTRACT_ID = 0xccf637b5b0071861f3074fcf963ef2339dab5d306d919b4c7e65024a7fbc64e6; // AUTO-CONTRACT-ID ../../test_contracts/balance_test_contract --release
 
 fn main() -> bool {
     let default_gas = 1_000_000_000_000;

diff --git a/...e2e_vm_tests/test_programs/should_pass/require_contract_deployment/bal_opcode/src/main.sw b/...e2e_vm_tests/test_programs/should_pass/require_contract_deployment/bal_opcode/src/main.sw
@@ -5,7 +5,7 @@ use balance_test_abi::BalanceTest;
 #[cfg(experimental_new_encoding = false)]
 const CONTRACT_ID = 0xf6cd545152ac83225e8e7df2efb5c6fa6e37bc9b9e977b5ea8103d28668925df;
 #[cfg(experimental_new_encoding = true)]
-const CONTRACT_ID = 0x51088b17e33a9fbbcac387cd3e462571dfce54e340579d7130c5b6fe08793ea9; // AUTO-CONTRACT-ID ../../test_contracts/balance_test_contract --release
+const CONTRACT_ID = 0xccf637b5b0071861f3074fcf963ef2339dab5d306d919b4c7e65024a7fbc64e6; // AUTO-CONTRACT-ID ../../test_contracts/balance_test_contract --release
 
 fn main() -> bool {
     let balance_test_contract = abi(BalanceTest, CONTRACT_ID);

diff --git a/...ts/test_programs/should_pass/require_contract_deployment/call_abi_with_tuples/src/main.sw b/...ts/test_programs/should_pass/require_contract_deployment/call_abi_with_tuples/src/main.sw
@@ -6,7 +6,7 @@ use abi_with_tuples::{MyContract, Location, Person};
 #[cfg(experimental_new_encoding = false)]
 const CONTRACT_ID = 0xfdc14550c8aee742cd556d0ab7f378b7be0d3b1e6e086c097352e94590d4ed02;
 #[cfg(experimental_new_encoding = true)]
-const CONTRACT_ID = 0x95a6752ed4ad94a442f54a90edcda5ff722889f6640c5ecc5dbde676443bad06; // AUTO-CONTRACT-ID ../../test_contracts/abi_with_tuples_contract --release
+const CONTRACT_ID = 0x0328999650df8c33503c894fd6ac49b0299c9d64147feb69daa1606521dbe86e; // AUTO-CONTRACT-ID ../../test_contracts/abi_with_tuples_contract --release
 
 fn main() -> bool {
     let the_abi = abi(MyContract, CONTRACT_ID);