code review 7035043: cmd/5l, cmd/6l, cmd/8l, cmd/cc, cmd/gc: new flag parsing

cmd/5l, cmd/6l, cmd/8l, cmd/cc, cmd/gc: new flag parsing

This CL adds a flag parser that matches the semantics of Go's
package flag. It also changes the linkers and compilers to use
the new flag parser.

Command lines that used to work, like
        8c -FVw
        6c -Dfoo
        5g -I/foo/bar
now need to be split into separate arguments:
        8c -F -V -w
        6c -D foo
        5g -I /foo/bar
The new spacing will work with both old and new tools.

The new parser also allows = for arguments, as in
        6c -D=foo
        5g -I=/foo/bar
but that syntax will not work with the old tools.

In addition to matching standard Go binary flag parsing,
the new flag parser generates more detailed usage messages
and opens the door to long flag names.

The recently added gc flag -= has been renamed -complete.

[rsc, 2012-12-30 02:47:34]

Hello golang-dev@googlegroups.com,

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

[remyoudompheng, 2012-12-30 08:37:16]

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c
File src/cmd/5l/obj.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c#newcode104
src/cmd/5l/obj.c:104: flagfn1("I", "interp: set ELF interp", setinterp);
There are P and G flags in asmout (debug assembler)

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c#newcode109
src/cmd/5l/obj.c:109: flagcount("Q", "debug byte-register code gen",
&debug['Q']);
I don't see a Q flag here.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c#newcode111
src/cmd/5l/obj.c:111: flagcount("S", "check type signatures", &debug['S']);
there is a M flag that seems to disable software div/mod

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c#newcode111
src/cmd/5l/obj.c:111: flagcount("S", "check type signatures", &debug['S']);
no S flag in 5l

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c#newcode128
src/cmd/5l/obj.c:128: flagcount("n", "dump symbol table", &debug['n']);
n is before o

https://codereview.appspot.com/7035043/diff/1002/src/cmd/6l/obj.c
File src/cmd/6l/obj.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/6l/obj.c#newcode97
src/cmd/6l/obj.c:97: flagstr("E", "sym: entry symbol", &INITENTRY);
E is after D (was it supposed to be sorted?)

https://codereview.appspot.com/7035043/diff/1002/src/cmd/6l/obj.c#newcode123
src/cmd/6l/obj.c:123: flagcount("n", "dump symbol table", &debug['n']);
n is before o

