Allow modification of AccountInfo lamports field (hyperledger-solang#…

…1448)

The field `lamports` of the struct `AccountInfo` is mutable, so we
should allow its modification. For this we needed two modifications:

1. lamports is of type `Type::Ref(Type::Ref(Type::Uint(64)))` after
accessing the struct member, we needed to include an extra load in sema
do deal with the double pointer.
2. When we have `AccountInfo ai = tx.accounts[0];`, `ai` becomes a
pointer to AccountInfo. The unused variable elimination was incorrectly
eliminating references to structs, so I fixed such an issue.

---------

Signed-off-by: Lucas Steuernagel <lucas.tnagel@gmail.com>

src/codegen/expression.rs

@@ -364,7 +364,7 @@ pub fn expression(
             // If we reach this condition, the assignment is inside an expression.
 
             if let Some(function) = func {
-                if should_remove_assignment(left, function, opt) {
+                if should_remove_assignment(left, function, opt, ns) {
                     return expression(right, cfg, contract_no, func, ns, vartab, opt);
                 }
             }

src/codegen/statements.rs

@@ -60,7 +60,7 @@ pub(crate) fn statement(
             }
         }
         Statement::VariableDecl(loc, pos, _, Some(init)) => {
-            if should_remove_variable(*pos, func, opt) {
+            if should_remove_variable(*pos, func, opt, ns) {
                 let mut params = SideEffectsCheckParameters {
                     cfg,
                     contract_no,
@@ -125,7 +125,7 @@ pub(crate) fn statement(
             );
         }
         Statement::VariableDecl(loc, pos, param, None) => {
-            if should_remove_variable(*pos, func, opt) {
+            if should_remove_variable(*pos, func, opt, ns) {
                 return;
             }
 
@@ -172,7 +172,7 @@ pub(crate) fn statement(
         }
         Statement::Expression(_, _, expr) => {
             if let ast::Expression::Assign { left, right, .. } = &expr {
-                if should_remove_assignment(left, func, opt) {
+                if should_remove_assignment(left, func, opt, ns) {
                     let mut params = SideEffectsCheckParameters {
                         cfg,
                         contract_no,
@@ -187,7 +187,7 @@ pub(crate) fn statement(
                 }
             } else if let ast::Expression::Builtin { args, .. } = expr {
                 // When array pop and push are top-level expressions, they can be removed
-                if should_remove_assignment(expr, func, opt) {
+                if should_remove_assignment(expr, func, opt, ns) {
                     let mut params = SideEffectsCheckParameters {
                         cfg,
                         contract_no,
@@ -989,7 +989,7 @@ fn destructure(
             DestructureField::VariableDecl(res, param) => {
                 let expr = try_load_and_cast(&param.loc, &right, &param.ty, ns, cfg, vartab);
 
-                if should_remove_variable(*res, func, opt) {
+                if should_remove_variable(*res, func, opt, ns) {
                     continue;
                 }
 
@@ -1005,7 +1005,7 @@ fn destructure(
             DestructureField::Expression(left) => {
                 let expr = try_load_and_cast(&left.loc(), &right, &left.ty(), ns, cfg, vartab);
 
-                if should_remove_assignment(left, func, opt) {
+                if should_remove_assignment(left, func, opt, ns) {
                     continue;
                 }
 

src/codegen/unused_variable.rs

@@ -19,29 +19,34 @@ pub struct SideEffectsCheckParameters<'a> {
 
 /// Check if we should remove an assignment. The expression in the argument is the left-hand side
 /// of the assignment
-pub fn should_remove_assignment(exp: &Expression, func: &Function, opt: &Options) -> bool {
+pub fn should_remove_assignment(
+    exp: &Expression,
+    func: &Function,
+    opt: &Options,
+    ns: &Namespace,
+) -> bool {
     if opt.opt_level == OptimizationLevel::None {
         return false;
     }
 
     match &exp {
-        Expression::Variable { var_no, .. } => should_remove_variable(*var_no, func, opt),
+        Expression::Variable { var_no, .. } => should_remove_variable(*var_no, func, opt, ns),
 
-        Expression::StructMember { expr, .. } => should_remove_assignment(expr, func, opt),
+        Expression::StructMember { expr, .. } => should_remove_assignment(expr, func, opt, ns),
 
-        Expression::Subscript { array, .. } => should_remove_assignment(array, func, opt),
+        Expression::Subscript { array, .. } => should_remove_assignment(array, func, opt, ns),
 
         Expression::StorageLoad { expr, .. }
         | Expression::Load { expr, .. }
         | Expression::Trunc { expr, .. }
         | Expression::Cast { expr, .. }
-        | Expression::BytesCast { expr, .. } => should_remove_assignment(expr, func, opt),
+        | Expression::BytesCast { expr, .. } => should_remove_assignment(expr, func, opt, ns),
 
         Expression::Builtin {
             kind: Builtin::ArrayLength,
             args,
             ..
-        } => should_remove_assignment(&args[0], func, opt),
+        } => should_remove_assignment(&args[0], func, opt, ns),
 
         Expression::Builtin {
             kind: Builtin::ArrayPop | Builtin::ArrayPush,
@@ -53,15 +58,15 @@ pub fn should_remove_assignment(exp: &Expression, func: &Function, opt: &Options
                 return false;
             }
 
-            should_remove_assignment(&args[0], func, opt)
+            should_remove_assignment(&args[0], func, opt, ns)
         }
 
         _ => false,
     }
 }
 
 /// Checks if we should remove a variable
-pub fn should_remove_variable(pos: usize, func: &Function, opt: &Options) -> bool {
+pub fn should_remove_variable(pos: usize, func: &Function, opt: &Options, ns: &Namespace) -> bool {
     if opt.opt_level == OptimizationLevel::None {
         return false;
     }
@@ -83,7 +88,7 @@ pub fn should_remove_variable(pos: usize, func: &Function, opt: &Options) -> boo
         )
     {
         // Variables that are reference to other cannot be removed
-        return !var.is_reference();
+        return !var.is_reference(ns);
     }
 
     false

src/sema/expression/assign.rs

@@ -116,6 +116,19 @@ pub(super) fn assign_single(
             right: Box::new(val.cast(&right.loc(), var_ty, true, ns, diagnostics)?),
         }),
         _ => match &var_ty {
+            // If the variable is a Type::Ref(Type::Ref(..)), we must load it.
+            Type::Ref(inner) if matches!(**inner, Type::Ref(_)) => Ok(Expression::Assign {
+                loc: *loc,
+                ty: inner.deref_memory().clone(),
+                left: Box::new(var.cast(loc, inner, true, ns, diagnostics)?),
+                right: Box::new(val.cast(
+                    &right.loc(),
+                    inner.deref_memory(),
+                    true,
+                    ns,
+                    diagnostics,
+                )?),
+            }),
             Type::Ref(r_ty) => Ok(Expression::Assign {
                 loc: *loc,
                 ty: *r_ty.clone(),
@@ -360,6 +373,22 @@ pub(super) fn assign_expr(
                         diagnostics,
                     )?),
                 }),
+                // If the variable is a Type::Ref(Type::Ref(..)), we must load it first.
+                Type::Ref(inner)
+                    if matches!(**inner, Type::Bytes(_) | Type::Int(_) | Type::Uint(_)) =>
+                {
+                    Ok(Expression::Assign {
+                        loc: *loc,
+                        ty: *inner.clone(),
+                        left: Box::new(var.cast(loc, r_ty, true, ns, diagnostics)?),
+                        right: Box::new(assign_operation(
+                            var.cast(loc, inner, true, ns, diagnostics)?,
+                            inner,
+                            ns,
+                            diagnostics,
+                        )?),
+                    })
+                }
                 _ => {
                     diagnostics.push(Diagnostic::error(
                         var.loc(),

src/sema/symtable.rs

@@ -39,13 +39,14 @@ impl VariableInitializer {
 }
 
 impl Variable {
-    pub fn is_reference(&self) -> bool {
+    pub fn is_reference(&self, ns: &Namespace) -> bool {
         // If the variable has the memory or storage keyword, it can be a reference to another variable.
         // In this case, an assigment may change the value of the variable it is referencing.
         if matches!(
             self.storage_location,
-            Some(pt::StorageLocation::Memory(_)) | Some(pt::StorageLocation::Storage(_))
-        ) {
+            Some(pt::StorageLocation::Memory(_)) | Some(pt::StorageLocation::Storage(_)) | None
+        ) && self.ty.is_reference_type(ns)
+        {
             if let VariableInitializer::Solidity(Some(expr)) = &self.initializer {
                 // If the initializer is an array allocation, a constructor or a struct literal,
                 // the variable is not a reference to another.

src/sema/unused_variable.rs

@@ -340,15 +340,12 @@ pub fn emit_warning_local_variable(
 
         VariableUsage::LocalVariable => {
             let assigned = variable.initializer.has_initializer() || variable.assigned;
-            if !assigned && !variable.read {
+            if !variable.assigned && !variable.read {
                 return Some(Diagnostic::warning(
                     variable.id.loc,
-                    format!(
-                        "local variable '{}' has never been read nor assigned",
-                        variable.id.name
-                    ),
+                    format!("local variable '{}' is unused", variable.id.name),
                 ));
-            } else if assigned && !variable.read && !variable.is_reference() {
+            } else if assigned && !variable.read && !variable.is_reference(ns) {
                 // Values assigned to variables that reference others change the value of its reference
                 // No warning needed in this case
                 return Some(Diagnostic::warning(

tests/codegen_testcases/solidity/unused_variable_elimination.sol

@@ -77,7 +77,7 @@ contract c3 {
         c2 ct = new c2();
 
         return 3;
-// CHECK: constructor(no: ) salt: value: gas:uint64 0 address: seeds: c2 encoded buffer: %abi_encoded.temp.117 accounts: 
+// CHECK: constructor(no: ) salt: value: gas:uint64 0 address: seeds: c2 encoded buffer: %abi_encoded.temp.117 accounts:
     }
 
 // BEGIN-CHECK: c3::function::test7

tests/contract_testcases/evm/rubixi.sol

@@ -156,5 +156,5 @@
  }
 
 // ---- Expect: diagnostics ----
-// warning: 67:31-43: local variable 'payoutToSend' has been assigned, but never read
+// warning: 67:31-43: local variable 'payoutToSend' is unused
 // warning: 150:87-94: return variable 'Address' has never been assigned

tests/contract_testcases/polkadot/contracts/constructor_call.sol

@@ -14,4 +14,4 @@ contract CrowdProposalFactory {
 // ---- Expect: diagnostics ----
 // warning: 3:25-29: function parameter 'uni_' is unused
 // warning: 3:31-37: 'public': visibility for constructors is ignored
-// warning: 11:23-31: local variable 'proposal' has been assigned, but never read
+// warning: 11:23-31: local variable 'proposal' is unused

tests/contract_testcases/polkadot/contracts/contract_name_04.sol

@@ -7,4 +7,4 @@ contract test {
 // warning: 2:13-32: function can be declared 'pure'
 // warning: 3:21-25: declaration of 'test' shadows contract name
 // 	note 1:1-5:10: previous declaration of contract name
-// warning: 3:21-25: local variable 'test' has never been read nor assigned
+// warning: 3:21-25: local variable 'test' is unused

tests/contract_testcases/polkadot/contracts/contract_name_07.sol

@@ -12,5 +12,5 @@
         }
 
 // ---- Expect: diagnostics ----
-// warning: 4:19-20: local variable 'y' has been assigned, but never read
+// warning: 4:19-20: local variable 'y' is unused
 // error: 10:23-30: circular reference creating contract 'a'

tests/contract_testcases/polkadot/contracts/contract_name_08.sol

@@ -18,6 +18,6 @@
         }
 
 // ---- Expect: diagnostics ----
-// warning: 4:19-20: local variable 'y' has been assigned, but never read
-// warning: 10:19-20: local variable 'y' has been assigned, but never read
+// warning: 4:19-20: local variable 'y' is unused
+// warning: 10:19-20: local variable 'y' is unused
 // error: 16:23-30: circular reference creating contract 'a'

tests/contract_testcases/polkadot/contracts/contract_type_01.sol

@@ -6,4 +6,4 @@
         }
 // ---- Expect: diagnostics ----
 // warning: 3:13-35: function can be declared 'pure'
-// warning: 4:25-26: local variable 'x' has been assigned, but never read
+// warning: 4:25-26: local variable 'x' is unused

tests/contract_testcases/polkadot/contracts/creation_code.sol

@@ -14,5 +14,5 @@
         }
 
 // ---- Expect: diagnostics ----
-// warning: 4:27-31: local variable 'code' has been assigned, but never read
+// warning: 4:27-31: local variable 'code' is unused
 // error: 12:31-38: circular reference creating contract 'a'

tests/contract_testcases/polkadot/expressions/bad_new_args.sol

@@ -15,6 +15,6 @@ contract c {
 
 // ---- Expect: diagnostics ----
 // warning: 4:20-25: function parameter 'count' is unused
-// warning: 5:17-22: local variable 'array' has been assigned, but never read
-// warning: 8:17-22: local variable 'array' has been assigned, but never read
+// warning: 5:17-22: local variable 'array' is unused
+// warning: 8:17-22: local variable 'array' is unused
 // error: 11:36-39: new dynamic array should have an unsigned length argument

tests/contract_testcases/polkadot/format/parse_08.sol

@@ -6,4 +6,4 @@
         }
 // ---- Expect: diagnostics ----
 // warning: 3:13-34: function can be declared 'pure'
-// warning: 4:24-25: local variable 's' has been assigned, but never read
+// warning: 4:24-25: local variable 's' is unused

tests/contract_testcases/polkadot/function_types/ext_02.sol

@@ -14,6 +14,6 @@
         }
 // ---- Expect: diagnostics ----
 // warning: 3:13-35: function can be declared 'view'
-// warning: 4:57-58: local variable 'x' has been assigned, but never read
+// warning: 4:57-58: local variable 'x' is unused
 // warning: 7:13-54: function can be declared 'pure'
 // warning: 11:13-54: function can be declared 'pure'

tests/contract_testcases/polkadot/function_types/variable_or_func_type.sol

@@ -17,5 +17,5 @@ contract bar {
 
 // ---- Expect: diagnostics ----
 // warning: 10:13-33: function can be declared 'view'
-// warning: 12:21-29: local variable 'variable' has been assigned, but never read
-// warning: 14:51-60: local variable 'func_type' has been assigned, but never read
+// warning: 12:21-29: local variable 'variable' is unused
+// warning: 14:51-60: local variable 'func_type' is unused

tests/contract_testcases/polkadot/functions/external_call_args.sol

@@ -68,4 +68,5 @@ contract Swap_ETH_TO_USDX {
     }
 }
 // ---- Expect: diagnostics ----
+// warning: 60:23-30: local variable 'amounts' is unused
 // warning: 61:20-53: conversion truncates uint256 to uint128, as value is type uint128 on target Polkadot

tests/contract_testcases/polkadot/functions/shadowing.sol

@@ -18,5 +18,5 @@
 // warning: 9:9-43: function can be declared 'pure'
 // warning: 10:20-26: declaration of 'result' shadows state variable
 // 	note 3:9-22: previous declaration of state variable
-// warning: 10:20-26: local variable 'result' has been assigned, but never read
+// warning: 10:20-26: local variable 'result' is unused
 // warning: 13:9-47: function can be declared 'view'

tests/contract_testcases/polkadot/modifier/mutability_01.sol

@@ -9,7 +9,7 @@ contract c {
     function bar3() foo3() public {}
 }
 // ---- Expect: diagnostics ----
-// warning: 3:31-32: local variable 'x' has been assigned, but never read
+// warning: 3:31-32: local variable 'x' is unused
 // error: 7:21-27: function declared 'pure' but modifier reads from state
 // 	note 3:35-38: read to state
 // error: 8:21-27: function declared 'view' but modifier writes to state

tests/contract_testcases/polkadot/primitives/address_payable_type.sol

@@ -6,4 +6,4 @@
         }
 // ---- Expect: diagnostics ----
 // warning: 3:13-52: function can be declared 'pure'
-// warning: 4:25-26: local variable 'b' has been assigned, but never read
+// warning: 4:25-26: local variable 'b' is unused

tests/contract_testcases/polkadot/primitives/address_payable_type_04.sol

@@ -11,5 +11,5 @@
         }
 // ---- Expect: diagnostics ----
 // warning: 3:13-52: function can be declared 'pure'
-// warning: 4:23-24: local variable 'b' has been assigned, but never read
+// warning: 4:23-24: local variable 'b' is unused
 // warning: 9:13-35: function can be declared 'pure'

tests/contract_testcases/polkadot/primitives/address_payable_type_05.sol

@@ -6,4 +6,4 @@
         }
 // ---- Expect: diagnostics ----
 // warning: 3:13-52: function can be declared 'pure'
-// warning: 4:25-26: local variable 'b' has been assigned, but never read
+// warning: 4:25-26: local variable 'b' is unused

tests/contract_testcases/polkadot/primitives/address_payable_type_07.sol

@@ -6,4 +6,4 @@
         }
 // ---- Expect: diagnostics ----
 // warning: 3:13-44: function can be declared 'pure'
-// warning: 4:33-34: local variable 'b' has been assigned, but never read
+// warning: 4:33-34: local variable 'b' is unused

tests/contract_testcases/polkadot/strings/basic_tests_03.sol

@@ -6,4 +6,4 @@
         }
 // ---- Expect: diagnostics ----
 // warning: 3:13-34: function can be declared 'pure'
-// warning: 4:28-29: local variable 'f' has been assigned, but never read
+// warning: 4:28-29: local variable 'f' is unused

tests/contract_testcases/polkadot/strings/basic_tests_04.sol

@@ -6,4 +6,4 @@
         }
 // ---- Expect: diagnostics ----
 // warning: 3:13-34: function can be declared 'pure'
-// warning: 4:27-28: local variable 'f' has been assigned, but never read
+// warning: 4:27-28: local variable 'f' is unused

tests/contract_testcases/polkadot/value/external_call_value_09.sol

@@ -18,5 +18,5 @@
 // warning: 8:13-18: storage variable 'x' has never been used
 // warning: 11:25-26: declaration of 'x' shadows state variable
 // 	note 8:13-18: previous declaration of state variable
-// warning: 11:25-26: local variable 'x' has been assigned, but never read
+// warning: 11:25-26: local variable 'x' is unused
 // warning: 13:31-32: conversion truncates uint256 to uint128, as value is type uint128 on target Polkadot

tests/contract_testcases/polkadot/variables/ext_fn_call_is_reading_variable_02.sol

@@ -10,4 +10,4 @@ contract A {
 }
 
 // ---- Expect: diagnostics ----
-// warning: 4:37-41: local variable 'func' has been assigned, but never read
+// warning: 4:37-41: local variable 'func' is unused

tests/contract_testcases/solana/expressions/units.sol

@@ -8,7 +8,7 @@
         }
 // ---- Expect: diagnostics ----
 // warning: 3:13-35: function can be declared 'pure'
-// warning: 4:23-28: local variable 'ether' has been assigned, but never read
+// warning: 4:23-28: local variable 'ether' is unused
 // warning: 4:31-38: ethereum currency unit used while targeting Solana
-// warning: 5:23-26: local variable 'sol' has been assigned, but never read
-// warning: 6:23-31: local variable 'lamports' has been assigned, but never read
+// warning: 5:23-26: local variable 'sol' is unused
+// warning: 6:23-31: local variable 'lamports' is unused

tests/contract_testcases/solana/large_contract.sol

@@ -897,4 +897,5 @@ contract ServiceRegistrySolana {
 // ---- Expect: diagnostics ----
 // warning: 204:5-209:14: function can be declared 'pure'
 // warning: 500:22-26: function parameter 'data' is unused
+// warning: 525:26-40: local variable 'agentInstances' is unused
 // warning: 867:31-35: function parameter 'data' is unused