Destructure can assign to storage variables (#1336)

Signed-off-by: Sean Young <sean@mess.org>

docs/examples/inline_assembly_calldata.sol

@@ -6,7 +6,7 @@ contract foo {
 
     test_stru storage_struct;
 
-    function bar(int256[] calldata vl) public pure {
+    function bar(int256[] calldata vl) public view {
         test_stru storage tts = storage_struct;
         assembly {
             // 'a' contains vl memory address

docs/examples/inline_assembly_storage.sol

@@ -6,7 +6,7 @@ contract foo {
 
     test_stru storage_struct;
 
-    function bar() public pure {
+    function bar() public view {
         test_stru storage tts = storage_struct;
         assembly {
             // The variables 'a' and 'b' contain zero

src/sema/mutability.rs

@@ -248,6 +248,7 @@ fn read_expression(expr: &Expression, state: &mut StateCheck) -> bool {
             right.recurse(state, read_expression);
             left.recurse(state, write_expression);
         }
+        Expression::StorageVariable { loc, .. } => state.read(loc),
         Expression::StorageArrayLength { loc, .. } | Expression::StorageLoad { loc, .. } => {
             state.read(loc)
         }

src/sema/statements.rs

@@ -1769,7 +1769,7 @@ fn destructure_values(
                 ty: right_tys[i].clone(),
                 var_no: i,
             }
-            .cast(&loc, left_ty.deref_memory(), true, ns, diagnostics)?;
+            .cast(&loc, left_ty.deref_any(), true, ns, diagnostics)?;
         }
     }
     Ok(expr)

src/sema/unused_variable.rs

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::sema::ast::{Builtin, CallArgs, Diagnostic, EventDecl, Expression, Namespace};
+use crate::sema::ast::{
+    Builtin, CallArgs, Diagnostic, EventDecl, Expression, Namespace, RetrieveType,
+};
 use crate::sema::symtable::{Symtable, VariableUsage};
 use crate::sema::{ast, symtable};
 use solang_parser::pt::{ContractTy, Loc};
@@ -27,7 +29,11 @@ pub fn assigned_variable(ns: &mut Namespace, exp: &Expression, symtable: &mut Sy
         }
 
         Expression::Subscript { array, index, .. } => {
-            assigned_variable(ns, array, symtable);
+            if array.ty().is_contract_storage() {
+                subscript_variable(ns, array, symtable);
+            } else {
+                assigned_variable(ns, array, symtable);
+            }
             used_variable(ns, index, symtable);
         }
 
@@ -43,6 +49,49 @@ pub fn assigned_variable(ns: &mut Namespace, exp: &Expression, symtable: &mut Sy
     }
 }
 
+// We have two cases here
+//  contract c {
+//      int[2] case1;
+//
+//      function f(int[2] storage case2) {
+//          case1[0] = 1;
+//          case2[0] = 1;
+//      }
+//  }
+//  The subscript for case1 is an assignment
+//  The subscript for case2 is a read
+fn subscript_variable(ns: &mut Namespace, exp: &Expression, symtable: &mut Symtable) {
+    match &exp {
+        Expression::StorageVariable {
+            contract_no,
+            var_no,
+            ..
+        } => {
+            ns.contracts[*contract_no].variables[*var_no].assigned = true;
+        }
+
+        Expression::Variable { var_no, .. } => {
+            let var = symtable.vars.get_mut(var_no).unwrap();
+            var.read = true;
+        }
+
+        Expression::StructMember { expr, .. } => {
+            subscript_variable(ns, expr, symtable);
+        }
+
+        Expression::Subscript { array, index, .. } => {
+            subscript_variable(ns, array, symtable);
+            subscript_variable(ns, index, symtable);
+        }
+
+        Expression::InternalFunctionCall { .. } | Expression::ExternalFunctionCall { .. } => {
+            check_function_call(ns, exp, symtable);
+        }
+
+        _ => (),
+    }
+}
+
 /// Mark variables as used, either in the symbol table (for local variables) or in the
 /// Namespace (for global constants and storage variables)
 /// The functions handles complex expressions in a recursive fashion, such as array length call,

src/sema/yul/tests/expression.rs

@@ -1114,7 +1114,7 @@ contract testTypes {
     }
 
     test tt1;
-    function testAsm(uint[] calldata vl) public pure {
+    function testAsm(uint[] calldata vl) public view {
         test storage tt2 = tt1;
         assembly {
             {

src/sema/yul/tests/unused_variable.rs

@@ -102,7 +102,7 @@ fn correct_contracts() {
         int b;
     }
     test tt1;
-    function testAsm(uint256[] calldata vl) pure public {
+    function testAsm(uint256[] calldata vl) view public {
         test storage tt2 = tt1;
         assembly {
             let g := vl.length

tests/contract_testcases/solana/unused_storage_variables.sol

@@ -0,0 +1,19 @@
+contract c {
+    int[2] case1;
+
+    function f(int[2] storage case2) internal {
+        case1[0] = 1;
+        case2[0] = 1;
+	g()[1] = 2;
+    }
+
+    function g() internal view returns (int[2] storage) {
+	    return case1;
+    }
+
+    function test() public {
+	    f(case1);
+    }
+}
+
+// ---- Expect: diagnostics ----

tests/contract_testcases/substrate/functions/mutability_04.sol

@@ -4,5 +4,4 @@ abstract contract test {
             }
         }
 // ---- Expect: diagnostics ----
-// warning: 2:40-43: function parameter 'foo' has never been read
 // error: 3:17-23: function declared 'view' but this expression writes to state

tests/evm.rs

@@ -248,7 +248,7 @@ fn ethereum_solidity_tests() {
         })
         .sum();
 
-    assert_eq!(errors, 1055);
+    assert_eq!(errors, 1049);
 }
 
 fn set_file_contents(source: &str, path: &Path) -> (FileResolver, Vec<String>) {

tests/solana_tests/destructure.rs

@@ -147,3 +147,37 @@ fn casting_destructure() {
 
     assert_eq!(returns, BorshToken::String(String::from("Hello")));
 }
+
+#[test]
+fn casting_storage_destructure() {
+    let mut vm = build_solidity(
+        r#"
+        contract c {
+            address factory;
+            int decimals;
+            int[2] arr1;
+            int[2] arr2;
+
+            constructor() {
+                int[2] storage x;
+
+                (x, factory, decimals) = foo();
+                x[0] = 2;
+            }
+
+            function foo() internal view returns (int[2] storage, address, int) {
+                return (arr2, address(2), 5);
+            }
+
+            function bar() public view {
+                require(factory == address(2), "address wrong");
+                require(decimals == 5, "int wrong");
+                require(arr2[0] == 2, "array wrong");
+            }
+        }"#,
+    );
+
+    vm.constructor(&[]);
+
+    vm.function("bar", &[]);
+}