[release-branch.r59] gc: fix closure bug

««« CL 4709042 / d30305e2898a
gc: fix closure bug

Fixes #2056.

R=rsc
CC=golang-dev
https://golang.org/cl/4709042
»»»

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/4814061

doc/devel/release.html

@@ -14,6 +14,113 @@ hg pull
 hg update release.r<i>NN</i>
 </pre>
 
+<h2 id="r59">r59 (released 2011/08/01)</h2>
+
+<p>
+The r59 release corresponds to 
+<code><a href="weekly.html#2011-07-07">weekly.2011-07-07</a></code>.
+This section highlights the most significant changes in this release.
+For a more detailed summary, see the
+<a href="weekly.html#2011-07-07">weekly release notes</a>.
+For complete information, see the
+<a href="http://code.google.com/p/go/source/list?r=release-branch.r59">Mercurial change list</a>.
+</p>
+
+<h3 id="r59.lang">Language</h3>
+
+<p>
+This release includes a language change that restricts the use of
+<code>goto</code>.  In essence, a <code>goto</code> statement outside a block
+cannot jump to a label inside that block. Your code may require changes if it
+uses <code>goto</code>.
+See <a href="http://code.google.com/p/go/source/detail?r=dc6d3cf9279d">this
+changeset</a> for how the new rule affected the Go tree.
+</p>
+
+<h3 id="r59.pkg">Packages</h3>
+
+<p>
+As usual, <a href="/cmd/gofix/">gofix</a> will handle the bulk of the rewrites
+necessary for these changes to package APIs.
+</p>
+
+<p>
+<a href="/pkg/http">Package http</a> has a new
+<a href="/pkg/http/#FileSystem">FileSystem</a> interface that provides access
+to files. The <a href="/pkg/http/#FileServer">FileServer</a> helper now takes a
+<code>FileSystem</code> argument instead of an explicit file system root. By
+implementing your own <code>FileSystem</code> you can use the
+<code>FileServer</code> to serve arbitrary data.
+</p>
+
+<p>
+<a href="/pkg/os/">Package os</a>'s <code>ErrorString</code> type has been
+hidden. Most uses of <code>os.ErrorString</code> can be replaced with
+<a href="/pkg/os/#NewError">os.NewError</a>.
+</p>
+
+<p>
+<a href="/pkg/reflect/">Package reflect</a> supports a new struct tag scheme
+that enables sharing of struct tags between multiple packages.
+In this scheme, the tags must be of the form:
+</p>
+<pre>
+	`key:"value" key2:"value2"`
+</pre>
+<p>
+The <a href="/pkg/reflect/#StructField">StructField</a> type's Tag field now
+has type <a href="/pkg/reflect/#StructTag">StructTag</a>, which has a
+<code>Get</code> method. Clients of <a href="/pkg/json">json</a> and
+<a href="/pkg/xml">xml</a> will need to be updated. Code that says
+</p>
+<pre>
+	type T struct {
+		X int "name"
+	}
+</pre>
+<p>
+should become
+</p>
+<pre>
+	type T struct {
+		X int `json:"name"`  // or `xml:"name"`
+	}
+</pre>
+<p>
+Use <a href="/cmd/govet/">govet</a> to identify struct tags that need to be
+changed to use the new syntax.
+</p>
+
+<p>
+<a href="/pkg/sort/">Package sort</a>'s <code>IntArray</code> type has been
+renamed to <a href="/pkg/sort/#IntSlice">IntSlice</a>, and similarly for
+<a href="/pkg/sort/#Float64Slice">Float64Slice</a> and
+<a href="/pkg/sort/#StringSlice">StringSlice</a>.
+</p>
+
+<p>
+<a href="/pkg/strings/">Package strings</a>'s <code>Split</code> function has
+itself been split into <a href="/pkg/strings/#Split">Split</a> and
+<a href="/pkg/strings/#SplitN">SplitN</a>.
+<code>SplitN</code> is the same as the old <code>Split</code>.
+The new <code>Split</code> is equivalent to <code>SplitN</code> with a final
+argument of -1.
+</p>
+
+<a href="/pkg/image/draw/">Package image/draw</a>'s
+<a href="/pkg/image/draw/#Draw">Draw</a> function now takes an additional
+argument, a compositing operator.
+If in doubt, use <a href="/pkg/image/draw/#Op">draw.Over</a>.
+</p>
+
+<h3 id="r59.cmd">Tools</h3>
+
+<p>
+<a href="/cmd/goinstall/">Goinstall</a> now installs packages and commands from
+arbitrary remote repositories (not just Google Code, Github, and so on).
+See the <a href="/cmd/goinstall/">goinstall documentation</a> for details.
+</p>
+
 <h2 id="r58">r58 (released 2011/06/29)</h2>
 
 <p>
@@ -86,6 +193,14 @@ the Go tree (and avoid writing Makefiles).
 </p>
 
 
+<h3 id="r58.minor">Minor revisions</h3>
+
+<p>r58.1 adds 
+<a href="http://code.google.com/p/go/source/detail?r=293c25943586">build</a> and
+<a href="http://code.google.com/p/go/source/detail?r=bf17e96b6582">runtime</a>
+changes to make Go run on OS X 10.7 Lion.
+</p>
+
 <h2 id="r57">r57 (released 2011/05/03)</h2>
 
 <p>

doc/devel/weekly.html

@@ -14,7 +14,7 @@ hg pull
 hg update weekly.<i>YYYY-MM-DD</i>
 </pre>
 
-<h2 id="2011-07-07">2011-07-07</h2>
+<h2 id="2011-07-07">2011-07-07 (<a href="release.html#r59">base for r59</a>)</h2>
 
 <pre>
 This weekly snapshot includes changes to the strings, http, reflect, json, and

