]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.regabi] cmd/compile: refactoring typecheck arith
authorCuong Manh Le <cuong.manhle.vn@gmail.com>
Tue, 29 Dec 2020 05:09:51 +0000 (12:09 +0700)
committerCuong Manh Le <cuong.manhle.vn@gmail.com>
Tue, 29 Dec 2020 07:55:55 +0000 (07:55 +0000)
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>
src/cmd/compile/internal/typecheck/expr.go
src/cmd/compile/internal/typecheck/typecheck.go

index 29d7a080114d71ab1d1887b23efe7cde531b6e85..f3e3a93150cb324c7cb0ed1777866cb6f5967e4b 100644 (file)
@@ -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.
index ff9178b5972f718fb70bcd963fb76aa94e9ff2df..e29d58cefa7dc39b26b62da3c2f357d00e93e9e8 100644 (file)
@@ -672,28 +672,98 @@ func typecheck1(n ir.Node, top int) ir.Node {
        case ir.ODEREF:
                n := n.(*ir.StarExpr)
                return tcStar(n, top)
-       // arithmetic exprs
-       case ir.OASOP,
-               ir.OADD,
-               ir.OAND,
-               ir.OANDAND,
-               ir.OANDNOT,
-               ir.ODIV,
-               ir.OEQ,
-               ir.OGE,
-               ir.OGT,
-               ir.OLE,
-               ir.OLT,
-               ir.OLSH,
-               ir.ORSH,
-               ir.OMOD,
-               ir.OMUL,
-               ir.ONE,
-               ir.OOR,
-               ir.OOROR,
-               ir.OSUB,
-               ir.OXOR:
-               return tcArith(n)
+
+       // x op= y
+       case ir.OASOP:
+               n := n.(*ir.AssignOpStmt)
+               n.X, n.Y = Expr(n.X), Expr(n.Y)
+               checkassign(n, n.X)
+               if n.IncDec && !okforarith[n.X.Type().Kind()] {
+                       base.Errorf("invalid operation: %v (non-numeric type %v)", n, n.X.Type())
+                       return n
+               }
+               switch n.AsOp {
+               case ir.OLSH, ir.ORSH:
+                       n.X, n.Y, _ = tcShift(n, n.X, n.Y)
+               case ir.OADD, ir.OAND, ir.OANDNOT, ir.ODIV, ir.OMOD, ir.OMUL, ir.OOR, ir.OSUB, ir.OXOR:
+                       n.X, n.Y, _ = tcArith(n, n.AsOp, n.X, n.Y)
+               default:
+                       base.Fatalf("invalid assign op: %v", n.AsOp)
+               }
+               return n
+
+       // logical operators
+       case ir.OANDAND, ir.OOROR:
+               n := n.(*ir.LogicalExpr)
+               n.X, n.Y = Expr(n.X), Expr(n.Y)
+               // 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.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
+               }
+               l, r, t := tcArith(n, n.Op(), n.X, n.Y)
+               n.X, n.Y = l, r
+               n.SetType(t)
+               return n
+
+       // shift operators
+       case ir.OLSH, ir.ORSH:
+               n := n.(*ir.BinaryExpr)
+               n.X, n.Y = Expr(n.X), Expr(n.Y)
+               l, r, t := tcShift(n, n.X, n.Y)
+               n.X, n.Y = l, r
+               n.SetType(t)
+               return n
+
+       // comparison operators
+       case ir.OEQ, ir.OGE, ir.OGT, ir.OLE, ir.OLT, ir.ONE:
+               n := n.(*ir.BinaryExpr)
+               n.X, n.Y = Expr(n.X), Expr(n.Y)
+               l, r, t := tcArith(n, n.Op(), n.X, n.Y)
+               if t != nil {
+                       n.X, n.Y = l, r
+                       n.SetType(types.UntypedBool)
+                       if con := EvalConst(n); con.Op() == ir.OLITERAL {
+                               return con
+                       }
+                       n.X, n.Y = defaultlit2(l, r, true)
+               }
+               return n
+
+       // binary operators
+       case ir.OADD, ir.OAND, ir.OANDNOT, ir.ODIV, ir.OMOD, ir.OMUL, ir.OOR, ir.OSUB, ir.OXOR:
+               n := n.(*ir.BinaryExpr)
+               n.X, n.Y = Expr(n.X), Expr(n.Y)
+               l, r, t := tcArith(n, n.Op(), n.X, n.Y)
+               if t != nil && t.Kind() == types.TSTRING && n.Op() == ir.OADD {
+                       // create or update OADDSTR node with list of strings in x + y + z + (w + v) + ...
+                       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
+               }
+               n.X, n.Y = l, r
+               n.SetType(t)
+               return n
 
        case ir.OBITNOT, ir.ONEG, ir.ONOT, ir.OPLUS:
                n := n.(*ir.UnaryExpr)