Bugfix: Propagate RHS type of resolved assignments (#1569)

When resolving an assignment, the resolved variable should inherit the RHS type to avoid various issues in later compilation stages (for example with assignment chains).
hyperledger-solang · Oct 23, 2023 · fd1b874 · fd1b874
1 parent 242ebfd
commit fd1b874
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 20 deletions.
diff --git a/src/codegen/expression.rs b/src/codegen/expression.rs
@@ -2579,9 +2579,9 @@ pub fn assign_single(
         }
         _ => {
             let left_ty = left.ty();
-            let ty = left_ty.deref_memory();
+            let ty = cfg_right.ty();
 
-            let pos = vartab.temp_anonymous(ty);
+            let pos = vartab.temp_anonymous(&ty);
 
             // Set a subscript in storage bytes needs special handling
             let set_storage_bytes = if let ast::Expression::Subscript { array_ty, .. } = &left {
@@ -2667,7 +2667,7 @@ pub fn assign_single(
 
             Expression::Variable {
                 loc: left.loc(),
-                ty: ty.clone(),
+                ty,
                 var_no: pos,
             }
         }

diff --git a/tests/codegen_testcases/solidity/borsh_decoding_complex_types.sol b/tests/codegen_testcases/solidity/borsh_decoding_complex_types.sol
@@ -247,7 +247,7 @@ contract Testing {
 
         // CHECK: block20: # buffer_read
         // CHECK: ty:struct Testing.NonConstantStruct[] %arr = %temp.33
-        // CHECK: ty:struct Testing.NonConstantStruct[] storage %temp.47 = %arr
+        // CHECK: ty:struct Testing.NonConstantStruct[] %temp.47 = %arr
         // CHECK: store storage slot(uint32 16) ty:struct Testing.NonConstantStruct[] = %temp.47
 
         storage_vec = arr;

diff --git a/tests/codegen_testcases/solidity/constant_folding.sol b/tests/codegen_testcases/solidity/constant_folding.sol
@@ -19,21 +19,21 @@ contract Enum {
     mapping(Lender => address) lenders;
 
     // BEGIN-CHECK: Enum::Enum::constructor::7d622c65
-    constructor(
-        address usdt,
-        address usdc,
-        address dai
-    ) {
+    constructor(address usdt, address usdc, address dai) {
         lenders[Lender.USDT] = usdt;
         lenders[Lender.USDC] = usdc;
         lenders[Lender.DAI] = dai;
 
-        // CHECK: ty:address storage %temp.17 = (arg #0)
-        // CHECK: store storage slot(hex"f31349e4056d5e5c8ce6d8359404f2ca89b2a6884691bff0f55ce7629f869af3") ty:address = %temp.17
-        // CHECK: ty:address storage %temp.18 = (arg #1)
-        // CHECK: store storage slot(hex"e062efc721ea447b5e3918617d57f26130f3d8bc01b883eed1efcb4864d73ac1") ty:address = %temp.18
-        // CHECK: ty:address storage %temp.19 = (arg #2)
-        // CHECK: store storage slot(hex"b2573af2738ebd4810a3198e92bab190f29b8718f1d5ed1b83e468f2bb322d10") ty:address = %temp.19
+        // TODO / FIXME:
+        // We need an unused variable detection pass in codegen, and run all optimization until the CFG converges.
+        // This will get rid of the unused temp variable assignments below.
+
+        // CHECK: ty:address %temp.17 = (arg #0)
+        // CHECK: store storage slot(hex"f31349e4056d5e5c8ce6d8359404f2ca89b2a6884691bff0f55ce7629f869af3") ty:address = (arg #0)
+        // CHECK: ty:address %temp.18 = (arg #1)
+        // CHECK: store storage slot(hex"e062efc721ea447b5e3918617d57f26130f3d8bc01b883eed1efcb4864d73ac1") ty:address = (arg #1)
+        // CHECK: ty:address %temp.19 = (arg #2)
+        // CHECK: store storage slot(hex"b2573af2738ebd4810a3198e92bab190f29b8718f1d5ed1b83e468f2bb322d10") ty:address = (arg #2)
     }
 
     function foo(Lender lender) public view returns (address) {

diff --git a/tests/codegen_testcases/solidity/scale.sol b/tests/codegen_testcases/solidity/scale.sol
@@ -5,7 +5,7 @@ contract ExternalFunctions {
 
     // BEGIN-CHECK: ExternalFunctions::ExternalFunctions::function::to_storage
     function to_storage() public {
-        // CHECK: ty:function(int32) external returns (uint64) storage %temp.4 = function(int32) external returns (uint64)(function(int32) external returns (uint64)(struct { hex"42761137", (load (builtin GetAddress ())) }))
+        // CHECK: ty:function(int32) external returns (uint64) %temp.4 = function(int32) external returns (uint64)(function(int32) external returns (uint64)(struct { hex"42761137", (load (builtin GetAddress ())) }))
         // CHECK: store storage slot(uint256 0) ty:function(int32) external returns (uint64) = %temp.4
         func = this.foo;
     }

diff --git a/tests/codegen_testcases/solidity/unchecked_cse.sol b/tests/codegen_testcases/solidity/unchecked_cse.sol
@@ -7,7 +7,7 @@ contract foo {
 //BEGIN-CHECK: foo::foo::function::mul__int64_int64
     function mul(int64 a, int64 b) public returns (int64) {
         unchecked {
-            // CHECK: ty:int64 storage %temp.14 = (overflowing (arg #0) * (arg #1))
+            // CHECK: ty:int64 %temp.14 = (overflowing (arg #0) * (arg #1))
             var = a * b;
         }
 
@@ -18,7 +18,7 @@ contract foo {
 //BEGIN-CHECK: foo::foo::function::add__int64_int64
     function add(int64 a, int64 b) public returns (int64) {
         unchecked {
-            // CHECK: ty:int64 storage %temp.15 = (overflowing (arg #0) + (arg #1))
+            // CHECK: ty:int64 %temp.15 = (overflowing (arg #0) + (arg #1))
             var = a + b;
         }
         // CHECK: return ((arg #0) + (arg #1))
@@ -28,7 +28,7 @@ contract foo {
 //BEGIN-CHECK: foo::foo::function::sub__int64_int64
     function sub(int64 a, int64 b) public returns (int64) {
         unchecked {
-            // CHECK: ty:int64 storage %temp.16 = (overflowing (arg #0) - (arg #1))
+            // CHECK: ty:int64 %temp.16 = (overflowing (arg #0) - (arg #1))
             var = a - b;
         }
         // CHECK: return ((arg #0) - (arg #1))
@@ -38,7 +38,7 @@ contract foo {
 //BEGIN-CHECK: foo::foo::function::power__uint64_uint64
     function power(uint64 a, uint64 b) public returns (uint64) {
         unchecked {
-            // CHECK: ty:uint64 storage %temp.17 = (overflowing (arg #0) ** (arg #1))
+            // CHECK: ty:uint64 %temp.17 = (overflowing (arg #0) ** (arg #1))
             var2 = a ** b;
         }
 

diff --git a/tests/polkadot_tests/expressions.rs b/tests/polkadot_tests/expressions.rs
@@ -1857,3 +1857,32 @@ fn sign_extend(sign: Sign) -> u8 {
         0
     }
 }
+
+/// Given a chain of assignments, with the leftmost hand being a return parameter.
+/// It should compile fine and all values in the chain should be assigned the right most value.
+#[test]
+fn assign_chained() {
+    let mut runtime = build_solidity(
+        r#"
+    contract C {
+        uint64 public foo;
+        uint64 public bar;
+    
+        function f(uint64 x) public returns (uint64) {
+            return foo = bar = x;
+        }
+    }
+    "#,
+    );
+
+    let expected_output = 42u64.encode();
+
+    runtime.function("f", expected_output.clone());
+    assert_eq!(runtime.output(), &expected_output[..]);
+
+    runtime.function("foo", Vec::new());
+    assert_eq!(runtime.output(), &expected_output[..]);
+
+    runtime.function("bar", Vec::new());
+    assert_eq!(runtime.output(), &expected_output[..]);
+}