[dev.regabi] cmd/compile: refactoring typecheck arith

Currently, the tcArith logic is complicated and involes many
un-necessary checks for some ir.Op. This CL refactors how it works:

 - Add a new tcShiftOp function, which only does necessary works for
   typechecking OLSH/ORSH. That ends up moving OLSH/ORSH to a separated
   case in typecheck1.

 - Move OASOP to separated case, so its logic is detached from tcArith.

 - Move OANDAND/OOROR to separated case, which does some validation
   dedicated to logical operators only.

Passes toolstash -cmp.

Change-Id: I0db7b7c7a3e52d6f9e9d87eee6967871f1c32200
Reviewed-on: https://go-review.googlesource.com/c/go/+/279442
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

1 files changed, 43 insertions, 143 deletions

diff --git a/src/cmd/compile/internal/typecheck/expr.go b/src/cmd/compile/internal/typecheck/expr.go
index 29d7a08011..f3e3a93150 100644
--- a/src/cmd/compile/internal/typecheck/expr.go
+++ b/src/cmd/compile/internal/typecheck/expr.go
@@ -55,103 +55,50 @@ func tcAddr(n *ir.AddrExpr) ir.Node {
 	return n
 }
 
-// tcArith typechecks a binary arithmetic expression.
-func tcArith(n ir.Node) ir.Node {
-	var l, r ir.Node
-	var setLR func()
-	switch n := n.(type) {
-	case *ir.AssignOpStmt:
-		l, r = n.X, n.Y
-		setLR = func() { n.X = l; n.Y = r }
-	case *ir.BinaryExpr:
-		l, r = n.X, n.Y
-		setLR = func() { n.X = l; n.Y = r }
-	case *ir.LogicalExpr:
-		l, r = n.X, n.Y
-		setLR = func() { n.X = l; n.Y = r }
-	}
-	l = Expr(l)
-	r = Expr(r)
-	setLR()
-	if l.Type() == nil || r.Type() == nil {
-		n.SetType(nil)
-		return n
+func tcShift(n, l, r ir.Node) (ir.Node, ir.Node, *types.Type) {
+	if l.Type() == nil || l.Type() == nil {
+		return l, r, nil
 	}
-	op := n.Op()
-	if n.Op() == ir.OASOP {
-		n := n.(*ir.AssignOpStmt)
-		checkassign(n, l)
-		if n.IncDec && !okforarith[l.Type().Kind()] {
-			base.Errorf("invalid operation: %v (non-numeric type %v)", n, l.Type())
-			n.SetType(nil)
-			return n
-		}
-		// TODO(marvin): Fix Node.EType type union.
-		op = n.AsOp
-	}
-	if op == ir.OLSH || op == ir.ORSH {
-		r = DefaultLit(r, types.Types[types.TUINT])
-		setLR()
-		t := r.Type()
-		if !t.IsInteger() {
-			base.Errorf("invalid operation: %v (shift count type %v, must be integer)", n, r.Type())
-			n.SetType(nil)
-			return n
-		}
-		if t.IsSigned() && !types.AllowsGoVersion(curpkg(), 1, 13) {
-			base.ErrorfVers("go1.13", "invalid operation: %v (signed shift count type %v)", n, r.Type())
-			n.SetType(nil)
-			return n
-		}
-		t = l.Type()
-		if t != nil && t.Kind() != types.TIDEAL && !t.IsInteger() {
-			base.Errorf("invalid operation: %v (shift of type %v)", n, t)
-			n.SetType(nil)
-			return n
-		}
 
-		// no defaultlit for left
-		// the outer context gives the type
-		n.SetType(l.Type())
-		if (l.Type() == types.UntypedFloat || l.Type() == types.UntypedComplex) && r.Op() == ir.OLITERAL {
-			n.SetType(types.UntypedInt)
-		}
-		return n
+	r = DefaultLit(r, types.Types[types.TUINT])
+	t := r.Type()
+	if !t.IsInteger() {
+		base.Errorf("invalid operation: %v (shift count type %v, must be integer)", n, r.Type())
+		return l, r, nil
+	}
+	if t.IsSigned() && !types.AllowsGoVersion(curpkg(), 1, 13) {
+		base.ErrorfVers("go1.13", "invalid operation: %v (signed shift count type %v)", n, r.Type())
+		return l, r, nil
+	}
+	t = l.Type()
+	if t != nil && t.Kind() != types.TIDEAL && !t.IsInteger() {
+		base.Errorf("invalid operation: %v (shift of type %v)", n, t)
+		return l, r, nil
 	}
 
-	// For "x == x && len(s)", it's better to report that "len(s)" (type int)
-	// can't be used with "&&" than to report that "x == x" (type untyped bool)
-	// can't be converted to int (see issue #41500).
-	if n.Op() == ir.OANDAND || n.Op() == ir.OOROR {
-		n := n.(*ir.LogicalExpr)
-		if !n.X.Type().IsBoolean() {
-			base.Errorf("invalid operation: %v (operator %v not defined on %s)", n, n.Op(), typekind(n.X.Type()))
-			n.SetType(nil)
-			return n
-		}
-		if !n.Y.Type().IsBoolean() {
-			base.Errorf("invalid operation: %v (operator %v not defined on %s)", n, n.Op(), typekind(n.Y.Type()))
-			n.SetType(nil)
-			return n
-		}
+	// no defaultlit for left
+	// the outer context gives the type
+	t = l.Type()
+	if (l.Type() == types.UntypedFloat || l.Type() == types.UntypedComplex) && r.Op() == ir.OLITERAL {
+		t = types.UntypedInt
 	}
+	return l, r, t
+}
 
-	// ideal mixed with non-ideal
+// tcArith typechecks operands of a binary arithmetic expression.
+// The result of tcArith MUST be assigned back to original operands,
+// t is the type of the expression, and should be set by the caller. e.g:
+//     n.X, n.Y, t = tcArith(n, op, n.X, n.Y)
+//     n.SetType(t)
+func tcArith(n ir.Node, op ir.Op, l, r ir.Node) (ir.Node, ir.Node, *types.Type) {
 	l, r = defaultlit2(l, r, false)
-	setLR()
-
 	if l.Type() == nil || r.Type() == nil {
-		n.SetType(nil)
-		return n
+		return l, r, nil
 	}
 	t := l.Type()
 	if t.Kind() == types.TIDEAL {
 		t = r.Type()
 	}
-	et := t.Kind()
-	if et == types.TIDEAL {
-		et = types.TINT
-	}
 	aop := ir.OXXX
 	if iscmp[n.Op()] && t.Kind() != types.TIDEAL && !types.Identical(l.Type(), r.Type()) {
 		// comparison is okay as long as one side is
@@ -167,15 +114,13 @@ func tcArith(n ir.Node) ir.Node {
 			if aop != ir.OXXX {
 				if r.Type().IsInterface() && !l.Type().IsInterface() && !types.IsComparable(l.Type()) {
 					base.Errorf("invalid operation: %v (operator %v not defined on %s)", n, op, typekind(l.Type()))
-					n.SetType(nil)
-					return n
+					return l, r, nil
 				}
 
 				types.CalcSize(l.Type())
 				if r.Type().IsInterface() == l.Type().IsInterface() || l.Type().Width >= 1<<16 {
 					l = ir.NewConvExpr(base.Pos, aop, r.Type(), l)
 					l.SetTypecheck(1)
-					setLR()
 				}
 
 				t = r.Type()
@@ -188,34 +133,28 @@ func tcArith(n ir.Node) ir.Node {
 			if aop != ir.OXXX {
 				if l.Type().IsInterface() && !r.Type().IsInterface() && !types.IsComparable(r.Type()) {
 					base.Errorf("invalid operation: %v (operator %v not defined on %s)", n, op, typekind(r.Type()))
-					n.SetType(nil)
-					return n
+					return l, r, nil
 				}
 
 				types.CalcSize(r.Type())
 				if r.Type().IsInterface() == l.Type().IsInterface() || r.Type().Width >= 1<<16 {
 					r = ir.NewConvExpr(base.Pos, aop, l.Type(), r)
 					r.SetTypecheck(1)
-					setLR()
 				}
 
 				t = l.Type()
 			}
 		}
-
-		et = t.Kind()
 	}
 
 	if t.Kind() != types.TIDEAL && !types.Identical(l.Type(), r.Type()) {
 		l, r = defaultlit2(l, r, true)
 		if l.Type() == nil || r.Type() == nil {
-			n.SetType(nil)
-			return n
+			return l, r, nil
 		}
 		if l.Type().IsInterface() == r.Type().IsInterface() || aop == 0 {
 			base.Errorf("invalid operation: %v (mismatched types %v and %v)", n, l.Type(), r.Type())
-			n.SetType(nil)
-			return n
+			return l, r, nil
 		}
 	}
 
@@ -224,85 +163,46 @@ func tcArith(n ir.Node) ir.Node {
 	}
 	if dt := defaultType(t); !okfor[op][dt.Kind()] {
 		base.Errorf("invalid operation: %v (operator %v not defined on %s)", n, op, typekind(t))
-		n.SetType(nil)
-		return n
+		return l, r, nil
 	}
 
 	// okfor allows any array == array, map == map, func == func.
 	// restrict to slice/map/func == nil and nil == slice/map/func.
 	if l.Type().IsArray() && !types.IsComparable(l.Type()) {
 		base.Errorf("invalid operation: %v (%v cannot be compared)", n, l.Type())
-		n.SetType(nil)
-		return n
+		return l, r, nil
 	}
 
 	if l.Type().IsSlice() && !ir.IsNil(l) && !ir.IsNil(r) {
 		base.Errorf("invalid operation: %v (slice can only be compared to nil)", n)
-		n.SetType(nil)
-		return n
+		return l, r, nil
 	}
 
 	if l.Type().IsMap() && !ir.IsNil(l) && !ir.IsNil(r) {
 		base.Errorf("invalid operation: %v (map can only be compared to nil)", n)
-		n.SetType(nil)
-		return n
+		return l, r, nil
 	}
 
 	if l.Type().Kind() == types.TFUNC && !ir.IsNil(l) && !ir.IsNil(r) {
 		base.Errorf("invalid operation: %v (func can only be compared to nil)", n)
-		n.SetType(nil)
-		return n
+		return l, r, nil
 	}
 
 	if l.Type().IsStruct() {
 		if f := types.IncomparableField(l.Type()); f != nil {
 			base.Errorf("invalid operation: %v (struct containing %v cannot be compared)", n, f.Type)
-			n.SetType(nil)
-			return n
+			return l, r, nil
 		}
 	}
 
-	if iscmp[n.Op()] {
-		t = types.UntypedBool
-		n.SetType(t)
-		if con := EvalConst(n); con.Op() == ir.OLITERAL {
-			return con
-		}
-		l, r = defaultlit2(l, r, true)
-		setLR()
-		return n
-	}
-
-	if et == types.TSTRING && n.Op() == ir.OADD {
-		// create or update OADDSTR node with list of strings in x + y + z + (w + v) + ...
-		n := n.(*ir.BinaryExpr)
-		var add *ir.AddStringExpr
-		if l.Op() == ir.OADDSTR {
-			add = l.(*ir.AddStringExpr)
-			add.SetPos(n.Pos())
-		} else {
-			add = ir.NewAddStringExpr(n.Pos(), []ir.Node{l})
-		}
-		if r.Op() == ir.OADDSTR {
-			r := r.(*ir.AddStringExpr)
-			add.List.Append(r.List.Take()...)
-		} else {
-			add.List.Append(r)
-		}
-		add.SetType(t)
-		return add
-	}
-
 	if (op == ir.ODIV || op == ir.OMOD) && ir.IsConst(r, constant.Int) {
 		if constant.Sign(r.Val()) == 0 {
 			base.Errorf("division by zero")
-			n.SetType(nil)
-			return n
+			return l, r, nil
 		}
 	}
 
-	n.SetType(t)
-	return n
+	return l, r, t
 }
 
 // The result of tcCompLit MUST be assigned back to n, e.g.