From 396b6c2e7c5c368c67e71824471d4f2d48f5c128 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Wed, 23 Dec 2020 16:14:59 -0800 Subject: [PATCH] [dev.regabi] cmd/compile: cleanup assignment typechecking The assignment type-checking code previously bounced around a lot between the LHS and RHS sides of the assignment. But there's actually a very simple, consistent pattern to how to type check assignments: 1. Check the RHS expression. 2. If the LHS expression is an identifier that was declared in this statement and it doesn't have an explicit type, give it the RHS expression's default type. 3. Check the LHS expression. 4. Try assigning the RHS expression to the LHS expression, adding implicit conversions as needed. This CL implements this algorithm, and refactors tcAssign and tcAssignList to use a common implementation. It also fixes the error messages to consistently say just "1 variable" or "1 value", rather than occasionally "1 variables" or "1 values". Fixes #43348. Passes toolstash -cmp. Change-Id: I749cb8d6ccbc7d22cd7cb0a381f58a39fc2696b5 Reviewed-on: https://go-review.googlesource.com/c/go/+/280112 Trust: Matthew Dempsky Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Cuong Manh Le --- src/cmd/compile/internal/typecheck/stmt.go | 245 +++++++----------- .../compile/internal/typecheck/typecheck.go | 5 + test/fixedbugs/issue27595.go | 2 +- test/fixedbugs/issue30087.go | 6 +- test/used.go | 1 + 5 files changed, 110 insertions(+), 149 deletions(-) diff --git a/src/cmd/compile/internal/typecheck/stmt.go b/src/cmd/compile/internal/typecheck/stmt.go index fe9ef400bbd..7e74b730bc1 100644 --- a/src/cmd/compile/internal/typecheck/stmt.go +++ b/src/cmd/compile/internal/typecheck/stmt.go @@ -93,47 +93,16 @@ func tcAssign(n *ir.AssignStmt) { defer tracePrint("typecheckas", n)(nil) } - // delicate little dance. - // the definition of n may refer to this assignment - // as its definition, in which case it will call typecheckas. - // in that case, do not call typecheck back, or it will cycle. - // if the variable has a type (ntype) then typechecking - // will not look at defn, so it is okay (and desirable, - // so that the conversion below happens). - n.X = Resolve(n.X) - - if !ir.DeclaredBy(n.X, n) || n.X.Name().Ntype != nil { + if n.Y == nil { n.X = AssignExpr(n.X) + return } - // Use ctxMultiOK so we can emit an "N variables but M values" error - // to be consistent with typecheckas2 (#26616). - n.Y = typecheck(n.Y, ctxExpr|ctxMultiOK) - checkassign(n, n.X) - if n.Y != nil && n.Y.Type() != nil { - if n.Y.Type().IsFuncArgStruct() { - base.Errorf("assignment mismatch: 1 variable but %v returns %d values", n.Y.(*ir.CallExpr).X, n.Y.Type().NumFields()) - // Multi-value RHS isn't actually valid for OAS; nil out - // to indicate failed typechecking. - n.Y.SetType(nil) - } else if n.X.Type() != nil { - n.Y = AssignConv(n.Y, n.X.Type(), "assignment") - } - } + lhs, rhs := []ir.Node{n.X}, []ir.Node{n.Y} + assign(n, lhs, rhs) + n.X, n.Y = lhs[0], rhs[0] - if ir.DeclaredBy(n.X, n) && n.X.Name().Ntype == nil { - n.Y = DefaultLit(n.Y, nil) - n.X.SetType(n.Y.Type()) - } - - // second half of dance. - // now that right is done, typecheck the left - // just to get it over with. see dance above. - n.SetTypecheck(1) - - if n.X.Typecheck() == 0 { - n.X = AssignExpr(n.X) - } + // TODO(mdempsky): This seems out of place. if !ir.IsBlank(n.X) { types.CheckSize(n.X.Type()) // ensure width is calculated for backend } @@ -144,132 +113,118 @@ func tcAssignList(n *ir.AssignListStmt) { defer tracePrint("typecheckas2", n)(nil) } - ls := n.Lhs - for i1, n1 := range ls { - // delicate little dance. - n1 = Resolve(n1) - ls[i1] = n1 + assign(n, n.Lhs, n.Rhs) +} - if !ir.DeclaredBy(n1, n) || n1.Name().Ntype != nil { - ls[i1] = AssignExpr(ls[i1]) +func assign(stmt ir.Node, lhs, rhs []ir.Node) { + // delicate little dance. + // the definition of lhs may refer to this assignment + // as its definition, in which case it will call typecheckas. + // in that case, do not call typecheck back, or it will cycle. + // if the variable has a type (ntype) then typechecking + // will not look at defn, so it is okay (and desirable, + // so that the conversion below happens). + + checkLHS := func(i int, typ *types.Type) { + lhs[i] = Resolve(lhs[i]) + if n := lhs[i]; typ != nil && ir.DeclaredBy(n, stmt) && n.Name().Ntype == nil { + if typ.Kind() != types.TNIL { + n.SetType(defaultType(typ)) + } else { + base.Errorf("use of untyped nil") + } + } + if lhs[i].Typecheck() == 0 { + lhs[i] = AssignExpr(lhs[i]) + } + checkassign(stmt, lhs[i]) + } + + assignType := func(i int, typ *types.Type) { + checkLHS(i, typ) + if typ != nil { + checkassignto(typ, lhs[i]) } } - cl := len(n.Lhs) - cr := len(n.Rhs) - if cl > 1 && cr == 1 { - n.Rhs[0] = typecheck(n.Rhs[0], ctxExpr|ctxMultiOK) + cr := len(rhs) + if len(rhs) == 1 { + rhs[0] = typecheck(rhs[0], ctxExpr|ctxMultiOK) + if rtyp := rhs[0].Type(); rtyp != nil && rtyp.IsFuncArgStruct() { + cr = rtyp.NumFields() + } } else { - Exprs(n.Rhs) - } - checkassignlist(n, n.Lhs) - - var l ir.Node - var r ir.Node - if cl == cr { - // easy - ls := n.Lhs - rs := n.Rhs - for il, nl := range ls { - nr := rs[il] - if nl.Type() != nil && nr.Type() != nil { - rs[il] = AssignConv(nr, nl.Type(), "assignment") - } - if ir.DeclaredBy(nl, n) && nl.Name().Ntype == nil { - rs[il] = DefaultLit(rs[il], nil) - nl.SetType(rs[il].Type()) - } - } - - goto out - } - - l = n.Lhs[0] - r = n.Rhs[0] - - // x,y,z = f() - if cr == 1 { - if r.Type() == nil { - goto out - } - switch r.Op() { - case ir.OCALLMETH, ir.OCALLINTER, ir.OCALLFUNC: - if !r.Type().IsFuncArgStruct() { - break - } - cr = r.Type().NumFields() - if cr != cl { - goto mismatch - } - r.(*ir.CallExpr).Use = ir.CallUseList - n.SetOp(ir.OAS2FUNC) - for i, l := range n.Lhs { - f := r.Type().Field(i) - if f.Type != nil && l.Type() != nil { - checkassignto(f.Type, l) - } - if ir.DeclaredBy(l, n) && l.Name().Ntype == nil { - l.SetType(f.Type) - } - } - goto out - } + Exprs(rhs) } // x, ok = y - if cl == 2 && cr == 1 { - if r.Type() == nil { - goto out - } +assignOK: + for len(lhs) == 2 && cr == 1 { + stmt := stmt.(*ir.AssignListStmt) + r := rhs[0] + switch r.Op() { - case ir.OINDEXMAP, ir.ORECV, ir.ODOTTYPE: - switch r.Op() { - case ir.OINDEXMAP: - n.SetOp(ir.OAS2MAPR) - case ir.ORECV: - n.SetOp(ir.OAS2RECV) - case ir.ODOTTYPE: - r := r.(*ir.TypeAssertExpr) - n.SetOp(ir.OAS2DOTTYPE) - r.SetOp(ir.ODOTTYPE2) - } - if l.Type() != nil { - checkassignto(r.Type(), l) - } - if ir.DeclaredBy(l, n) { - l.SetType(r.Type()) - } - l := n.Lhs[1] - if l.Type() != nil && !l.Type().IsBoolean() { - checkassignto(types.Types[types.TBOOL], l) - } - if ir.DeclaredBy(l, n) && l.Name().Ntype == nil { - l.SetType(types.Types[types.TBOOL]) - } - goto out + case ir.OINDEXMAP: + stmt.SetOp(ir.OAS2MAPR) + case ir.ORECV: + stmt.SetOp(ir.OAS2RECV) + case ir.ODOTTYPE: + r := r.(*ir.TypeAssertExpr) + stmt.SetOp(ir.OAS2DOTTYPE) + r.SetOp(ir.ODOTTYPE2) + default: + break assignOK } + + assignType(0, r.Type()) + assignType(1, types.UntypedBool) + return } -mismatch: - switch r.Op() { - default: - base.Errorf("assignment mismatch: %d variables but %d values", cl, cr) - case ir.OCALLFUNC, ir.OCALLMETH, ir.OCALLINTER: - r := r.(*ir.CallExpr) - base.Errorf("assignment mismatch: %d variables but %v returns %d values", cl, r.X, cr) + if len(lhs) != cr { + if r, ok := rhs[0].(*ir.CallExpr); ok && len(rhs) == 1 { + if r.Type() != nil { + base.ErrorfAt(stmt.Pos(), "assignment mismatch: %d variable%s but %v returns %d value%s", len(lhs), plural(len(lhs)), r.X, cr, plural(cr)) + } + } else { + base.ErrorfAt(stmt.Pos(), "assignment mismatch: %d variable%s but %v value%s", len(lhs), plural(len(lhs)), len(rhs), plural(len(rhs))) + } + + for i := range lhs { + checkLHS(i, nil) + } + return } - // second half of dance -out: - n.SetTypecheck(1) - ls = n.Lhs - for i1, n1 := range ls { - if n1.Typecheck() == 0 { - ls[i1] = AssignExpr(ls[i1]) + // x,y,z = f() + if cr > len(rhs) { + stmt := stmt.(*ir.AssignListStmt) + stmt.SetOp(ir.OAS2FUNC) + r := rhs[0].(*ir.CallExpr) + r.Use = ir.CallUseList + rtyp := r.Type() + + for i := range lhs { + assignType(i, rtyp.Field(i).Type) + } + return + } + + for i, r := range rhs { + checkLHS(i, r.Type()) + if lhs[i].Type() != nil { + rhs[i] = AssignConv(r, lhs[i].Type(), "assignment") } } } +func plural(n int) string { + if n == 1 { + return "" + } + return "s" +} + // tcFor typechecks an OFOR node. func tcFor(n *ir.ForStmt) ir.Node { Stmts(n.Init()) diff --git a/src/cmd/compile/internal/typecheck/typecheck.go b/src/cmd/compile/internal/typecheck/typecheck.go index 87daee123d4..05a346b8c88 100644 --- a/src/cmd/compile/internal/typecheck/typecheck.go +++ b/src/cmd/compile/internal/typecheck/typecheck.go @@ -1690,6 +1690,11 @@ func checkassignlist(stmt ir.Node, l ir.Nodes) { } func checkassignto(src *types.Type, dst ir.Node) { + // TODO(mdempsky): Handle all untyped types correctly. + if src == types.UntypedBool && dst.Type().IsBoolean() { + return + } + if op, why := assignop(src, dst.Type()); op == ir.OXXX { base.Errorf("cannot assign %v to %L in multiple assignment%s", src, dst, why) return diff --git a/test/fixedbugs/issue27595.go b/test/fixedbugs/issue27595.go index af5c7a10d9b..b9328a68132 100644 --- a/test/fixedbugs/issue27595.go +++ b/test/fixedbugs/issue27595.go @@ -8,7 +8,7 @@ package main var a = twoResults() // ERROR "assignment mismatch: 1 variable but twoResults returns 2 values" var b, c, d = twoResults() // ERROR "assignment mismatch: 3 variables but twoResults returns 2 values" -var e, f = oneResult() // ERROR "assignment mismatch: 2 variables but oneResult returns 1 values" +var e, f = oneResult() // ERROR "assignment mismatch: 2 variables but oneResult returns 1 value" func twoResults() (int, int) { return 1, 2 diff --git a/test/fixedbugs/issue30087.go b/test/fixedbugs/issue30087.go index 3ad9c8c8d90..a8f6202329e 100644 --- a/test/fixedbugs/issue30087.go +++ b/test/fixedbugs/issue30087.go @@ -7,8 +7,8 @@ package main func main() { - var a, b = 1 // ERROR "assignment mismatch: 2 variables but 1 values|wrong number of initializations" - _ = 1, 2 // ERROR "assignment mismatch: 1 variables but 2 values|number of variables does not match" - c, d := 1 // ERROR "assignment mismatch: 2 variables but 1 values|wrong number of initializations" + var a, b = 1 // ERROR "assignment mismatch: 2 variables but 1 value|wrong number of initializations" + _ = 1, 2 // ERROR "assignment mismatch: 1 variable but 2 values|number of variables does not match" + c, d := 1 // ERROR "assignment mismatch: 2 variables but 1 value|wrong number of initializations" e, f := 1, 2, 3 // ERROR "assignment mismatch: 2 variables but 3 values|wrong number of initializations" } diff --git a/test/used.go b/test/used.go index 5c7aad24a6c..76f3fc91ccc 100644 --- a/test/used.go +++ b/test/used.go @@ -63,6 +63,7 @@ func _() { _ = f1() // ok _, _ = f2() // ok _ = f2() // ERROR "assignment mismatch: 1 variable but f2 returns 2 values" + _ = f1(), 0 // ERROR "assignment mismatch: 1 variable but 2 values" T.M0 // ERROR "T.M0 evaluated but not used" t.M0 // ERROR "t.M0 evaluated but not used" cap // ERROR "use of builtin cap not in function call"