Skip to content

Commit

Permalink
cmd/compile: rewrite f(g()) for multi-value g() during typecheck
Browse files Browse the repository at this point in the history
This CL moves order.go's copyRet logic for rewriting f(g()) into t1,
t2, ... = g(); f(t1, t2, ...) earlier into typecheck. This allows the
rest of the compiler to stop worrying about multi-value functions
appearing outside of OAS2FUNC nodes.

This changes compiler behavior in a few observable ways:

1. Typechecking error messages for builtin functions now use general
case error messages rather than unnecessarily differing ones.

2. Because f(g()) is rewritten before inlining, saved inline bodies
now see the rewritten form too. This could be addressed, but doesn't
seem worthwhile.

3. Most notably, this simplifies escape analysis and fixes a memory
corruption issue in esc.go. See #29197 for details.

Fixes #15992.
Fixes #29197.

Change-Id: I86a70668301efeec8fbd11fe2d242e359a3ad0af
Reviewed-on: https://go-review.googlesource.com/c/153841
Reviewed-by: Robert Griesemer <[email protected]>
  • Loading branch information
mdempsky committed Feb 28, 2019
1 parent 9d40fad commit d96b7fb
Show file tree
Hide file tree
Showing 13 changed files with 157 additions and 267 deletions.
7 changes: 0 additions & 7 deletions src/cmd/compile/internal/gc/esc.go
Original file line number Diff line number Diff line change
Expand Up @@ -1604,13 +1604,6 @@ func (e *EscState) esccall(call *Node, parent *Node) {
}

argList := call.List
if argList.Len() == 1 {
arg := argList.First()
if arg.Type.IsFuncArgStruct() { // f(g())
argList = e.nodeEscState(arg).Retval
}
}

args := argList.Slice()

