assignments, local variables, better tests, other bug fixes (#332)

ivoysey · web-flow · commit eba0ad5e9775 · 2021-05-06T15:36:49.000-04:00
* formatting the test, removing old code

* new test case with less going on

* updating primops test case to use local vars not fields

* scala style, whitespace, a new util function, comments, variable declaration first cut

* updating bool literal test to refer to a local rather than a field

* no prizes for guessing where i copied this file from

* assign instead of declare; removing type annotations from declarations output with a comment about why

* whitespace

* similar enough case for decls, so might as well

* updating ifthenelse test case to use locals not fields, fixing a bug in the output code

* adding case for if, since it\'s so similar to ifthenelse

* intconst test to locals not fields

* removing sstore for local variables

* fixing scaladocs

* updating simplecall test to  not use fields

* adding a top level return to simple call so its testable

* scala style

* whitespace

* removing temp symbol table
diff --git a/resources/tests/GanacheTests/AssignLocalAdd.json b/resources/tests/GanacheTests/AssignLocalAdd.json
@@ -0,0 +1,6 @@
+{
+    "gas" : 30000000,
+    "gasprice" : "0x9184e72a000",
+    "startingeth" : 5000000,
+    "numaccts" : 1
+}
diff --git a/resources/tests/GanacheTests/AssignLocalAdd.obs b/resources/tests/GanacheTests/AssignLocalAdd.obs
@@ -0,0 +1,8 @@
+main contract AssignLocalAdd{
+    transaction assignlocaladd() {
+        int x;
+        bool y;
+        x = 5 + 12;
+        return;
+    }
+}
diff --git a/resources/tests/GanacheTests/BoolLiteral.obs b/resources/tests/GanacheTests/BoolLiteral.obs
@@ -1,7 +1,6 @@
 main contract BoolLiteral{
-	 bool x;
-
 	 transaction BoolLiteral () {
+	     bool x;
 	     x = true;
 	     x = false;
 	 }
diff --git a/resources/tests/GanacheTests/If.json b/resources/tests/GanacheTests/If.json
@@ -0,0 +1,6 @@
+{
+    "gas" : 30000000,
+    "gasprice" : "0x9184e72a000",
+    "startingeth" : 5000000,
+    "numaccts" : 1
+}
diff --git a/resources/tests/GanacheTests/If.obs b/resources/tests/GanacheTests/If.obs
@@ -0,0 +1,9 @@
+main contract If {
+        transaction iftest() {
+            int x;
+            if (true) {
+	       x = 1;
+	    }
+            return;
+        }
+}
diff --git a/resources/tests/GanacheTests/IfThenElse.obs b/resources/tests/GanacheTests/IfThenElse.obs
@@ -1,11 +1,11 @@
 main contract IfThenElse {
-        int x;
         transaction ifthenelse() {
+            int x;
             if (true) {
-	       x = 1;
-	    } else {
-	       x = 0;
-	    }
+	            x = 1;
+	        } else {
+	            x = 0;
+	        }
             return;
         }
 }
diff --git a/resources/tests/GanacheTests/IntConst.obs b/resources/tests/GanacheTests/IntConst.obs
@@ -1,6 +1,6 @@
 main contract IntConst{
-        int x;
         transaction intconst() {
+            int x;
             x = 4;
             return;
         }
diff --git a/resources/tests/GanacheTests/PrimOps.obs b/resources/tests/GanacheTests/PrimOps.obs
@@ -1,28 +1,28 @@
 main contract PrimOps{
-        int x;
-        bool y;
-
         transaction primops() {
-            y = true && false;
-            y = true || false;
+           int x;
+           bool y;
+
+           y = true && false;
+           y = true || false;
 
-            x = 4 + 4;
-            x = 4 - 4;
-            x = 4 / 4;
-            x = 4 * 4;
-            x = 4 % 4;
+           x = 4 + 4;
+           x = 4 - 4;
+           x = 4 / 4;
+           x = 4 * 4;
+           x = 4 % 4;
 
-            y = 1 == 2;
-            y = 1 != 2;
-            y = 3 > 4;
-            y = 3 >= 4;
-            y = 3 < 3;
-            y = 3 <= 3;
-            y = 3 != 3;
+           y = 1 == 2;
+           y = 1 != 2;
+           y = 3 > 4;
+           y = 3 >= 4;
+           y = 3 < 3;
+           y = 3 <= 3;
+           y = 3 != 3;
 
-            y = !y;
-            x = - x;
+           y = !y;
+           x = - x;
 
-            return;
+           return;
         }
 }
diff --git a/resources/tests/GanacheTests/SimpleCall.obs b/resources/tests/GanacheTests/SimpleCall.obs
@@ -1,12 +1,11 @@
 main contract SimpleCall{
-        int x;
-        transaction updatex() {
-            x = 4;
-            return;
+        transaction val() returns int {
+            return 4;
         }
-        transaction main(){
+        transaction main() returns int{
+            int x;
             x = 9;
-            updatex();
-            return;
+            x = val();
+            return x;
         }
 }
diff --git a/src/main/scala/edu/cmu/cs/obsidian/codegen/CodeGenYul.scala b/src/main/scala/edu/cmu/cs/obsidian/codegen/CodeGenYul.scala
@@ -1,7 +1,6 @@
 package edu.cmu.cs.obsidian.codegen
 
 import edu.cmu.cs.obsidian.CompilerOptions
-import edu.cmu.cs.obsidian.codegen.LiteralKind.LiteralKind
 
 import java.io.{File, FileWriter}
 import java.nio.file.{Files, Path, Paths}
@@ -17,7 +16,6 @@ import scala.collection.immutable.Map
 object CodeGenYul extends CodeGenerator {
 
     // TODO improve this temporary symbol table
-    var tempSymbolTable: Map[String, Int] = Map() // map from field identifiers to index in storage
     var tempTableIdx: Int = 0 // counter indicating the next available slot in the table
     var stateIdx: Int = -1 // whether or not there is a state
     var stateEnumMapping: Map[String, Int] = Map() // map from state name to an enum value
@@ -156,7 +154,6 @@ object CodeGenYul extends CodeGenerator {
     def translateField(f: Field): Seq[YulStatement] = {
         // Reserve a slot in the storage by assigning a index in the symbol table
         // since field declaration has not yet be assigned, there is no need to do sstore
-        tempSymbolTable += f.name -> tempTableIdx
         tempTableIdx += 1
         Seq() // TODO: do we really mean to always return the empty sequence?
     }
@@ -198,7 +195,6 @@ object CodeGenYul extends CodeGenerator {
             }
         }
 
-
         Seq(FunctionDefinition(
             transaction.name, // TODO rename transaction name (by adding prefix/suffix)
             transaction.args.map(v => TypedName(v.varName, mapObsTypeToABI(v.typIn.toString))),
@@ -222,46 +218,55 @@ object CodeGenYul extends CodeGenerator {
                     case Some(retVarName) =>
                         val temp_id = nextTemp()
                         val e_yul = translateExpr(temp_id, e, contractName, checkedTable)
-                        decl_0exp(temp_id) +: e_yul :+ assign1(Identifier(retVarName), temp_id) :+ Leave()
+                        decl_0exp(temp_id) +:
+                            e_yul :+
+                            assign1(Identifier(retVarName), temp_id) :+
+                            Leave()
                     case None => assert(assertion = false, "error: returning an expression from a transaction without a return type")
                         Seq()
                 }
             case Assignment(assignTo, e) =>
                 assignTo match {
                     case ReferenceIdentifier(x) =>
-                        val kind: LiteralKind = e match {
-                            case NumLiteral(_) => LiteralKind.number
-                            case TrueLiteral() => LiteralKind.boolean
-                            case FalseLiteral() => LiteralKind.boolean
-                            case StringLiteral(_) => LiteralKind.string
-                            case _ =>
-                                assert(assertion = false, s"unimplemented assignment case ${assignTo.toString}")
-                                LiteralKind.number
-                        }
-                        Seq(ExpressionStatement(FunctionCall(Identifier("sstore"),
-                            Seq(intlit(tempSymbolTable(x)), Literal(kind, e.toString, kind.toString))))) //todo: this is likely wrong for strings
-                    case e =>
-                        assert(assertion = false, "TODO: translate assignment case" + e.toString)
+                        //todo: easy optimization is to look at e; if it happens to be a literal we can save a temp.
+                        val id = nextTemp()
+                        val e_yul = translateExpr(id, e, contractName, checkedTable)
+                        decl_0exp(id) +:
+                            e_yul :+
+                            assign1(Identifier(x), id)
+                    case _ =>
+                        assert(assertion = false, "trying to assign to non-assignable: " + e.toString)
                         Seq()
                 }
             case IfThenElse(scrutinee, pos, neg) =>
-                val id = nextTemp()
-                val scrutinee_yul: Seq[YulStatement] = translateExpr(id, scrutinee, contractName, checkedTable)
-                val pos_yul: Seq[YulStatement] = pos.flatMap(s => translateStatement(s, retVar, contractName, checkedTable))
-                val neg_yul: Seq[YulStatement] = neg.flatMap(s => translateStatement(s, retVar, contractName, checkedTable))
-                decl_0exp(id) +:
+                val id_scrutinee: Identifier = nextTemp()
+                val scrutinee_yul: Seq[YulStatement] = translateExpr(id_scrutinee, scrutinee, contractName, checkedTable)
+                val pos_yul: Seq[YulStatement] =
+                    pos.flatMap(s => {
+                        val id_s: Identifier = nextTemp()
+                        decl_0exp(id_s) +: translateStatement(s, Some(id_s.name), contractName, checkedTable)
+                    })
+                val neg_yul: Seq[YulStatement] =
+                    neg.flatMap(s => {
+                        val id_s: Identifier = nextTemp()
+                        decl_0exp(id_s) +: translateStatement(s, Some(id_s.name), contractName, checkedTable)
+                    })
+
+                decl_0exp(id_scrutinee) +:
                     scrutinee_yul :+
-                    edu.cmu.cs.obsidian.codegen.Switch(id,
+                    edu.cmu.cs.obsidian.codegen.Switch(id_scrutinee,
                         Seq(
                             Case(boollit(true), Block(pos_yul)),
                             Case(boollit(false), Block(neg_yul))))
             case e: Expression => translateExpr(nextTemp(), e, contractName, checkedTable)
             case VariableDecl(typ, varName) =>
-                assert(assertion = false, s"TODO: translateStatement unimplemented for ${s.toString}")
-                Seq()
+                Seq(decl_0exp_t(Identifier(varName), typ))
             case VariableDeclWithInit(typ, varName, e) =>
-                assert(assertion = false, s"TODO: translateStatement unimplemented for ${s.toString}")
-                Seq()
+                val id = nextTemp()
+                val e_yul = translateExpr(id, e, contractName, checkedTable)
+                decl_0exp(id) +:
+                    e_yul :+
+                    decl_0exp_t_init(Identifier(varName), typ, id)
             case VariableDeclWithSpec(typIn, typOut, varName) =>
                 assert(assertion = false, s"TODO: translateStatement unimplemented for ${s.toString}")
                 Seq()
@@ -271,9 +276,18 @@ object CodeGenYul extends CodeGenerator {
             case Revert(maybeExpr) =>
                 assert(assertion = false, s"TODO: translateStatement unimplemented for ${s.toString}")
                 Seq()
-            case If(eCond, s) =>
-                assert(assertion = false, s"TODO: translateStatement unimplemented for ${s.toString}")
-                Seq()
+            case If(scrutinee, s) =>
+                val id_scrutinee: Identifier = nextTemp()
+                val scrutinee_yul: Seq[YulStatement] = translateExpr(id_scrutinee, scrutinee, contractName, checkedTable)
+                val s_yul: Seq[YulStatement] =
+                    s.flatMap(s => {
+                        val id_s: Identifier = nextTemp()
+                        decl_0exp(id_s) +: translateStatement(s, Some(id_s.name), contractName, checkedTable)
+                    })
+
+                decl_0exp(id_scrutinee) +:
+                    scrutinee_yul :+
+                    edu.cmu.cs.obsidian.codegen.If(id_scrutinee, Block(s_yul))
             case IfInState(e, ePerm, typeState, s1, s2) =>
                 assert(assertion = false, s"TODO: translateStatement unimplemented for ${s.toString}")
                 Seq()
@@ -325,7 +339,7 @@ object CodeGenYul extends CodeGenerator {
             case e: AtomicExpression =>
                 e match {
                     case ReferenceIdentifier(x) =>
-                        Seq(assign1(retvar, apply("sload", intlit(tempSymbolTable(x)))))
+                        Seq(assign1(retvar, Identifier(x))) //todo: in general this will need to know to look in memory / storage / stack
                     case NumLiteral(n) =>
                         Seq(assign1(retvar, intlit(n)))
                     case StringLiteral(value) =>
diff --git a/src/main/scala/edu/cmu/cs/obsidian/codegen/Util.scala b/src/main/scala/edu/cmu/cs/obsidian/codegen/Util.scala
@@ -96,7 +96,27 @@ object Util {
       */
     def decl_0exp(id: Identifier): VariableDeclaration = VariableDeclaration(Seq((id, None)), None)
 
-
+    /**
+      * shorthand for building the yul expression that declares one variable with a type and no
+      * initial value
+      *
+      * @param id the name of the variable to be declared
+      * @param t the type for id
+      * @return the expression declaring the variable
+      */
+    def decl_0exp_t(id: Identifier, t: ObsidianType): VariableDeclaration =
+        VariableDeclaration(Seq((id, Some(mapObsTypeToABI(t.baseTypeName)))), None)
+    /**
+      * shorthand for building the yul expression that declares one variable with a type and no
+      * initial value
+      *
+      * @param id the name of the variable to be declared
+      * @param t the type for id
+      * @param e the expression of type t to which id will be bound
+      * @return the expression declaring the variable
+      */
+    def decl_0exp_t_init(id: Identifier, t: ObsidianType, e: Expression): VariableDeclaration =
+        VariableDeclaration(Seq((id, Some(mapObsTypeToABI(t.baseTypeName)))), Some(e))
     /**
       * shorthand for building the yul expression that declares a sequence (non-empty) of identifiers
       * with an initial value but no typing information
diff --git a/src/main/scala/edu/cmu/cs/obsidian/codegen/yulAST.scala b/src/main/scala/edu/cmu/cs/obsidian/codegen/yulAST.scala
@@ -102,10 +102,19 @@ case class Assignment(variableNames: Seq[Identifier], value: Expression) extends
 case class VariableDeclaration(variables: Seq[(Identifier, Option[String])], value: Option[Expression]) extends YulStatement {
     override def toString: String = {
         s"let ${
-            variables.map(v => v._1.name + (v._2 match {
-                case Some(t) => s" : $t"
-                case None => ""
-            })).mkString(", ")
+            variables.map(v => v._1.name
+                // todo:
+                // this code correctly adds type annotations to the output Yul, but as of
+                // Version: 0.8.1+commit.df193b15.Linux.g++ of solc, you get errors like
+                // "Error: "bool" is not a valid type (user defined types are not yet supported)."
+                // when you run that code through solc even though the spec says otherwise.
+                /*
+                                + (v._2 match {
+                                    case Some(t) => s" : $t"
+                                    case None => ""
+                                })
+                */
+            ).mkString(", ")
         }" +
             (value match {
                 case Some(e) => s" := ${e.toString}"

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,6 @@`
`1`	`1`	`main contract BoolLiteral{`
`2`		`- bool x;`
`3`		`-`
`4`	`2`	`transaction BoolLiteral () {`
	`3`	`+ bool x;`
`5`	`4`	`x = true;`
`6`	`5`	`x = false;`
`7`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`main contract IntConst{`
`2`		`- int x;`
`3`	`2`	`transaction intconst() {`
	`3`	`+ int x;`
`4`	`4`	`x = 4;`
`5`	`5`	`return;`
`6`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,12 +1,11 @@`
`1`	`1`	`main contract SimpleCall{`
`2`		`- int x;`
`3`		`- transaction updatex() {`
`4`		`- x = 4;`
`5`		`- return;`
	`2`	`+ transaction val() returns int {`
	`3`	`+ return 4;`
`6`	`4`	`}`
`7`		`- transaction main(){`
	`5`	`+ transaction main() returns int{`
	`6`	`+ int x;`
`8`	`7`	`x = 9;`
`9`		`- updatex();`
`10`		`- return;`
	`8`	`+ x = val();`
	`9`	`+ return x;`
`11`	`10`	`}`
`12`	`11`	`}`