code review 3987045: cc: mode to generate go-code for types and variables.

cc: mode to generate go-code for types and variables.

[rsc, 2011-01-24 19:36:31]

Looks pretty reasonable.
Please print the output formatted in gofmt-standard style.

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/dcl.c
File src/cmd/cc/dcl.c (right):

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/dcl.c#newcode4
src/cmd/cc/dcl.c:4: //	Copyright © 1994-1999 Lucent Technologies Inc.	All rights
reserved.
revert change to this line

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c
File src/cmd/cc/godefs.c (right):

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode106
src/cmd/cc/godefs.c:106: if (n != nil)
1,$s/(if|for|switch) \(/\1(/

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode136
src/cmd/cc/godefs.c:136: Bprint(&outbuf, "uintptr  // *void");
unsafe.Pointer

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode142
src/cmd/cc/godefs.c:142: case TIND: Bprint(&outbuf, "*"); continue;
break line

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode158
src/cmd/cc/godefs.c:158: case TINT:	Bprint(&outbuf, "int"); break;
line breaks
(define a table if you want compact code)

static char *gonames[] = {
    [TINT] = "int",
    [TUINT] = "uint",
...
}

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode187
src/cmd/cc/godefs.c:187: dontrun(int f) {
s/{/\n{

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode203
src/cmd/cc/godefs.c:203: // Non-static functions follow
d

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode226
src/cmd/cc/godefs.c:226: // for unions, comment out all but first
Too much work, no reward.  Instead:

[N]byte  // union

where N is the size of the union.

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode248
src/cmd/cc/godefs.c:248: if (dontrun('q')) return;
line breaks

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode255
src/cmd/cc/godefs.c:255: case  TENUM:
s/  / /

[rsc, 2011-01-24 19:37:41]

On Mon, Jan 24, 2011 at 14:36,  <rsc@golang.org> wrote:
> Looks pretty reasonable.
> Please print the output formatted in gofmt-standard style.

If you do, here is gouniq

#!/bin/sh

awk '
/^(var|type) / {
	if(seen[$2]++) {
		if(/{$/) {
			skip = 1;
		}
		next;
	}
}

skip {
	if(/^}/)
		skip = 0;
	next;
}

{print}
' "$@"

[lvd, 2011-01-25 11:52:57]

mostly done.  made functions work too. see http://codereview.appspot.com/4047047
(in progress) on how its going to be used.  that will require some renaming of c
vs Go's struct *Type.

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/dcl.c
File src/cmd/cc/dcl.c (right):

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/dcl.c#newcode4
src/cmd/cc/dcl.c:4: //	Copyright © 1994-1999 Lucent Technologies Inc.	All rights
reserved.
On 2011/01/24 19:36:31, rsc wrote:
> revert change to this line

Done.

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c
File src/cmd/cc/godefs.c (right):

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode106
src/cmd/cc/godefs.c:106: if (n != nil)
On 2011/01/24 19:36:31, rsc wrote:
> 1,$s/(if|for|switch) \(/\1(/

Done.

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode136
src/cmd/cc/godefs.c:136: Bprint(&outbuf, "uintptr  // *void");
On 2011/01/24 19:36:31, rsc wrote:
> unsafe.Pointer

Done.

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode142
src/cmd/cc/godefs.c:142: case TIND: Bprint(&outbuf, "*"); continue;
On 2011/01/24 19:36:31, rsc wrote:
> break line

Done.

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode187
src/cmd/cc/godefs.c:187: dontrun(int f) {
On 2011/01/24 19:36:31, rsc wrote:
> s/{/\n{

Done.

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode203
src/cmd/cc/godefs.c:203: // Non-static functions follow
On 2011/01/24 19:36:31, rsc wrote:
> d

Done.

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode226
src/cmd/cc/godefs.c:226: // for unions, comment out all but first
On 2011/01/24 19:36:31, rsc wrote:
> Too much work, no reward.  Instead:
> 
> [N]byte  // union
> 
> where N is the size of the union.

found useful compromise

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode248
src/cmd/cc/godefs.c:248: if (dontrun('q')) return;
On 2011/01/24 19:36:31, rsc wrote:
> line breaks

Done.

http://codereview.appspot.com/3987045/diff/1006/src/cmd/cc/godefs.c#newcode255
src/cmd/cc/godefs.c:255: case  TENUM:
On 2011/01/24 19:36:31, rsc wrote:
> s/  / /

Done.

[rsc, 2011-01-25 16:10:58]

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/dcl.c
File src/cmd/cc/dcl.c (right):

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/dcl.c#newcode4
src/cmd/cc/dcl.c:4: //	Copyright © 1994-1999 Lucent Technologies Inc.	All rights
reserved.
revert tab change

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c
File src/cmd/cc/godefs.c (right):

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode89
src/cmd/cc/godefs.c:89: 
if(upper)
    return s;

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode125
src/cmd/cc/godefs.c:125: 
delete 2 blank lines of the 3

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode165
src/cmd/cc/godefs.c:165: case TINT:	Bprint(&outbuf, "int"); break;
same comment as before
split lines
make a table

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode215
src/cmd/cc/godefs.c:215: static int publicidentifiers = 0;
move to top of file
=0 implied

static int upper
would be fine (shorter)

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode218
src/cmd/cc/godefs.c:218: dontrun()
s/()/(void)

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode241
src/cmd/cc/godefs.c:241: char str[STRINGSZ];
s/STRINGSZ/&+1/

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode248
src/cmd/cc/godefs.c:248: str[0] = publicidentifiers ? toupper(*s) : tolower(*s);

if(upper)
    str[0] = toupper(*s)
else
    str[0] = tolower(*s)

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode255
src/cmd/cc/godefs.c:255: 
delete 2 blank lines of the 3

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode326
src/cmd/cc/godefs.c:326: for (t1 = t->down; t1 != T; t1 = t1->down) {
,s/for (/for(/g
,s/if (/if(/g

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/lex.c
File src/cmd/cc/lex.c (right):

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/lex.c#newcode74
src/cmd/cc/lex.c:74: *	-q		print equivalent Go code for variables and types
(locase identifiers)
s/locase/lower-case/
s/upcase/upper-case/

okay to go past 80 characters

[lvd, 2011-01-25 17:11:13]

all done

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/dcl.c
File src/cmd/cc/dcl.c (right):

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/dcl.c#newcode4
src/cmd/cc/dcl.c:4: //	Copyright © 1994-1999 Lucent Technologies Inc.	All rights
reserved.
On 2011/01/25 16:10:58, rsc wrote:
> revert tab change

Done.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c
File src/cmd/cc/godefs.c (right):

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode89
src/cmd/cc/godefs.c:89: 
On 2011/01/25 16:10:58, rsc wrote:
> if(upper)
>     return s;
no i don't think so.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode125
src/cmd/cc/godefs.c:125: 
On 2011/01/25 16:10:58, rsc wrote:
> delete 2 blank lines of the 3

Done.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode165
src/cmd/cc/godefs.c:165: case TINT:	Bprint(&outbuf, "int"); break;
On 2011/01/25 16:10:58, rsc wrote:
> same comment as before
> split lines
> make a table 

Done.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode215
src/cmd/cc/godefs.c:215: static int publicidentifiers = 0;
On 2011/01/25 16:10:58, rsc wrote:
> move to top of file
> =0 implied
> 
> static int upper
> would be fine (shorter)

Done.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode218
src/cmd/cc/godefs.c:218: dontrun()
On 2011/01/25 16:10:58, rsc wrote:
> s/()/(void)

Done.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode241
src/cmd/cc/godefs.c:241: char str[STRINGSZ];
On 2011/01/25 16:10:58, rsc wrote:
> s/STRINGSZ/&+1/

Done.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode248
src/cmd/cc/godefs.c:248: str[0] = publicidentifiers ? toupper(*s) : tolower(*s);
On 2011/01/25 16:10:58, rsc wrote:
> 
> if(upper)
>     str[0] = toupper(*s)
> else
>     str[0] = tolower(*s)

Done.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode255
src/cmd/cc/godefs.c:255: 
On 2011/01/25 16:10:58, rsc wrote:
> delete 2 blank lines of the 3

Done.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode326
src/cmd/cc/godefs.c:326: for (t1 = t->down; t1 != T; t1 = t1->down) {
On 2011/01/25 16:10:58, rsc wrote:
> ,s/for (/for(/g
> ,s/if (/if(/g

Done.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/lex.c
File src/cmd/cc/lex.c (right):

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/lex.c#newcode74
src/cmd/cc/lex.c:74: *	-q		print equivalent Go code for variables and types
(locase identifiers)
On 2011/01/25 16:10:58, rsc wrote:
> s/locase/lower-case/
> s/upcase/upper-case/
> 
> okay to go past 80 characters

Done.

[rsc, 2011-01-25 17:52:58]

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c
File src/cmd/cc/godefs.c (right):

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode89
src/cmd/cc/godefs.c:89: 
On 2011/01/25 17:11:13, lvd wrote:
> On 2011/01/25 16:10:58, rsc wrote:
> > if(upper)
> >     return s;
> no i don't think so.

agree that that's not quite right.
what i was trying to do was make
sure that if you had a field named,
say, type, it would not be renamed
to _type and then uppercased to _type.
in that case you want to skip the _ 
processing and just format it as Type.
so maybe the real fix is 

if(!upper) { 
   ... current binary search
}

http://codereview.appspot.com/3987045/diff/25001/src/cmd/cc/godefs.c#newcode166
src/cmd/cc/godefs.c:166: case TUINT:	Bprint(&outbuf, "uint"); break;
this is fine if you want to leave it this way
but it still seems like a table would be easier
to edit.  

static char *gonames[] = {
    [TINT] = "int",
    [TUINT] = "uint",
...
}

default:
    if(t->etype < nelem(gonames) && gonames[t->etype] != nil) {
        Bprint(&outbuf, "%s", gonames[t->etype]);
        return;
    }

either way

[lvd, 2011-01-25 17:56:46]

On Tue, Jan 25, 2011 at 18:52, <rsc@golang.org> wrote:

>
> http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c
> File src/cmd/cc/godefs.c (right):
>
>
> http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode89
> src/cmd/cc/godefs.c:89:
> On 2011/01/25 17:11:13, lvd wrote:
>
>> On 2011/01/25 16:10:58, rsc wrote:
>> > if(upper)
>> >     return s;
>> no i don't think so.
>>
>
> agree that that's not quite right.
> what i was trying to do was make
> sure that if you had a field named,
> say, type, it would not be renamed
> to _type and then uppercased to _type.
> in that case you want to skip the _
> processing and just format it as Type.
> so maybe the real fix is
>
> if(!upper) {
>   ... current binary search
> }
>

done


>
>
>
http://codereview.appspot.com/3987045/diff/25001/src/cmd/cc/godefs.c#newcode166
> src/cmd/cc/godefs.c:166: case TUINT:    Bprint(&outbuf, "uint"); break;
> this is fine if you want to leave it this way
> but it still seems like a table would be easier
> to edit.


if i ever change it i'll make it a table.


>
>
> static char *gonames[] = {
>    [TINT] = "int",
>    [TUINT] = "uint",
> ...
> }
>
> default:
>    if(t->etype < nelem(gonames) && gonames[t->etype] != nil) {
>        Bprint(&outbuf, "%s", gonames[t->etype]);
>        return;
>    }
>
> either way
>
>
> http://codereview.appspot.com/3987045/
>

[rsc, 2011-01-25 17:58:33]

LGTM

[lvd, 2011-01-25 18:00:13]

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

I'd like you to review this change.

[lvd, 2011-01-25 18:00:20]

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

cc: mode to generate go-code for types and variables.

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

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c
File src/cmd/cc/godefs.c (right):

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode89
src/cmd/cc/godefs.c:89: 
On 2011/01/25 17:52:58, rsc wrote:
> On 2011/01/25 17:11:13, lvd wrote:
> > On 2011/01/25 16:10:58, rsc wrote:
> > > if(upper)
> > >     return s;
> > no i don't think so.
> 
> agree that that's not quite right.
> what i was trying to do was make
> sure that if you had a field named,
> say, type, it would not be renamed
> to _type and then uppercased to _type.
> in that case you want to skip the _ 
> processing and just format it as Type.
> so maybe the real fix is 
> 
> if(!upper) { 
>    ... current binary search
> }
> 

Done.

http://codereview.appspot.com/3987045/diff/15001/src/cmd/cc/godefs.c#newcode125
src/cmd/cc/godefs.c:125: 
On 2011/01/25 17:11:13, lvd wrote:
> On 2011/01/25 16:10:58, rsc wrote:
> > delete 2 blank lines of the 3
> 
> Done.

Done.