if indirect {
Expand Down
13 changes: 6 additions & 7 deletions src/cmd/compile/internal/gc/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -1404,14 +1404,11 @@ func (n *Node) exprfmt(s fmt.State, prec int, mode fmtMode) {
}
mode.Fprintf(s, "sliceheader{%v,%v,%v}", n.Left, n.List.First(), n.List.Second())

case OCOPY:
mode.Fprintf(s, "%#v(%v, %v)", n.Op, n.Left, n.Right)

case OCOMPLEX:
if n.List.Len() == 1 {
mode.Fprintf(s, "%#v(%v)", n.Op, n.List.First())
} else {
case OCOMPLEX, OCOPY:
if n.Left != nil {
mode.Fprintf(s, "%#v(%v, %v)", n.Op, n.Left, n.Right)
} else {
mode.Fprintf(s, "%#v(%.v)", n.Op, n.List)
}

case OCONV,
Expand Down Expand Up @@ -1540,6 +1537,8 @@ func (n *Node) nodefmt(s fmt.State, flag FmtFlag, mode fmtMode) {
if flag&FmtLong != 0 && t != nil {
if t.Etype == TNIL {
fmt.Fprint(s, "nil")
} else if n.Op == ONAME && n.Name.AutoTemp() {
mode.Fprintf(s, "%v value", t)
} else {
mode.Fprintf(s, "%v (type %v)", n, t)
}
Expand Down
3 changes: 2 additions & 1 deletion src/cmd/compile/internal/gc/iexport.go
Original file line number Diff line number Diff line change
Expand Up @@ -1387,7 +1387,8 @@ func (w *exportWriter) localIdent(s *types.Sym, v int32) {
return
}

if i := strings.LastIndex(name, "."); i >= 0 {
// TODO(mdempsky): Fix autotmp hack.
if i := strings.LastIndex(name, "."); i >= 0 && !strings.HasPrefix(name, ".autotmp_") {
Fatalf("unexpected dot in identifier: %v", name)
}

Expand Down
9 changes: 9 additions & 0 deletions src/cmd/compile/internal/gc/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import (
// the name, normally "pkg.init", is altered to "pkg.init.0".
var renameinitgen int

// Dummy function for autotmps generated during typechecking.
var dummyInitFn = nod(ODCLFUNC, nil, nil)

func renameinit() *types.Sym {
s := lookupN("init.", renameinitgen)
renameinitgen++
Expand Down Expand Up @@ -114,6 +117,12 @@ func fninit(n []*Node) {
initsym := lookup("init")
fn := dclfunc(initsym, nod(OTFUNC, nil, nil))

for _, dcl := range dummyInitFn.Func.Dcl {
dcl.Name.Curfn = fn
}
fn.Func.Dcl = append(fn.Func.Dcl, dummyInitFn.Func.Dcl...)
dummyInitFn = nil

// (3)
a := nod(OIF, nil, nil)
a.Left = nod(OGT, gatevar, nodintconst(1))
Expand Down
18 changes: 2 additions & 16 deletions src/cmd/compile/internal/gc/inl.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,24 +589,13 @@ func inlnode(n *Node, maxCost int32) *Node {
}

inlnodelist(n.List, maxCost)
switch n.Op {
case OBLOCK:
if n.Op == OBLOCK {
for _, n2 := range n.List.Slice() {
if n2.Op == OINLCALL {
inlconv2stmt(n2)
}
}

case ORETURN, OCALLFUNC, OCALLMETH, OCALLINTER, OAPPEND, OCOMPLEX:
// if we just replaced arg in f(arg()) or return arg with an inlined call
// and arg returns multiple values, glue as list
if n.List.Len() == 1 && n.List.First().Op == OINLCALL && n.List.First().Rlist.Len() > 1 {
n.List.Set(inlconv2list(n.List.First()))
break
}
fallthrough

default:
} else {
s := n.List.Slice()
for i1, n1 := range s {
if n1 != nil && n1.Op == OINLCALL {
Expand Down Expand Up @@ -1016,9 +1005,6 @@ func mkinlcall(n, fn *Node, maxCost int32) *Node {
// to pass as a slice.

numvals := n.List.Len()
if numvals == 1 && n.List.First().Type.IsFuncArgStruct() {
numvals = n.List.First().Type.NumFields()
}

x := as.List.Len()
for as.List.Len() < numvals {
Expand Down
60 changes: 3 additions & 57 deletions src/cmd/compile/internal/gc/order.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,66 +380,12 @@ func (o *Order) init(n *Node) {
n.Ninit.Set(nil)
}

// Ismulticall reports whether the list l is f() for a multi-value function.
// Such an f() could appear as the lone argument to a multi-arg function.
func ismulticall(l Nodes) bool {
// one arg only
if l.Len() != 1 {
return false
}
n := l.First()

// must be call
switch n.Op {
default:
return false
case OCALLFUNC, OCALLMETH, OCALLINTER:
// call must return multiple values
return n.Left.Type.NumResults() > 1
}
}

// copyRet emits t1, t2, ... = n, where n is a function call,
// and then returns the list t1, t2, ....
func (o *Order) copyRet(n *Node) []*Node {
if !n.Type.IsFuncArgStruct() {
Fatalf("copyret %v %d", n.Type, n.Left.Type.NumResults())
}

slice := n.Type.Fields().Slice()
l1 := make([]*Node, len(slice))
l2 := make([]*Node, len(slice))
for i, t := range slice {
tmp := temp(t.Type)
l1[i] = tmp
l2[i] = tmp
}

as := nod(OAS2, nil, nil)
as.List.Set(l1)
as.Rlist.Set1(n)
as = typecheck(as, ctxStmt)
o.stmt(as)

return l2
}

// callArgs orders the list of call arguments *l.
func (o *Order) callArgs(l *Nodes) {
if ismulticall(*l) {
// return f() where f() is multiple values.
l.Set(o.copyRet(l.First()))
} else {
o.exprList(*l)
}
}

// call orders the call expression n.
// n.Op is OCALLMETH/OCALLFUNC/OCALLINTER or a builtin like OCOPY.
func (o *Order) call(n *Node) {
n.Left = o.expr(n.Left, nil)
n.Right = o.expr(n.Right, nil) // ODDDARG temp
o.callArgs(&n.List)
o.exprList(n.List)

if n.Op != OCALLFUNC {
return
Expand Down Expand Up @@ -811,7 +757,7 @@ func (o *Order) stmt(n *Node) {
o.cleanTemp(t)

case ORETURN:
o.callArgs(&n.List)
o.exprList(n.List)
o.out = append(o.out, n)

// Special: clean case temporaries in each block entry.
Expand Down Expand Up @@ -1174,7 +1120,7 @@ func (o *Order) expr(n, lhs *Node) *Node {
n.List.SetFirst(o.expr(n.List.First(), nil)) // order x
n.List.Second().Left = o.expr(n.List.Second().Left, nil) // order y
} else {
o.callArgs(&n.List)
o.exprList(n.List)
}

if lhs == nil || lhs.Op != ONAME && !samesafeexpr(lhs, n.List.First()) {
Expand Down
Loading

0 comments on commit d96b7fb

Please sign in to comment.