code review 5364043: gc: Preserve original blank name for .anon substitution...

gc: Preserve original blank name for .anon substitution on out params.

Fixes issue 1802.

[lvd, 2011-11-07 11:01:37]

Hello rsc@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[lvd, 2011-11-07 11:22:04]

hold on this breaks something.

On Mon, Nov 7, 2011 at 12:01, <lvd@google.com> wrote:

> Reviewers: rsc,
>
> Message:
> Hello rsc@golang.org (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> gc: Preserve original blank name for .anon substitution on out params.
>
> Fixes issue 1802.
>
> This CL depends on 5351042
>
> Please review this at
http://codereview.appspot.com/**5364043/<http://codereview.appspot.com/5364043/>
>
> Affected files:
>  M src/cmd/gc/dcl.c
>  A test/fixedbugs/bug377.dir/one.**go
>  A test/fixedbugs/bug377.dir/two.**go
>  A test/fixedbugs/bug377.go
>
>
>

[lvd, 2011-11-07 12:24:42]

On Mon, Nov 7, 2011 at 12:22, Luuk van Dijk <lvd@google.com> wrote:

> hold on this breaks something.
>
>
slightly klunkier now, but at least works and still fairly minimal.

best viewed after 5351042 is in.


>
> On Mon, Nov 7, 2011 at 12:01, <lvd@google.com> wrote:
>
>> Reviewers: rsc,
>>
>> Message:
>> Hello rsc@golang.org (cc: golang-dev@googlegroups.com),
>>
>> I'd like you to review this change to
>> https://go.googlecode.com/hg/
>>
>>
>> Description:
>> gc: Preserve original blank name for .anon substitution on out params.
>>
>> Fixes issue 1802.
>>
>> This CL depends on 5351042
>>
>> Please review this at
http://codereview.appspot.com/**5364043/<http://codereview.appspot.com/5364043/>
>>
>> Affected files:
>>  M src/cmd/gc/dcl.c
>>  A test/fixedbugs/bug377.dir/one.**go
>>  A test/fixedbugs/bug377.dir/two.**go
>>  A test/fixedbugs/bug377.go
>>
>>
>>
>

[rsc, 2011-11-09 04:34:07]

LGTM

http://codereview.appspot.com/5364043/diff/11001/src/cmd/gc/fmt.c
File src/cmd/gc/fmt.c (right):

http://codereview.appspot.com/5364043/diff/11001/src/cmd/gc/fmt.c#newcode685
src/cmd/gc/fmt.c:685: if (t->nname && isblank(t->nname->orig)) {
Why is blank a special case here?  Isn't it a sym like any other?

http://codereview.appspot.com/5364043/diff/11001/test/fixedbugs/bug377.dir/tw...
File test/fixedbugs/bug377.dir/two.go (right):

http://codereview.appspot.com/5364043/diff/11001/test/fixedbugs/bug377.dir/tw...
test/fixedbugs/bug377.dir/two.go:3: import _ "one"
please use import _ "./one"
and then you can drop the -I. from the recipe.
-I comes after the standard packages.
"./one" is the reliable way to get a package in the
current directory.

[rsc, 2011-11-09 04:34:22]

another

http://codereview.appspot.com/5364043/diff/11001/test/fixedbugs/bug377.go
File test/fixedbugs/bug377.go (right):

http://codereview.appspot.com/5364043/diff/11001/test/fixedbugs/bug377.go#new...
test/fixedbugs/bug377.go:3: // Copyright 2009 The Go Authors. All rights
reserved.
2011

[lvd, 2011-11-09 10:18:25]

http://codereview.appspot.com/5364043/diff/11001/src/cmd/gc/fmt.c
File src/cmd/gc/fmt.c (right):

http://codereview.appspot.com/5364043/diff/11001/src/cmd/gc/fmt.c#newcode685
src/cmd/gc/fmt.c:685: if (t->nname && isblank(t->nname->orig)) {
On 2011/11/09 04:34:07, rsc wrote:
> Why is blank a special case here?  Isn't it a sym like any other?

you're right.  simplified and tested.

http://codereview.appspot.com/5364043/diff/11001/test/fixedbugs/bug377.dir/tw...
File test/fixedbugs/bug377.dir/two.go (right):

http://codereview.appspot.com/5364043/diff/11001/test/fixedbugs/bug377.dir/tw...
test/fixedbugs/bug377.dir/two.go:3: import _ "one"
On 2011/11/09 04:34:07, rsc wrote:
> please use import _ "./one"
> and then you can drop the -I. from the recipe.
> -I comes after the standard packages.
> "./one" is the reliable way to get a package in the
> current directory.

Done.

http://codereview.appspot.com/5364043/diff/11001/test/fixedbugs/bug377.go
File test/fixedbugs/bug377.go (right):

http://codereview.appspot.com/5364043/diff/11001/test/fixedbugs/bug377.go#new...
test/fixedbugs/bug377.go:3: // Copyright 2009 The Go Authors. All rights
reserved.
On 2011/11/09 04:34:23, rsc wrote:
> 2011

time flies

[lvd, 2011-11-09 10:27:31]

*** Submitted as http://code.google.com/p/go/source/detail?r=afa4ccab6746 ***

gc: Preserve original blank name for .anon substitution on out params.

Fixes issue 1802.

R=rsc
CC=golang-dev
http://codereview.appspot.com/5364043