Commit a069cf04 authored by Russ Cox's avatar Russ Cox

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
parent e5d742fc
......@@ -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;
}
......
......@@ -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);
}
......
......@@ -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.
......
......@@ -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
}
......
......@@ -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
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment