cmd/gc: distinguish unnamed vs blank-named return variables better

Before, an unnamed return value turned into an ONAME node n with n->sym
named ~anon%d, and n->orig == n.

A blank-named return value turned into an ONAME node n with n->sym
named ~anon%d but n->orig == the original blank n. Code generation and
printing uses n->orig, so that this node formatted as _.

But some code does not use n->orig. In particular the liveness code does
not know about the n->orig convention and so mishandles blank identifiers.
It is possible to fix but seemed better to avoid the confusion entirely.

Now the first kind of node is named ~r%d and the second ~b%d; both have
n->orig == n, so that it doesn't matter whether code uses n or n->orig.

After this change the ->orig field is only used for other kinds of expressions,
not for ONAME nodes.

This requires distinguishing ~b from ~r names in a few places that care.
It fixes a liveness analysis bug without actually changing the liveness code.

TBR=ken2
CC=golang-codereviews
https://golang.org/cl/63630043

diff --git a/src/cmd/gc/dcl.c b/src/cmd/gc/dcl.c
index 7df1d97a8c6af87303381af6e9d57dbf817f6b12..d105c74f699375077788b8a4629297b8b8542648 100644 (file)
--- a/src/cmd/gc/dcl.c
+++ b/src/cmd/gc/dcl.c
@@ -643,8 +643,8 @@ funcargs(Node *nt)
                        fatal("funcargs out %O", n->op);
 
                if(n->left == N) {
-                       // give it a name so escape analysis has nodes to work with
-                       snprint(namebuf, sizeof(namebuf), "~anon%d", gen++);
+                       // Name so that escape analysis can track it. ~r stands for 'result'.
+                       snprint(namebuf, sizeof(namebuf), "~r%d", gen++);
                        n->left = newname(lookup(namebuf));
                        // TODO: n->left->missing = 1;
                } 
@@ -652,14 +652,20 @@ funcargs(Node *nt)
                n->left->op = ONAME;
 
                if(isblank(n->left)) {
-                       // Give it a name so we can assign to it during return.
-                       // preserve the original in ->orig
+                       // Give it a name so we can assign to it during return. ~b stands for 'blank'.
+                       // The name must be different from ~r above because if you have
+                       //      func f() (_ int)
+                       //      func g() int
+                       // f is allowed to use a plain 'return' with no arguments, while g is not.
+                       // So the two cases must be distinguished.
+                       // We do not record a pointer to the original node (n->orig).
+                       // Having multiple names causes too much confusion in later passes.
                        nn = nod(OXXX, N, N);
                        *nn = *n->left;
+                       nn->orig = nn;
+                       snprint(namebuf, sizeof(namebuf), "~b%d", gen++);
+                       nn->sym = lookup(namebuf);
                        n->left = nn;
-                       
-                       snprint(namebuf, sizeof(namebuf), "~anon%d", gen++);
-                       n->left->sym = lookup(namebuf);
                }
 
                n->left->ntype = n->right;
@@ -1209,7 +1215,7 @@ functype(Node *this, NodeList *in, NodeList *out)
        t->outnamed = 0;
        if(t->outtuple > 0 && out->n->left != N && out->n->left->orig != N) {
                s = out->n->left->orig->sym;
-               if(s != S && s->name[0] != '~')
+               if(s != S && s->name[0] != '~' || s->name[1] != 'r') // ~r%d is the name invented for an unnamed result
                        t->outnamed = 1;
        }
 

