Implement tests for unused variable detection

Signed-off-by: LucasSte <[email protected]>

.gitignore

@@ -2,5 +2,4 @@
 Cargo.lock
 /target
 **/*.rs.bk
-.DS_Store
-.idea
+

Cargo.toml

@@ -30,7 +30,7 @@ serde = "1.0"
 serde_derive = { version = "1.0" }
 inkwell = { version = "0.1.0-beta.2", features = ["target-webassembly", "target-bpf", "llvm11-0"] }
 blake2-rfc = "0.2.18"
-phf = { version = "0.8", features = ["macros"] }
+phf = { version = "0.9", features = ["macros"] }
 unicode-xid = "0.2.0"
 handlebars = "3.4"
 contract-metadata = "0.2.0"
@@ -54,7 +54,7 @@ ethereum-types = "0.10"
 wasmi = "0.9"
 rand = "0.8"
 sha2 = "0.9"
-solana_rbpf = "0.2"
+solana_rbpf = "^0.2.11"
 byteorder = "1.3"
 assert_cmd = "1.0"
 bincode = "1.3"

build.rs

@@ -11,7 +11,7 @@ fn main() {
     let cxxflags = Command::new("llvm-config")
         .args(&["--cxxflags"])
         .output()
-        .unwrap();
+        .expect("could not execute llvm-config");
 
     let cxxflags = String::from_utf8(cxxflags.stdout).unwrap();
 

src/codegen/cfg.rs

@@ -616,6 +616,12 @@ impl ControlFlowGraph {
                 ty.to_string(ns),
                 self.expr_to_string(contract, ns, e)
             ),
+            Expression::BytesCast(_, ty, from, e) => format!(
+                "{} from:{} ({})",
+                ty.to_string(ns),
+                from.to_string(ns),
+                self.expr_to_string(contract, ns, e)
+            ),
             Expression::Builtin(_, _, builtin, args) => format!(
                 "(builtin {:?} ({}))",
                 builtin,

src/codegen/constant_folding.rs

@@ -1062,7 +1062,7 @@ fn expression(
         | Expression::FormatString { .. }
         | Expression::InternalFunctionCfg(_) => (expr.clone(), false),
         // nothing else is permitted in cfg
-        _ => unreachable!(),
+        _ => panic!("expr should not be in cfg: {:?}", expr),
     }
 }
 

src/codegen/expression.rs

@@ -708,6 +708,12 @@ pub fn expression(
                 )
             }
         }
+        Expression::BytesCast(loc, ty, from, e) => Expression::BytesCast(
+            *loc,
+            ty.clone(),
+            from.clone(),
+            Box::new(expression(e, cfg, contract_no, ns, vartab)),
+        ),
         Expression::Load(loc, ty, e) => Expression::Load(
             *loc,
             ty.clone(),
@@ -1784,24 +1790,25 @@ fn array_subscript(
             Type::Bytes(array_length) => {
                 let res_ty = Type::Bytes(1);
                 let from_ty = Type::Bytes(*array_length);
+                let index_ty = Type::Uint(*array_length as u16 * 8);
 
                 Expression::Trunc(
                     *loc,
                     res_ty,
                     Box::new(Expression::ShiftRight(
                         *loc,
-                        from_ty.clone(),
+                        from_ty,
                         Box::new(array),
                         // shift by (array_length - 1 - index) * 8
                         Box::new(Expression::ShiftLeft(
                             *loc,
-                            from_ty.clone(),
+                            index_ty.clone(),
                             Box::new(Expression::Subtract(
                                 *loc,
-                                from_ty.clone(),
+                                index_ty.clone(),
                                 Box::new(Expression::NumberLiteral(
                                     *loc,
-                                    from_ty.clone(),
+                                    index_ty.clone(),
                                     BigInt::from_u8(array_length - 1).unwrap(),
                                 )),
                                 Box::new(cast_shift_arg(
@@ -1814,7 +1821,7 @@ fn array_subscript(
                             )),
                             Box::new(Expression::NumberLiteral(
                                 *loc,
-                                from_ty,
+                                index_ty,
                                 BigInt::from_u8(3).unwrap(),
                             )),
                         )),

src/sema/contracts.rs

@@ -870,9 +870,9 @@ fn resolve_bodies(
         {
             broken = true;
         } else {
-            for (_, variable) in &ns.functions[function_no].symtable.vars {
+            for variable in ns.functions[function_no].symtable.vars.values() {
                 if let Some(warning) = emit_warning_local_variable(&variable) {
-                    ns.diagnostics.push(*warning);
+                    ns.diagnostics.push(warning);
                 }
             }
         }

src/sema/expression.rs

@@ -1321,17 +1321,25 @@ pub fn expression(
     resolve_to: Option<&Type>,
 ) -> Result<Expression, ()> {
     match expr {
-        pt::Expression::ArrayLiteral(loc, exprs) => resolve_array_literal(
-            loc,
-            exprs,
-            file_no,
-            contract_no,
-            ns,
-            symtable,
-            is_constant,
-            diagnostics,
-            resolve_to,
-        ),
+        pt::Expression::ArrayLiteral(loc, exprs) => {
+            let res = resolve_array_literal(
+                loc,
+                exprs,
+                file_no,
+                contract_no,
+                ns,
+                symtable,
+                is_constant,
+                diagnostics,
+                resolve_to,
+            );
+
+            if let Ok(exp) = &res {
+                used_variable(ns, exp, symtable);
+            }
+
+            res
+        }
         pt::Expression::BoolLiteral(loc, v) => Ok(Expression::BoolLiteral(*loc, *v)),
         pt::Expression::StringLiteral(v) => {
             // Concatenate the strings
@@ -2479,6 +2487,7 @@ pub fn expression(
                 diagnostics,
                 resolve_to,
             )?;
+            used_variable(ns, &cond, symtable);
 
             let cond = cast(&c.loc(), cond, &Type::Bool, true, ns, diagnostics)?;
 
@@ -2759,26 +2768,18 @@ pub fn expression(
             is_constant,
             diagnostics,
         ),
-        pt::Expression::MemberAccess(loc, e, id) => {
-            let res = member_access(
-                loc,
-                e,
-                id,
-                file_no,
-                contract_no,
-                ns,
-                symtable,
-                is_constant,
-                diagnostics,
-                resolve_to,
-            );
-
-            if let Ok(exp) = &res {
-                used_variable(ns, &exp, symtable);
-            }
-
-            res
-        }
+        pt::Expression::MemberAccess(loc, e, id) => member_access(
+            loc,
+            e,
+            id,
+            file_no,
+            contract_no,
+            ns,
+            symtable,
+            is_constant,
+            diagnostics,
+            resolve_to,
+        ),
         pt::Expression::Or(loc, left, right) => {
             let boolty = Type::Bool;
             let l = cast(
@@ -4426,7 +4427,10 @@ fn member_access(
             if id.name == "length" {
                 return match dim.last().unwrap() {
                     None => Ok(Expression::DynamicArrayLength(*loc, Box::new(expr))),
-                    Some(d) => bigint_to_expression(loc, d, ns, diagnostics, Some(&Type::Uint(32))),
+                    Some(d) => {
+                        used_variable(ns, &expr, symtable);
+                        bigint_to_expression(loc, d, ns, diagnostics, Some(&Type::Uint(32)))
+                    }
                 };
             }
         }
@@ -4793,7 +4797,7 @@ fn struct_literal(
                 diagnostics,
                 Some(&struct_def.fields[i].ty),
             )?;
-
+            used_variable(ns, &expr, symtable);
             fields.push(cast(
                 loc,
                 expr,
@@ -5408,7 +5412,7 @@ fn named_struct_literal(
                         diagnostics,
                         Some(&f.ty),
                     )?;
-
+                    used_variable(ns, &expr, symtable);
                     fields[i] = cast(loc, expr, &f.ty, true, ns, diagnostics)?;
                 }
                 None => {
@@ -6640,6 +6644,7 @@ fn resolve_array_literal(
         first.ty()
     };
 
+    used_variable(ns, &first, symtable);
     let mut exprs = vec![first];
 
     for e in flattened {

src/sema/mod.rs

@@ -41,16 +41,18 @@ pub const SOLANA_BUCKET_SIZE: u64 = 251;
 pub const SOLANA_FIRST_OFFSET: u64 = 16;
 pub const SOLANA_SPARSE_ARRAY_SIZE: u64 = 1024;
 
-/// Performs semantic analysis and checks for unused variables
+/// Load a file file from the cache, parse and resolve it. The file must be present in
+/// the cache.
 pub fn sema(file: ResolvedFile, cache: &mut FileCache, ns: &mut ast::Namespace) {
-    perform_sema(file, cache, ns);
+    sema_file(file, cache, ns);
+
+    // Checks for unused variables
     check_unused_namespace_variables(ns);
     check_unused_events(ns);
 }
 
-/// Load a file file from the cache, parse and resolve it. The file must be present in
-/// the cache. This function is recursive for imports.
-pub fn perform_sema(file: ResolvedFile, cache: &mut FileCache, ns: &mut ast::Namespace) {
+/// Parse and resolve a file and its imports in a recursive manner.
+fn sema_file(file: ResolvedFile, cache: &mut FileCache, ns: &mut ast::Namespace) {
     let file_no = ns.files.len();
 
     let source_code = cache.get_file_contents(&file.full_path);
@@ -177,7 +179,7 @@ fn resolve_import(
         }
         Ok(file) => {
             if !ns.files.iter().any(|f| *f == file.full_path) {
-                perform_sema(file.clone(), cache, ns);
+                sema_file(file.clone(), cache, ns);
 
                 // give up if we failed
                 if diagnostics::any_errors(&ns.diagnostics) {