Remove unused return value of var_decl

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

src/sema/contracts.rs

@@ -749,7 +749,8 @@ fn resolve_declarations<'a>(
     let mut function_no_bodies = Vec::new();
     let mut resolve_bodies = Vec::new();
 
-    // resolve state variables. We may need a constant to resolve a function type
+    // resolve state variables. We may need a constant to resolve the array
+    // dimension of a function argument.
     variables::contract_variables(def, file_no, contract_no, ns);
 
     // resolve function signatures

src/sema/mod.rs

@@ -33,7 +33,7 @@ use self::eval::eval_const_number;
 use self::expression::{expression, ExprContext, ResolveTo};
 use self::functions::{resolve_params, resolve_returns};
 use self::symtable::Symtable;
-use self::variables::var_decl;
+use self::variables::variable_decl;
 use crate::file_resolver::{FileResolver, ResolvedFile};
 use crate::parser::pt::CodeLocation;
 use crate::parser::pt::OptionalCodeLocation;
@@ -129,7 +129,7 @@ fn sema_file(file: &ResolvedFile, resolver: &mut FileResolver, ns: &mut ast::Nam
                 }
             }
             pt::SourceUnitPart::VariableDefinition(var) => {
-                var_decl(None, var, file_no, None, ns, &mut Symtable::new());
+                variable_decl(None, var, file_no, None, ns, &mut Symtable::new());
             }
             _ => (),
         }

src/sema/variables.rs

@@ -15,54 +15,24 @@ pub fn contract_variables(
     file_no: usize,
     contract_no: usize,
     ns: &mut Namespace,
-) -> bool {
-    let mut broken = false;
+) {
     let mut symtable = Symtable::new();
 
     for parts in &def.parts {
         if let pt::ContractPart::VariableDefinition(ref s) = parts {
-            // don't even attempt to parse contract variables for interfaces, they are never allowed
-            if matches!(def.ty, pt::ContractTy::Interface(_)) {
-                ns.diagnostics.push(Diagnostic::error(
-                    s.loc,
-                    format!(
-                        "{} ‘{}’ is not allowed to have contract variable ‘{}’",
-                        def.ty, def.name.name, s.name.name
-                    ),
-                ));
-                broken = true;
-                continue;
-            }
-
-            match var_decl(Some(def), s, file_no, Some(contract_no), ns, &mut symtable) {
-                None => {
-                    broken = true;
-                }
-                Some(false) if matches!(def.ty, pt::ContractTy::Library(_)) => {
-                    ns.diagnostics.push(Diagnostic::error(
-                        s.loc,
-                        format!(
-                            "{} ‘{}’ is not allowed to have state variable ‘{}’",
-                            def.ty, def.name.name, s.name.name
-                        ),
-                    ));
-                }
-                _ => (),
-            }
+            variable_decl(Some(def), s, file_no, Some(contract_no), ns, &mut symtable);
         }
     }
-
-    broken
 }
 
-pub fn var_decl(
+pub fn variable_decl(
     contract: Option<&pt::ContractDefinition>,
     s: &pt::VariableDefinition,
     file_no: usize,
     contract_no: Option<usize>,
     ns: &mut Namespace,
     symtable: &mut Symtable,
-) -> Option<bool> {
+) {
     let mut attrs = s.attrs.clone();
     let mut ty = s.ty.clone();
 
@@ -96,7 +66,7 @@ pub fn var_decl(
         Ok(s) => s,
         Err(()) => {
             ns.diagnostics.extend(diagnostics);
-            return None;
+            return;
         }
     };
 
@@ -143,14 +113,14 @@ pub fn var_decl(
                     v.loc().unwrap(),
                     format!("‘{}’: global variable cannot have visibility specifier", v),
                 ));
-                return None;
+                return;
             }
             pt::VariableAttribute::Visibility(pt::Visibility::External(loc)) => {
                 ns.diagnostics.push(Diagnostic::error(
                     loc.unwrap(),
                     "variable cannot be declared external".to_string(),
                 ));
-                return None;
+                return;
             }
             pt::VariableAttribute::Visibility(v) => {
                 if let Some(e) = &visibility {
@@ -160,7 +130,7 @@ pub fn var_decl(
                         e.loc().unwrap(),
                         format!("location of previous declaration of `{}'", e),
                     ));
-                    return None;
+                    return;
                 }
 
                 visibility = Some(v.clone());
@@ -193,22 +163,37 @@ pub fn var_decl(
         has_override = None;
     }
 