src/cmd/gc/closure.c

@@ -116,12 +116,11 @@ typecheckclosure(Node *func, int top)
 	}
 }
 
-Node*
-walkclosure(Node *func, NodeList **init)
+static Node*
+makeclosure(Node *func, NodeList **init, int nowrap)
 {
-	int narg;
-	Node *xtype, *v, *addr, *xfunc, *call, *clos;
-	NodeList *l, *in;
+	Node *xtype, *v, *addr, *xfunc;
+	NodeList *l;
 	static int closgen;
 	char *p;
 
@@ -133,7 +132,6 @@ walkclosure(Node *func, NodeList **init)
 
 	// each closure variable has a corresponding
 	// address parameter.
-	narg = 0;
 	for(l=func->cvars; l; l=l->next) {
 		v = l->n;
 		if(v->op == 0)
@@ -146,7 +144,6 @@ walkclosure(Node *func, NodeList **init)
 		addr->class = PPARAM;
 		addr->addable = 1;
 		addr->ullman = 1;
-		narg++;
 
 		v->heapaddr = addr;
 
@@ -154,7 +151,8 @@ walkclosure(Node *func, NodeList **init)
 	}
 
 	// then a dummy arg where the closure's caller pc sits
-	xtype->list = list(xtype->list, nod(ODCLFIELD, N, typenod(types[TUINTPTR])));
+	if (!nowrap)
+		xtype->list = list(xtype->list, nod(ODCLFIELD, N, typenod(types[TUINTPTR])));
 
 	// then the function arguments
 	xtype->list = concat(xtype->list, func->list);
@@ -176,15 +174,36 @@ walkclosure(Node *func, NodeList **init)
 	typecheck(&xfunc, Etop);
 	closures = list(closures, xfunc);
 
+	return xfunc;
+}
+
+Node*
+walkclosure(Node *func, NodeList **init)
+{
+	int narg;
+	Node *xtype, *xfunc, *call, *clos;
+	NodeList *l, *in;
+
+	/*
+	 * wrap body in external function
+	 * with extra closure parameters.
+	 */
+
+	// create the function
+	xfunc = makeclosure(func, init, 0);
+	xtype = xfunc->nname->ntype;
+
 	// prepare call of sys.closure that turns external func into func literal value.
 	clos = syslook("closure", 1);
 	clos->type = T;
 	clos->ntype = nod(OTFUNC, N, N);
 	in = list1(nod(ODCLFIELD, N, typenod(types[TINT])));	// siz
 	in = list(in, nod(ODCLFIELD, N, xtype));
+	narg = 0;
 	for(l=func->cvars; l; l=l->next) {
 		if(l->n->op == 0)
 			continue;
+		narg++;
 		in = list(in, nod(ODCLFIELD, N, l->n->heapaddr->ntype));
 	}
 	clos->ntype->list = in;
@@ -211,33 +230,18 @@ walkclosure(Node *func, NodeList **init)
 void
 walkcallclosure(Node *n, NodeList **init)
 {
-	Node *z;
-	NodeList *ll, *cargs;
+	if (n->op != OCALLFUNC || n->left->op != OCLOSURE) {
+		dump("walkcallclosure", n);
+		fatal("abuse of walkcallclosure");
+	}
 
-	walkexpr(&n->left, init);
-	cargs =	n->left    // FUNC runtime.closure
-		->list     // arguments
-		->next     // skip first
-		->next;    // skip second
-
-	n->left = n->left  // FUNC runtime.closure
-		->list     // arguments
-		->next     // skip first
-		->n        // AS (to indreg) 
-		->right;   // argument  == the generated function 
-
-	// New arg list for n. First the closure-args, stolen from
-	// runtime.closure's 3rd and following,
-	ll = nil;
-	for (; cargs; cargs = cargs->next)
-		ll = list(ll, cargs->n->right);  // cargs->n is the OAS(INDREG, arg)
-
-	// then an extra zero, to fill the dummy return pointer slot,
-	z = nod(OXXX, N, N);
-	nodconst(z, types[TUINTPTR], 0);
-	z->typecheck = 1;
-	ll = list(ll, z);
-
-	// and finally the original parameter list.
-	n->list = concat(ll, n->list);
+	// New arg list for n. First the closure-args
+	// and then the original parameter list.
+	n->list = concat(n->left->enter, n->list);
+	n->left = makeclosure(n->left, init, 1)->nname;
+	dowidth(n->left->type);
+	n->type = getoutargx(n->left->type);
+	// for a single valued function, pull the field type out of the struct
+	if (n->type && n->type->type && !n->type->type->down)
+		n->type = n->type->type->type;
 }

src/cmd/gc/walk.c

@@ -494,9 +494,9 @@ walkexpr(Node **np, NodeList **init)
 		if(n->left->op == OCLOSURE) {
 			walkcallclosure(n, init);
 			t = n->left->type;
-		} else
-			walkexpr(&n->left, init);
+		}
 
+		walkexpr(&n->left, init);
 		walkexprlist(n->list, init);
 
 		ll = ascompatte(n->op, n->isddd, getinarg(t), n->list, 0, init);

test/fixedbugs/bug346.go

@@ -0,0 +1,19 @@
+// $G $D/$F.go && $L $F.$A && ./$A.out || echo BUG: issue2056
+
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import "os"
+
+func main() {
+	x := 4
+	a, b, c, d := func(i int) (p int, q int, r int, s int) { return 1, i, 3, x }(2)
+
+	if a != 1 || b != 2 || c != 3 || d != 4 {
+		println("abcd: expected 1 2 3 4 got", a, b, c, d)
+		os.Exit(1)
+	}
+}