code review 4528124: cgo: support non intel gcc machine flags

cgo: support non intel gcc machine flags

[dave_cheney.net, 2011-06-07 05:53:54]

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

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

[adg, 2011-06-07 06:03:18]

http://codereview.appspot.com/4528124/diff/5001/src/cmd/cgo/gcc.go
File src/cmd/cgo/gcc.go (right):

http://codereview.appspot.com/4528124/diff/5001/src/cmd/cgo/gcc.go#newcode663
src/cmd/cgo/gcc.go:663: func (p *Package) gccMachine(args ...string) []string {
There's no reason this should take args. Make it
func (p *Package) gccMachine() []string

http://codereview.appspot.com/4528124/diff/5001/src/cmd/cgo/gcc.go#newcode678
src/cmd/cgo/gcc.go:678: c := append(p.gccMachine(p.gccName()),
Leave this the way it was

http://codereview.appspot.com/4528124/diff/5001/src/cmd/cgo/gcc.go#newcode687
src/cmd/cgo/gcc.go:687: c = append(c, p.GccOptions...)
Then, below this line, add
c = append(c, p.gccMachine()...)

http://codereview.appspot.com/4528124/diff/5001/src/cmd/cgo/gcc.go#newcode723
src/cmd/cgo/gcc.go:723: base := append(p.gccMachine(p.gccName()), "-E", "-dM",
"-xc")
base := []string{p.gccName(), "-E", "-dM", "-xc"}
base = append(base, p.gccMachine()...)

[dave_cheney.net, 2011-06-07 06:11:38]

Thanks for your feedback, PTAL.

http://codereview.appspot.com/4528124/diff/5001/src/cmd/cgo/gcc.go
File src/cmd/cgo/gcc.go (right):

http://codereview.appspot.com/4528124/diff/5001/src/cmd/cgo/gcc.go#newcode663
src/cmd/cgo/gcc.go:663: func (p *Package) gccMachine(args ...string) []string {
That was my other option, I'll do another pass.

On 2011/06/07 06:03:19, adg wrote:
> There's no reason this should take args. Make it
> func (p *Package) gccMachine() []string
> 
>

http://codereview.appspot.com/4528124/diff/5001/src/cmd/cgo/gcc.go#newcode678
src/cmd/cgo/gcc.go:678: c := append(p.gccMachine(p.gccName()),
On 2011/06/07 06:03:19, adg wrote:
> Leave this the way it was

Done.

http://codereview.appspot.com/4528124/diff/5001/src/cmd/cgo/gcc.go#newcode687
src/cmd/cgo/gcc.go:687: c = append(c, p.GccOptions...)
I'm fine with this if you're happy that the position of -m is no longer first

http://codereview.appspot.com/4528124/diff/5001/src/cmd/cgo/gcc.go#newcode723
src/cmd/cgo/gcc.go:723: base := append(p.gccMachine(p.gccName()), "-E", "-dM",
"-xc")
On 2011/06/07 06:03:19, adg wrote:
> base := []string{p.gccName(), "-E", "-dM", "-xc"}
> base = append(base, p.gccMachine()...)

Done.

[adg, 2011-06-07 06:43:22]

LGTM

http://codereview.appspot.com/4528124/diff/9001/src/cmd/cgo/gcc.go
File src/cmd/cgo/gcc.go (right):

http://codereview.appspot.com/4528124/diff/9001/src/cmd/cgo/gcc.go#newcode670
src/cmd/cgo/gcc.go:670: return []string{}
return nil

[dave_cheney.net, 2011-06-07 06:53:05]

Thanks again. PTAL

http://codereview.appspot.com/4528124/diff/9001/src/cmd/cgo/gcc.go
File src/cmd/cgo/gcc.go (right):

http://codereview.appspot.com/4528124/diff/9001/src/cmd/cgo/gcc.go#newcode670
src/cmd/cgo/gcc.go:670: return []string{}
Done, thanks, that makes sense.

[adg, 2011-06-07 06:59:45]

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

cgo: support non intel gcc machine flags

R=adg
CC=golang-dev
http://codereview.appspot.com/4528124

Committer: Andrew Gerrand <adg@golang.org>