diff --git a/src/cmd/gc/fmt.c b/src/cmd/gc/fmt.c
index 6f40c7ff305e0ff44eef5ded20c2747e6f152441..bffe8dfc7e69de04e67f0b447fa519d15e07a4ea 100644 (file)
--- a/src/cmd/gc/fmt.c
+++ b/src/cmd/gc/fmt.c
@@ -678,12 +678,17 @@ typefmt(Fmt *fp, Type *t)
                if(!(fp->flags&FmtShort)) {
                        s = t->sym;
 
-                       // Take the name from the original, lest we substituted it with ~anon%d
+                       // Take the name from the original, lest we substituted it with ~r%d or ~b%d.
+                       // ~r%d is a (formerly) unnamed result.
                        if ((fmtmode == FErr || fmtmode == FExp) && t->nname != N) {
                                if(t->nname->orig != N) {
                                        s = t->nname->orig->sym;
-                                       if(s != S && s->name[0] == '~')
-                                               s = S;
+                                       if(s != S && s->name[0] == '~') {
+                                               if(s->name[1] == 'r') // originally an unnamed result
+                                                       s = S;
+                                               else if(s->name[1] == 'b') // originally the blank identifier _
+                                                       s = lookup("_");
+                                       }
                                } else 
                                        s = S;
                        }
@@ -1104,7 +1109,10 @@ exprfmt(Fmt *f, Node *n, int prec)
                case PAUTO:
                case PPARAM:
                case PPARAMOUT:
-                       if(fmtmode == FExp && n->sym && !isblanksym(n->sym) && n->vargen > 0)
+                       // _ becomes ~b%d internally; print as _ for export
+                       if(fmtmode == FExp && n->sym && n->sym->name[0] == '~' && n->sym->name[1] == 'b')
+                               return fmtprint(f, "_");
+                       if(fmtmode == FExp && n->sym && !isblank(n) && n->vargen > 0)
                                return fmtprint(f, "%S·%d", n->sym, n->vargen);
                }
 

diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c
index eb5a3f1b6b9f7e633787447821b5be1a751c7c6e..1bceae9982a5976156a979fc73038e22e510e520 100644 (file)
--- a/src/cmd/gc/walk.c
+++ b/src/cmd/gc/walk.c
@@ -2430,7 +2430,7 @@ paramstoheap(Type **argin, int out)
        nn = nil;
        for(t = structfirst(&savet, argin); t != T; t = structnext(&savet)) {
                v = t->nname;
-               if(v && v->sym && v->sym->name[0] == '~')
+               if(v && v->sym && v->sym->name[0] == '~' && v->sym->name[1] == 'r') // unnamed result
                        v = N;
                // In precisestack mode, the garbage collector assumes results
                // are always live, so zero them always.

diff --git a/test/escape5.go b/test/escape5.go
index c9646872d51ac54486b3a000caad76c5afdc25ac..a33daeee18a02a102f4ac8b2e0649a25bd8bc2e9 100644 (file)
--- a/test/escape5.go
+++ b/test/escape5.go
@@ -17,19 +17,19 @@ func leaktoret(p *int) *int { // ERROR "leaking param: p to result"
        return p
 }
 
-func leaktoret2(p *int) (*int, *int) { // ERROR "leaking param: p to result .anon1" "leaking param: p to result .anon2"
+func leaktoret2(p *int) (*int, *int) { // ERROR "leaking param: p to result ~r1" "leaking param: p to result ~r2"
        return p, p
 }
 
-func leaktoret22(p, q *int) (*int, *int) { // ERROR "leaking param: p to result .anon2" "leaking param: q to result .anon3"
+func leaktoret22(p, q *int) (*int, *int) { // ERROR "leaking param: p to result ~r2" "leaking param: q to result ~r3"
        return p, q
 }
 
-func leaktoret22b(p, q *int) (*int, *int) { // ERROR "leaking param: p to result .anon3" "leaking param: q to result .anon2"
+func leaktoret22b(p, q *int) (*int, *int) { // ERROR "leaking param: p to result ~r3" "leaking param: q to result ~r2"
        return leaktoret22(q, p)
 }
 
-func leaktoret22c(p, q *int) (*int, *int) { // ERROR "leaking param: p to result .anon3" "leaking param: q to result .anon2"
+func leaktoret22c(p, q *int) (*int, *int) { // ERROR "leaking param: p to result ~r3" "leaking param: q to result ~r2"
        r, s := leaktoret22(q, p)
        return r, s
 }

diff --git a/test/live.go b/test/live.go
index dc2ec86fdeedbad7e296b05dcc83f2aa2475388b..c0ea13129465cf9b67b8a8127894bb8688c40455 100644 (file)
--- a/test/live.go
+++ b/test/live.go
@@ -79,3 +79,10 @@ func f5(b1 bool) {
        }
        print(**z) // ERROR "live at call to printint: x y$"
 }
+
+// confusion about the _ result used to cause spurious "live at entry to f6: _".
+
+func f6() (_, y string) {
+       y = "hello"
+       return
+}