Fix pretty-printing to use correct precedence, refactor binops to BinOp

Author: skius

ast/and.go

@@ -18,3 +18,7 @@ func (a Arg) Eval(c *Context) Val {
 func (a Arg) String() string {
 	return "$" + strconv.Itoa(int(a))
 }
+func (a Arg) Precedence() int {
+	// Leaf, not operator
+	return LeafPrecedence
+}

ast/arg.go

@@ -18,3 +18,6 @@ func (a Assn) Eval(c *Context) Val {
 func (a Assn) String() string {
 	return a.V.String() + " = " + a.E.String()
 }
+func (a Assn) Precedence() int {
+	return 0
+}

ast/assn.go

@@ -9,8 +9,12 @@ type Attrib interface{}
 type Expr interface {
 	Eval(*Context) Val
 	String() string
+	Precedence() int
+	//IsRightAssociative() bool // Unnecessary, we only have left-associative expressions currently.
 }
 
+const LeafPrecedence int = 100
+
 type Program struct {
 	Funcs []FuncDecl
 	Code  Block
@@ -40,6 +44,10 @@ func (p Program) String() string {
 	result := strings.Join(funcdecls, "\n")
 	return result + "\n" + p.Code.String()
 }
+func (p Program) Precedence() int {
+	// Unused
+	return -1
+}
 
 type Block []Expr
 
@@ -70,3 +78,7 @@ func (b Block) String() string {
 	}
 	return str
 }
+func (b Block) Precedence() int {
+	// Unused
+	return -1
+}

ast/ast.go

@@ -0,0 +1,154 @@
+package ast
+
+import (
+	"fmt"
+	"strconv"
+)
+
+type Op int
+
+// Op are operators used for BinOp, encoded as their precedence
+const (
+	OrOp Op = (iota + 1) * 10
+	AndOp
+	NotEqualsOp
+	EqualsOp
+	ConcatOp
+)
+
+func (o Op) String() string {
+	switch o {
+	case OrOp:
+		return "||"
+	case AndOp:
+		return "&&"
+	case NotEqualsOp:
+		return "!="
+	case EqualsOp:
+		return "=="
+	case ConcatOp:
+		return "+"
+	}
+	panic("Op not found:" + strconv.Itoa(int(o)))
+}
+
+type BinOp struct {
+	Lhs Expr
+	Rhs Expr
+	Op Op
+}
+
+func NewOr(a, b Attrib) (Expr, error) {
+	return BinOp{
+		Lhs: a.(Expr),
+		Rhs: b.(Expr),
+		Op:  OrOp,
+	}, nil
+}
+func NewAnd(a, b Attrib) (Expr, error) {
+	return BinOp{
+		Lhs: a.(Expr),
+		Rhs: b.(Expr),
+		Op:  AndOp,
+	}, nil
+}
+func NewNotEquals(a, b Attrib) (Expr, error) {
+	return BinOp{
+		Lhs: a.(Expr),
+		Rhs: b.(Expr),
+		Op:  NotEqualsOp,
+	}, nil
+}
+func NewEquals(a, b Attrib) (Expr, error) {
+	return BinOp{
+		Lhs: a.(Expr),
+		Rhs: b.(Expr),
+		Op:  EqualsOp,
+	}, nil
+}
+func NewConcat(a, b Attrib) (Expr, error) {
+	return BinOp{
+		Lhs: a.(Expr),
+		Rhs: b.(Expr),
+		Op:  ConcatOp,
+	}, nil
+}
+
+func (b BinOp) Eval(c *Context) Val {
+	switch b.Op {
+	case OrOp:
+		return b.evalOr(c)
+	case AndOp:
+		return b.evalAnd(c)
+	case NotEqualsOp:
+		return b.evalNotEquals(c)
+	case EqualsOp:
+		return b.evalEquals(c)
+	case ConcatOp:
+		return b.evalConcat(c)
+	}
+	panic("BinOp Op not found")
+}
+func (b BinOp) String() string {
+	lhs := b.Lhs.String()
+	rhs := b.Rhs.String()
+
+	// Example:
+	// 	Lhs: a == b
+	// 	Op: +
+	// 	Rhs: c
+	// We can't just print "Lhs Op Rhs" <=> "a == b + c", because + has higher precedence, which would "capture" only the b:
+	// 	Lhs: a			!! INCORRECT
+	// 	Op: ==			!! INCORRECT
+	// 	Rhs: b + c      !! INCORRECT
+	// Hence we must add parentheses around Lhs
+
+	// For Lhs we need '>', because same precedence operators (i.e. the same operator) can be chained:
+	// 	a + b + c <=> (a + b) + c  !! Parentheses unnecessary
+	if b.Precedence() > b.Lhs.Precedence() {
+		lhs = "(" + lhs + ")"
+	}
+	// However, because we only have left-associative operators, for the Rhs we must treat the case of same precedence differently,
+	// i.e. a + (b + c) may not be printed as a + b + c, because that is equal to (a + b) + c =/= a + (b + c)
+	// (Note: If we had different associativity, see here: https://stackoverflow.com/questions/6277747/pretty-print-expression-with-as-few-parentheses-as-possible)
+	if b.Precedence() >= b.Rhs.Precedence() {
+		rhs = "(" + rhs + ")"
+	}
+	return fmt.Sprintf("%v %v %v", lhs, b.Op.String(), rhs)
+}
+func (b BinOp) Precedence() int {
+	return int(b.Op)
+}
+
+
+func (b BinOp) evalOr(c *Context) Val {
+	if BoolOf(b.Lhs.Eval(c)) || BoolOf(b.Rhs.Eval(c)) {
+		return Val("true")
+	} else {
+		return Val("false")
+	}
+}
+func (b BinOp) evalAnd(c *Context) Val {
+	if BoolOf(b.Lhs.Eval(c)) && BoolOf(b.Rhs.Eval(c)) {
+		return Val("true")
+	} else {
+		return Val("false")
+	}
+}
+func (b BinOp) evalNotEquals(c *Context) Val {
+	val := "false"
+	if b.Lhs.Eval(c) != b.Rhs.Eval(c) {
+		val = "true"
+	}
+	return Val(val)
+}
+func (b BinOp) evalEquals(c *Context) Val {
+	val := "false"
+	if b.Lhs.Eval(c) == b.Rhs.Eval(c) {
+		val = "true"
+	}
+	return Val(val)
+}
+func (b BinOp) evalConcat(c *Context) Val {
+	return b.Lhs.Eval(c) + b.Rhs.Eval(c)
+}

ast/binop.go

@@ -49,19 +49,27 @@ func (ca Call) Eval(c *Context) Val {
 	fnAst, err := c.parseFn([]byte(fnSource))
 	if err != nil {
 		fmt.Println("Error occurred:", err)
+		fmt.Println("Fn is:")
+		fmt.Println(ca.Fn)
+		fmt.Println("source is:")
+		fmt.Println(string(fnSource))
 		return ""
 	}
 	fnProg := fnAst.(Program)
 	if len(fnProg.Code) != 1 {
 		// Must consist of exactly one lambda
-		fmt.Println("Length of parsed program is not equal to 1")
+		fmt.Println("Length of parsed program is not equal to 1, Fn expression is:")
+		fmt.Println(ca.Fn)
+		fmt.Println("source is:")
+		fmt.Println(string(fnSource))
 		return ""
 	}
 	fst := fnProg.Code[0]
 	lam, ok := fst.(Lambda)
 	if !ok {
 		// Must be lambda
-		fmt.Println("Parsed program is not a Lambda")
+		fmt.Println("Parsed program is not a Lambda:")
+		fmt.Println(string(fnSource))
 		return ""
 	}
 
@@ -80,7 +88,16 @@ func (ca Call) String() string {
 		args = append(args, arg.String())
 	}
 
-	return ca.Fn.String() + "(" + strings.Join(args, ", ") + ")"
+	fnStr := ca.Fn.String()
+	if ca.Fn.Precedence() < LeafPrecedence {
+		fnStr = "(" + fnStr + ")"
+	}
+
+	return fnStr + "(" + strings.Join(args, ", ") + ")"
+}
+func (ca Call) Precedence() int {
+	// Leaf, not operator
+	return LeafPrecedence
 }
 
 // CallArgs is not an Expr, since it can never appear on its own

ast/call.go

@@ -38,3 +38,14 @@ func (c *Context) GetExitChannel() chan int {
 	}
 	return c.exitChannel
 }
+
+func (c *Context) FuncNames() Set {
+	names := make(Set, len(c.FunctionMap) + len(c.UserFunctionMap))
+	for fId := range c.UserFunctionMap {
+		names.Add(fId)
+	}
+	for fId := range c.FunctionMap {
+		names.Add(fId)
+	}
+	return names
+}

ast/concat.go

@@ -31,3 +31,7 @@ func (e IfElse) String() string {
 	str := "if (" + e.Cond.String() + ") {\n\t" + thenStr + "\n} else {\n\t" + elseStr + "\n}"
 	return str
 }
+func (e IfElse) Precedence() int {
+	// Leaf, not operator
+	return LeafPrecedence
+}

ast/context.go

@@ -25,5 +25,13 @@ func (i Index) Eval(c *Context) Val {
 	return Val(src[idx])
 }
 func (i Index) String() string {
-	return i.Source.String() + "[" + i.I.String() + "]"
+	srcStr := i.Source.String()
+	if i.Source.Precedence() < LeafPrecedence {
+		srcStr = "(" + srcStr + ")"
+	}
+	return srcStr + "[" + i.I.String() + "]"
+}
+func (i Index) Precedence() int {
+	// Leaf, not operator
+	return LeafPrecedence
 }

ast/equals.go

@@ -17,8 +17,8 @@ func NewLambda(ps, b Attrib) (Expr, error) {
 }
 
 func (l Lambda) Eval(c *Context) Val {
-	// Closure by value, copy the value of l's body's free vars in the current context into its body
-	fvMap := FreeVars(l)
+	// Closure by value, copy the value of l's body's var that are used before defined in the current context into its body
+	fvMap := UsedBeforeDefVars(l, c.FuncNames())
 
 	// Need to sort as slice because key traversal in maps is non-deterministic
 	fv := make([]string, 0, len(fvMap))
@@ -46,6 +46,10 @@ func (l Lambda) String() string {
 	return res
 }
 
+func (l Lambda) Precedence() int {
+	return LeafPrecedence
+}
+
 func (l Lambda) Call(c *Context, args []Val) Val {
 	// Construct corresponding FuncDecl and call that instead
 	fDecl := FuncDecl{