https://codereview.appspot.com/7035043/diff/1002/src/cmd/6l/pass.c
File src/cmd/6l/pass.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/6l/pass.c#newcode656
src/cmd/6l/pass.c:656: if(zerostack && autoffset &&
!(cursym->text->from.scale&NOSPLIT)) {
where does zerostack come from? it's debug['Z'] in obj.c

https://codereview.appspot.com/7035043/diff/1002/src/cmd/cc/lex.c
File src/cmd/cc/lex.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/cc/lex.c#newcode136
src/cmd/cc/lex.c:136: flagcount("<", "debug shift", &debug['<']);
flag A debugs alignment

https://codereview.appspot.com/7035043/diff/1002/src/cmd/cc/lex.c#newcode138
src/cmd/cc/lex.c:138: flagfn1("D", "name[=value]: add #define", dodef);
if thechar=='5'!
  flag C debugs constprop
  flag H debugs shiftprop

https://codereview.appspot.com/7035043/diff/1002/src/cmd/cc/lex.c#newcode148
src/cmd/cc/lex.c:148: flagcount("T", "enable type signatures", &debug['S']);
I think you meant debug['T']

https://codereview.appspot.com/7035043/diff/1002/src/cmd/cc/lex.c#newcode165
src/cmd/cc/lex.c:165: flagcount("q", "print Go definitions", &debug['q']);
there seems to be a 's' flag in acid.c

https://codereview.appspot.com/7035043/diff/1002/src/cmd/dist/build.c
File src/cmd/dist/build.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/dist/build.c#newcode864
src/cmd/dist/build.c:864: vadd(&compile, "-FTVw");
should this be fixed?

https://codereview.appspot.com/7035043/diff/1002/src/cmd/gc/lex.c
File src/cmd/gc/lex.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/gc/lex.c#newcode240
src/cmd/gc/lex.c:240: flagcount("complete", "compiling complete package (no C or
assembly)", &pure_go);
the race detection flag has disappeared

https://codereview.appspot.com/7035043/diff/1002/src/cmd/gc/lex.c#newcode247
src/cmd/gc/lex.c:247: flagcount("j", "debug runtime-initialized variables",
&debug['i']);
debug['j']

[DMorsing, 2012-12-30 09:34:18]

This CL gives a bunch of errors on gcc 4.7.2, mostly because of giving int
pointers to the char elements of the debug array.

[rsc, 2012-12-30 15:24:49]

On Sun, Dec 30, 2012 at 4:34 AM, <daniel.morsing@gmail.com> wrote:

> This CL gives a bunch of errors on gcc 4.7.2, mostly because of giving
> int pointers to the char elements of the debug array.


in which directory? i probably forgot to put a file in the CL
or it's in another pending CL of mine. i thought i changed
all the debug arrays to be ints.

russ

[rsc, 2012-12-30 15:40:37]

PTAL

Thank you for the very detailed review.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c
File src/cmd/5l/obj.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c#newcode104
src/cmd/5l/obj.c:104: flagfn1("I", "interp: set ELF interp", setinterp);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> There are P and G flags in asmout (debug assembler)

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c#newcode109
src/cmd/5l/obj.c:109: flagcount("Q", "debug byte-register code gen",
&debug['Q']);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> I don't see a Q flag here.

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c#newcode111
src/cmd/5l/obj.c:111: flagcount("S", "check type signatures", &debug['S']);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> there is a M flag that seems to disable software div/mod

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c#newcode111
src/cmd/5l/obj.c:111: flagcount("S", "check type signatures", &debug['S']);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> no S flag in 5l

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/5l/obj.c#newcode128
src/cmd/5l/obj.c:128: flagcount("n", "dump symbol table", &debug['n']);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> n is before o

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/6l/obj.c
File src/cmd/6l/obj.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/6l/obj.c#newcode97
src/cmd/6l/obj.c:97: flagstr("E", "sym: entry symbol", &INITENTRY);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> E is after D (was it supposed to be sorted?)

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/6l/obj.c#newcode123
src/cmd/6l/obj.c:123: flagcount("n", "dump symbol table", &debug['n']);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> n is before o

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/6l/pass.c
File src/cmd/6l/pass.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/6l/pass.c#newcode656
src/cmd/6l/pass.c:656: if(zerostack && autoffset &&
!(cursym->text->from.scale&NOSPLIT)) {
On 2012/12/30 08:37:16, remyoudompheng wrote:
> where does zerostack come from? it's debug['Z'] in obj.c

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/cc/lex.c
File src/cmd/cc/lex.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/cc/lex.c#newcode136
src/cmd/cc/lex.c:136: flagcount("<", "debug shift", &debug['<']);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> flag A debugs alignment

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/cc/lex.c#newcode138
src/cmd/cc/lex.c:138: flagfn1("D", "name[=value]: add #define", dodef);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> if thechar=='5'!
>   flag C debugs constprop
>   flag H debugs shiftprop

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/cc/lex.c#newcode148
src/cmd/cc/lex.c:148: flagcount("T", "enable type signatures", &debug['S']);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> I think you meant debug['T']

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/cc/lex.c#newcode165
src/cmd/cc/lex.c:165: flagcount("q", "print Go definitions", &debug['q']);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> there seems to be a 's' flag in acid.c

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/dist/build.c
File src/cmd/dist/build.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/dist/build.c#newcode864
src/cmd/dist/build.c:864: vadd(&compile, "-FTVw");
On 2012/12/30 08:37:16, remyoudompheng wrote:
> should this be fixed?

This is setting up for the host compiler, to build the tools. So it's okay: it's
talking to the real Plan 9 compiler not the Go variant.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/gc/lex.c
File src/cmd/gc/lex.c (right):

https://codereview.appspot.com/7035043/diff/1002/src/cmd/gc/lex.c#newcode240
src/cmd/gc/lex.c:240: flagcount("complete", "compiling complete package (no C or
assembly)", &pure_go);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> the race detection flag has disappeared

Done.

https://codereview.appspot.com/7035043/diff/1002/src/cmd/gc/lex.c#newcode247
src/cmd/gc/lex.c:247: flagcount("j", "debug runtime-initialized variables",
&debug['i']);
On 2012/12/30 08:37:16, remyoudompheng wrote:
> debug['j']

Done.

[minux1, 2012-12-30 15:41:55]

On 2012/12/30 15:24:49, rsc wrote:
> in which directory? i probably forgot to put a file in the CL
> or it's in another pending CL of mine. i thought i changed
> all the debug arrays to be ints.
You need the following change:
--- a/src/cmd/gc/go.h   Sun Dec 30 15:40:42 2012 +1100
+++ b/src/cmd/gc/go.h   Sun Dec 30 23:37:35 2012 +0800
@@ -841,7 +841,7 @@
 EXTERN char    namebuf[NSYMB];
 EXTERN char    lexbuf[NSYMB];
 EXTERN char    litbuf[NSYMB];
-EXTERN char    debug[256];
+EXTERN int     debug[256];
 EXTERN Sym*    hash[NHASH];
 EXTERN Sym*    importmyname;   // my name for package
 EXTERN Pkg*    localpkg;       // package being compiled

and you need to declare the 3rd parameter of flagint64 as vlong*:
void    flagint64(char*, char*, vlong*);

[rsc, 2012-12-30 15:45:51]

where is vlong != int64?

[rsc, 2012-12-30 15:48:31]

I cleaned out the other CLs from my client and uploaded a new one here. It
should be complete now.

[minux1, 2012-12-30 15:56:15]

On Sun, Dec 30, 2012 at 11:45 PM, Russ Cox <rsc@golang.org> wrote:

> where is vlong != int64?
>
INITTEXT and INITDAT used to be typed vlong, and gcc 4.8.0 thinks
on amd64, long long != int64_t.

This is fixed in your latest patch set.

[DMorsing, 2012-12-30 16:21:39]

All errors/warnings are fixed now.

This CL exposes the various debug only flags via the usage message, but I think
it's ok since invoking go tool interactively isn't usually done. If not, there
should probably be an extra parameter for normal flags/debug flags.

Do we need to point this change out in go1.1.html for people who are using the
various tools through shell scripts?

[minux1, 2012-12-30 18:08:35]

found three used but not declared flags in cc.

https://codereview.appspot.com/7035043/diff/6019/src/cmd/cc/lex.c
File src/cmd/cc/lex.c (right):

https://codereview.appspot.com/7035043/diff/6019/src/cmd/cc/lex.c#newcode157
src/cmd/cc/lex.c:157: flagcount("Y", "debug index generation", &debug['Y']);
flagcount("Z", "cc/cc.y:138", &debug['Z']);

https://codereview.appspot.com/7035043/diff/6019/src/cmd/cc/lex.c#newcode158
src/cmd/cc/lex.c:158: flagcount("a", "print acid definitions", &debug['a']);
flagcount("c", "cc/pgen.c:479", &debug['c']);

https://codereview.appspot.com/7035043/diff/6019/src/cmd/cc/lex.c#newcode162
src/cmd/cc/lex.c:162: flagcount("g", "debug code generation", &debug['g']);
flagcount("h", "cc/sub.c:851, 5c/peep.c:827", &debug['h']);

[rsc, 2013-01-02 20:51:21]

PTAL

https://codereview.appspot.com/7035043/diff/6019/src/cmd/cc/lex.c
File src/cmd/cc/lex.c (right):

https://codereview.appspot.com/7035043/diff/6019/src/cmd/cc/lex.c#newcode157
src/cmd/cc/lex.c:157: flagcount("Y", "debug index generation", &debug['Y']);
On 2012/12/30 18:08:36, minux wrote:
> flagcount("Z", "cc/cc.y:138", &debug['Z']);

Done.

https://codereview.appspot.com/7035043/diff/6019/src/cmd/cc/lex.c#newcode158
src/cmd/cc/lex.c:158: flagcount("a", "print acid definitions", &debug['a']);
On 2012/12/30 18:08:36, minux wrote:
> flagcount("c", "cc/pgen.c:479", &debug['c']);

Done.

https://codereview.appspot.com/7035043/diff/6019/src/cmd/cc/lex.c#newcode162
src/cmd/cc/lex.c:162: flagcount("g", "debug code generation", &debug['g']);
On 2012/12/30 18:08:36, minux wrote:
> flagcount("h", "cc/sub.c:851, 5c/peep.c:827", &debug['h']);

Done. (I deleted the references to debug['h'].)

[iant, 2013-01-04 17:17:55]

LGTM

https://codereview.appspot.com/7035043/diff/11005/doc/go1.1.html
File doc/go1.1.html (right):

https://codereview.appspot.com/7035043/diff/11005/doc/go1.1.html#newcode24
doc/go1.1.html:24: In the gc toolchain, the assemblers, compilers, and linkers
now use the
This CL doesn't change the assemblers--is that a followup?

https://codereview.appspot.com/7035043/diff/11005/src/cmd/8l/obj.c
File src/cmd/8l/obj.c (right):

https://codereview.appspot.com/7035043/diff/11005/src/cmd/8l/obj.c#newcode82
src/cmd/8l/obj.c:82: debug['I'] = 1; // denote cmdline interpreter override
This function occurs three times, and could be moved into ld.

https://codereview.appspot.com/7035043/diff/11005/src/lib9/flag.c
File src/lib9/flag.c (right):

https://codereview.appspot.com/7035043/diff/11005/src/lib9/flag.c#newcode100
src/lib9/flag.c:100: *(int64*)p = strtoll(s, 0, 0);
Check for non-number?

https://codereview.appspot.com/7035043/diff/11005/src/lib9/flag.c#newcode117
src/lib9/flag.c:117: *(int32*)p = strtol(s, 0, 0);
Check for non-number?

https://codereview.appspot.com/7035043/diff/11005/src/lib9/flag.c#newcode152
src/lib9/flag.c:152: ((void(*)(void))p)();
USED(s); ?

[rsc, 2013-01-06 20:09:16]

https://codereview.appspot.com/7035043/diff/11005/doc/go1.1.html
File doc/go1.1.html (right):

https://codereview.appspot.com/7035043/diff/11005/doc/go1.1.html#newcode24
doc/go1.1.html:24: In the gc toolchain, the assemblers, compilers, and linkers
now use the
On 2013/01/04 17:17:55, iant wrote:
> This CL doesn't change the assemblers--is that a followup?

I'm undecided. I didn't mean to write assemblers here. Removed.

https://codereview.appspot.com/7035043/diff/11005/src/cmd/8l/obj.c
File src/cmd/8l/obj.c (right):

https://codereview.appspot.com/7035043/diff/11005/src/cmd/8l/obj.c#newcode82
src/cmd/8l/obj.c:82: debug['I'] = 1; // denote cmdline interpreter override
On 2013/01/04 17:17:55, iant wrote:
> This function occurs three times, and could be moved into ld.

Done.

https://codereview.appspot.com/7035043/diff/11005/src/lib9/flag.c
File src/lib9/flag.c (right):

https://codereview.appspot.com/7035043/diff/11005/src/lib9/flag.c#newcode100
src/lib9/flag.c:100: *(int64*)p = strtoll(s, 0, 0);
On 2013/01/04 17:17:55, iant wrote:
> Check for non-number?

Done.

https://codereview.appspot.com/7035043/diff/11005/src/lib9/flag.c#newcode117
src/lib9/flag.c:117: *(int32*)p = strtol(s, 0, 0);
On 2013/01/04 17:17:55, iant wrote:
> Check for non-number?

Done.

https://codereview.appspot.com/7035043/diff/11005/src/lib9/flag.c#newcode152
src/lib9/flag.c:152: ((void(*)(void))p)();
On 2013/01/04 17:17:55, iant wrote:
> USED(s); ?

Done.

[rsc, 2013-01-06 20:26:27]

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

cmd/5l, cmd/6l, cmd/8l, cmd/cc, cmd/gc: new flag parsing

This CL adds a flag parser that matches the semantics of Go's
package flag. It also changes the linkers and compilers to use
the new flag parser.

Command lines that used to work, like
        8c -FVw
        6c -Dfoo
        5g -I/foo/bar
now need to be split into separate arguments:
        8c -F -V -w
        6c -D foo
        5g -I /foo/bar
The new spacing will work with both old and new tools.

The new parser also allows = for arguments, as in
        6c -D=foo
        5g -I=/foo/bar
but that syntax will not work with the old tools.

In addition to matching standard Go binary flag parsing,
the new flag parser generates more detailed usage messages
and opens the door to long flag names.

The recently added gc flag -= has been renamed -complete.

R=remyoudompheng, daniel.morsing, minux.ma, iant
CC=golang-dev
https://codereview.appspot.com/7035043