code review 4964046: src/cmd/gc/*: Adjustments for compilation using Plan 9 ...

src/cmd/gc/*: Adjustments for compilation using Plan 9 tool chain.

gc/bits.c
. improved format with associated cast;
gc/closure.c
gc/dcl.c
gc/range.c
gc/reflect.c
gc/sinit.c
. dropped unnecessary assignments;
gc/gen.c
. dropped unnecessary assignment;
. added static qualifier to local function definition;
gc/go.h
. added varargck pragmas;
gc/lex.c
. used {} instead of ; in if statement to suppress warning;
. replaced exit(0) with exits(0);
. added compilation conditions for SIGBUS/SIGSEGV;
. dropped unnecessary assignment;
gc/mparith2.c
. dropped four unnecessary assignments/initialisations;
gc/obj.c
. added type cast to local pointer;
gc/pgen.c
. added cast and related print format;
gc/subr.c
. replaced exit(1) with exits("error");
. replaced unlink() with remove();
. renamed local cistrmp() as ucistrmp() to remove conflict with
  Plan 9 function by the same name;
gc/swt.c
. added braces instead of ; as empty statment;
gc/typecheck.c
. added static qualifier to local function definition;
. dropped unnecessary assignments;
gc/walk.c
. dropped unnecessary assignments;
. added static qualifier to local function definitions;

[lucio, 2011-08-26 21:14:57]

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

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

[rsc, 2011-08-26 21:38:18]

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/esc.c
File src/cmd/gc/esc.c (right):

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/esc.c#newcode1
src/cmd/gc/esc.c:1: // Copyright 2011 The Go Authors. All rights reserved.
please revert all changes to esc.c.
you can send a separate CL for esc.c
once the current work in esc.c has
stopped.

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/go.h
File src/cmd/gc/go.h (right):

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/go.h#newcode5
src/cmd/gc/go.h:5: #pragma	incomplete _off_
Huh?
This looks like it is saying that _off_ is incomplete.
That can't be what you mean.
Or does this turn off all checking?
I'd rather not do that either.
Please remove for this CL.  We can come back to it.

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/lex.c
File src/cmd/gc/lex.c (left):

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/lex.c#oldcode456
src/cmd/gc/lex.c:456: 
Why delete a blank line?

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/obj.c
File src/cmd/gc/obj.c (right):

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/obj.c#newcode273
src/cmd/gc/obj.c:273: snprint(namebuf, sizeof(namebuf), "\"%Z\"", (Strlit
*)&tmp);
s/(Strlit *)&tmp/&tmp.lit/

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/sinit.c
File src/cmd/gc/sinit.c (left):

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/sinit.c#oldcode527
src/cmd/gc/sinit.c:527: ctxt = 0;
Deleting this line, even though it is suspicious,
is a compiler change.  Please don't do that.
I think you meant to write

USED(ctxt);
ctxt = 0;

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/subr.c
File src/cmd/gc/subr.c (right):

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/subr.c#newcode2834
src/cmd/gc/subr.c:2834: * renamed to avoid conflict with Plan 9 instance
delete line

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/subr.c#newcode2838
src/cmd/gc/subr.c:2838: gccistrcmp(char *p, char *q)
ucistrcmp please

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/typecheck.c
File src/cmd/gc/typecheck.c (left):

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/typecheck.c#oldcod...
src/cmd/gc/typecheck.c:1076: op = OCONV;
This line is supposed to say n->op = OCONV;

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/walk.c
File src/cmd/gc/walk.c (left):

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/walk.c#oldcode1175
src/cmd/gc/walk.c:1175: fatal("missing switch %O", n->op);
Then you can put this line back.
I know it's redundant but the switch is so large,
I like having a backstop just in case it ever
doesn't goto ret.

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/walk.c
File src/cmd/gc/walk.c (right):

http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/walk.c#newcode387
src/cmd/gc/walk.c:387: goto ret;
s/goto ret/break/

[lucio, 2011-08-26 22:33:19]

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

Please take another look.

[lucio, 2011-08-26 22:33:37]

I'm concerned that the code may not compile under plan 9 because of
incomplete structures, but right now I don't have enough time to deal
with that.  As you suggest, we can get back to it.

Everything else seems OK, but it's late here, I may be a bit too tired
to pick up all the possible problems.  Further comments below.

On 8/26/11, rsc@golang.org <rsc@golang.org> wrote:
>
> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/esc.c
> File src/cmd/gc/esc.c (right):
>
> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/esc.c#newcode1
> src/cmd/gc/esc.c:1: // Copyright 2011 The Go Authors. All rights
> reserved.
> please revert all changes to esc.c.
> you can send a separate CL for esc.c
> once the current work in esc.c has
> stopped.
>
There was just a redundant assignment, reverted.

> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/go.h
> File src/cmd/gc/go.h (right):
>
> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/go.h#newcode5
> src/cmd/gc/go.h:5: #pragma	incomplete _off_
> Huh?
> This looks like it is saying that _off_ is incomplete.

No, the book says that's how checking is turned off.

> That can't be what you mean.
> Or does this turn off all checking?
> I'd rather not do that either.
> Please remove for this CL.  We can come back to it.
>
I fear we'll have to, but I am not going to check right now.

> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/lex.c
> File src/cmd/gc/lex.c (left):
>
> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/lex.c#oldcode456
> src/cmd/gc/lex.c:456:
> Why delete a blank line?
>
Because it looked a little lost?  Restored, hopefully in the right place.

> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/obj.c
> File src/cmd/gc/obj.c (right):
>
> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/obj.c#newcode273
> src/cmd/gc/obj.c:273: snprint(namebuf, sizeof(namebuf), "\"%Z\"",
> (Strlit *)&tmp);
> s/(Strlit *)&tmp/&tmp.lit/
>
I didn't figure that one out, thanks for the clarification.

> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/sinit.c
> File src/cmd/gc/sinit.c (left):
>
> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/sinit.c#oldcode527
> src/cmd/gc/sinit.c:527: ctxt = 0;
> Deleting this line, even though it is suspicious,
> is a compiler change.  Please don't do that.
> I think you meant to write
>
> USED(ctxt);
> ctxt = 0;
>
It seems more consistent the other way around, but restored as
requested.  It is suspicious.

> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/subr.c
> File src/cmd/gc/subr.c (right):
>
> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/subr.c#newcode2834
> src/cmd/gc/subr.c:2834: * renamed to avoid conflict with Plan 9 instance
> delete line
>
Done.

> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/subr.c#newcode2838
> src/cmd/gc/subr.c:2838: gccistrcmp(char *p, char *q)
> ucistrcmp please
>
Done.

> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/typecheck.c
> File src/cmd/gc/typecheck.c (left):
>
>
http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/typecheck.c#oldcod...
> src/cmd/gc/typecheck.c:1076: op = OCONV;
> This line is supposed to say n->op = OCONV;
>
Done.

> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/walk.c
> File src/cmd/gc/walk.c (left):
>
> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/walk.c#oldcode1175
> src/cmd/gc/walk.c:1175: fatal("missing switch %O", n->op);
> Then you can put this line back.
> I know it's redundant but the switch is so large,
> I like having a backstop just in case it ever
> doesn't goto ret.
>
Not very clear, please make sure I followed your instructions correctly.

> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/walk.c
> File src/cmd/gc/walk.c (right):
>
> http://codereview.appspot.com/4964046/diff/4001/src/cmd/gc/walk.c#newcode387
> src/cmd/gc/walk.c:387: goto ret;
> s/goto ret/break/
>
Done.

> http://codereview.appspot.com/4964046/
>
Lucio.

[lucio, 2011-08-27 16:47:50]

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

Please take another look.

[rsc, 2011-08-29 13:09:00]

LGTM

[rsc, 2011-08-29 13:35:07]

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

gc: fix build on Plan 9

gc/bits.c
. improved format with associated cast;
gc/closure.c
gc/dcl.c
gc/range.c
gc/reflect.c
gc/sinit.c
. dropped unnecessary assignments;
gc/gen.c
. dropped unnecessary assignment;
. added static qualifier to local function definition;
gc/go.h
. added varargck pragmas;
gc/lex.c
. used {} instead of ; in if statement to suppress warning;
. replaced exit(0) with exits(0);
. added compilation conditions for SIGBUS/SIGSEGV;
. dropped unnecessary assignment;
gc/mparith2.c
. dropped four unnecessary assignments/initialisations;
gc/obj.c
. added type cast to local pointer;
gc/pgen.c
. added cast and related print format;
gc/subr.c
. replaced exit(1) with exits("error");
. replaced unlink() with remove();
. renamed local cistrmp() as ucistrmp() to remove conflict with
  Plan 9 function by the same name;
gc/swt.c
. added braces instead of ; as empty statment;
gc/typecheck.c
. added static qualifier to local function definition;
. dropped unnecessary assignments;
gc/walk.c
. dropped unnecessary assignments;
. added static qualifier to local function definitions;

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

Committer: Russ Cox <rsc@golang.org>