-    if contract_no.is_none() {
+    if let Some(def) = contract {
+        if matches!(def.ty, pt::ContractTy::Interface(_))
+            || (matches!(def.ty, pt::ContractTy::Library(_)) && !constant)
+        {
+            ns.diagnostics.push(Diagnostic::error(
+                s.loc,
+                format!(
+                    "{} ‘{}’ is not allowed to have contract variable ‘{}’",
+                    def.ty, def.name.name, s.name.name
+                ),
+            ));
+            return;
+        }
+    } else {
         if !constant {
             ns.diagnostics.push(Diagnostic::error(
                 s.ty.loc(),
                 "global variable must be constant".to_string(),
             ));
-            return None;
+            return;
         }
         if ty.contains_internal_function(ns) {
             ns.diagnostics.push(Diagnostic::error(
                 s.ty.loc(),
                 "global variable cannot be of type internal function".to_string(),
             ));
-            return None;
+            return;
         }
-    } else if ty.contains_internal_function(ns)
+    }
+
+    if ty.contains_internal_function(ns)
         && matches!(
             visibility,
             pt::Visibility::Public(_) | pt::Visibility::External(_)
@@ -221,21 +206,15 @@ pub fn var_decl(
                 visibility
             ),
         ));
-        return None;
+        return;
     } else if let Some(ty) = ty.contains_builtins(ns, BuiltinStruct::AccountInfo) {
-        let message = format!(
-            "variable of cannot be builtin of type ‘{}’",
-            ty.to_string(ns)
-        );
+        let message = format!("variable cannot be of builtin type ‘{}’", ty.to_string(ns));
         ns.diagnostics.push(Diagnostic::error(s.ty.loc(), message));
-        return None;
+        return;
     } else if let Some(ty) = ty.contains_builtins(ns, BuiltinStruct::AccountMeta) {
-        let message = format!(
-            "variable of cannot be builtin of type ‘{}’",
-            ty.to_string(ns)
-        );
+        let message = format!("variable cannot be of builtin type ‘{}’", ty.to_string(ns));
         ns.diagnostics.push(Diagnostic::error(s.ty.loc(), message));
-        return None;
+        return;
     }
 
     let initializer = if let Some(initializer) = &s.initializer {
@@ -422,9 +401,6 @@ pub fn var_decl(
             );
         }
     }
-
-    // Return true if the value is constant
-    Some(constant)
 }
 
 /// For accessor functions, create the parameter list and the return expression

tests/contract_testcases/solana/account_info.dot

@@ -6,7 +6,7 @@ strict digraph "tests/contract_testcases/solana/account_info.sol" {
 	parameters [label="parameters\nstruct AccountInfo "]
 	returns [label="returns\nstruct AccountInfo "]
 	diagnostic [label="found contract ‘c’\nlevel Debug\ntests/contract_testcases/solana/account_info.sol:1:1-12"]
-	diagnostic_10 [label="variable of cannot be builtin of type ‘struct AccountInfo’\nlevel Error\ntests/contract_testcases/solana/account_info.sol:2:2-13"]
+	diagnostic_10 [label="variable cannot be of builtin type ‘struct AccountInfo’\nlevel Error\ntests/contract_testcases/solana/account_info.sol:2:2-13"]
 	diagnostic_11 [label="parameter of type ‘struct AccountInfo’ not alowed in public or external functions\nlevel Error\ntests/contract_testcases/solana/account_info.sol:4:15-26"]
 	diagnostic_12 [label="return type ‘struct AccountInfo’ not allowed in public or external functions\nlevel Error\ntests/contract_testcases/solana/account_info.sol:4:44-55"]
 	diagnostic_13 [label="builtin struct ‘AccountInfo’ cannot be created using struct literal\nlevel Error\ntests/contract_testcases/solana/account_info.sol:8:8-36"]

tests/contract_testcases/substrate/libraries/restrictions_08.dot

@@ -1,12 +1,8 @@
 strict digraph "tests/contract_testcases/substrate/libraries/restrictions_08.sol" {
 	contract [label="contract c\ntests/contract_testcases/substrate/libraries/restrictions_08.sol:2:9-19"]
-	var [label="variable x\nvisibility internal\ntests/contract_testcases/substrate/libraries/restrictions_08.sol:3:13-18"]
 	diagnostic [label="found library ‘c’\nlevel Debug\ntests/contract_testcases/substrate/libraries/restrictions_08.sol:2:9-19"]
-	diagnostic_6 [label="library ‘c’ is not allowed to have state variable ‘x’\nlevel Error\ntests/contract_testcases/substrate/libraries/restrictions_08.sol:3:13-18"]
-	diagnostic_7 [label="storage variable ‘x‘ has never been used\nlevel Warning\ntests/contract_testcases/substrate/libraries/restrictions_08.sol:3:13-18"]
+	diagnostic_5 [label="library ‘c’ is not allowed to have contract variable ‘x’\nlevel Error\ntests/contract_testcases/substrate/libraries/restrictions_08.sol:3:13-18"]
 	contracts -> contract
-	contract -> var [label="variable"]
 	diagnostics -> diagnostic [label="Debug"]
-	diagnostics -> diagnostic_6 [label="Error"]
-	diagnostics -> diagnostic_7 [label="Warning"]
+	diagnostics -> diagnostic_5 [label="Error"]
 }