code review 116440046: cmd/gc: prohibit short variable declarations containing...

cmd/gc: prohibit short variable declarations containing duplicate symbols

Fixes issue 6764.
Fixes issue 8435.

[esk, 2014-08-01 03:23:20]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[esk, 2014-08-17 00:18:39]

On 2014/08/01 03:23:20, esk wrote:
> Hello mailto:golang-codereviews@googlegroups.com,
> 
> I'd like you to review this change to
> https://code.google.com/p/go/

Are we holding this change until the compiler has been translated to Go?

[r, 2014-08-26 17:51:40]

No, we're just holding fast on most compiler changes at the moment.

[gobot, 2014-09-07 05:36:21]

R=rsc@golang.org (assigned by r@golang.org)

[rsc, 2014-09-24 19:08:45]

It may not matter in practice in this case, but I try hard not to put quadratic
algorithms into the compiler when possible. Here, you're trying to find
duplicated entries in the list, which is easy if you mark them and then check
for the marks.

https://codereview.appspot.com/116440046/diff/40001/src/cmd/gc/dcl.c
File src/cmd/gc/dcl.c (right):

https://codereview.appspot.com/116440046/diff/40001/src/cmd/gc/dcl.c#newcode493
src/cmd/gc/dcl.c:493: for(l=left; l; l=l->next) {
for(l=left; l; l=l->next)
    if(l->n->sym != S)
        l->n->sym->flags |= SymUniq;

for(l=left; l; l=l->next) {

https://codereview.appspot.com/116440046/diff/40001/src/cmd/gc/dcl.c#newcode502
src/cmd/gc/dcl.c:502: if(n->sym->block == block) {
if((n->sym->flags & SymUniq) == 0)
    yyerrorl(defn->lineno, "%s repeated on left side of :=");
n->sym->flags &= ~SymUniq;

if(n->sym->block == block)
    continue;

[rsc, 2014-09-24 19:09:43]

Please also add a test for issue 6764. It can go into the same test file.

[esk, 2014-09-26 02:34:02]

PTAL

https://codereview.appspot.com/116440046/diff/40001/src/cmd/gc/dcl.c
File src/cmd/gc/dcl.c (right):

https://codereview.appspot.com/116440046/diff/40001/src/cmd/gc/dcl.c#newcode493
src/cmd/gc/dcl.c:493: for(l=left; l; l=l->next) {
On 2014/09/24 19:08:45, rsc wrote:
> for(l=left; l; l=l->next)
>     if(l->n->sym != S)
>         l->n->sym->flags |= SymUniq;
> 
> for(l=left; l; l=l->next) {

Done.

https://codereview.appspot.com/116440046/diff/40001/src/cmd/gc/dcl.c#newcode502
src/cmd/gc/dcl.c:502: if(n->sym->block == block) {
On 2014/09/24 19:08:45, rsc wrote:
> if((n->sym->flags & SymUniq) == 0)
>     yyerrorl(defn->lineno, "%s repeated on left side of :=");
> n->sym->flags &= ~SymUniq;
> 
> if(n->sym->block == block)
>     continue;
> 

Done.

[rsc, 2014-10-06 20:36:52]

LGTM

[rsc, 2014-10-06 21:16:45]

*** Submitted as https://code.google.com/p/go/source/detail?r=895c694ab135 ***

cmd/gc: prohibit short variable declarations containing duplicate symbols

Fixes issue 6764.
Fixes issue 8435.

LGTM=rsc
R=golang-codereviews, r, gobot, rsc
CC=golang-codereviews
https://codereview.appspot.com/116440046

Committer: Russ Cox <rsc@golang.org>