code review 5797046: cmd/cgo: add support for function export for gccgo.

cmd/cgo: add support for function export for gccgo.

A "gccgoprefix" flag is added and used by the go tool,
to mirror the -fgo-prefix flag for gccgo, whose value
is required to know how to access functions from C.

Trying to export Go methods or unexported Go functions
will not work.

Also fix go test on "main" packages.

Updates issue 2313.
Fixes issue 3262.

[remyoudompheng, 2012-03-09 20:58:38]

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

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

[mpimenov, 2012-03-11 12:14:18]

in CL description:

s/mathods/methods/

>> As cgo needs to work what go-prefix is used by gccgo
I can't understand this line.

[remyoudompheng, 2012-03-11 12:40:52]

Sorry, I wrote too quick, updated the description.

[mpimenov, 2012-03-11 12:50:33]

Still not that slow :)

s/funcctions/functions/

On Sun, Mar 11, 2012 at 3:40 PM, <remyoudompheng@gmail.com> wrote:

> Sorry, I wrote too quick, updated the description.
>
>
http://codereview.appspot.com/**5797046/<http://codereview.appspot.com/5797046/>
>

[rsc, 2012-03-12 19:25:44]

Seems plausible; Ian should look too.

http://codereview.appspot.com/5797046/diff/4001/src/cmd/go/build.go
File src/cmd/go/build.go (right):

http://codereview.appspot.com/5797046/diff/4001/src/cmd/go/build.go#newcode1308
src/cmd/go/build.go:1308: case p.Name == "main":
Can we fix issue 3262 while we are here?  I think this should be

case p.build.IsCommand() && !p.forceLibrary:

and then we can add a forceLibrary bool to type Package,
    forceLibrary bool  // this is a library, even if named package main

and then go test can set forceLibrary = true for the ptest package.

Russ

[iant, 2012-03-12 20:24:40]

http://codereview.appspot.com/5797046/diff/4001/src/cmd/cgo/out.go
File src/cmd/cgo/out.go (right):

http://codereview.appspot.com/5797046/diff/4001/src/cmd/cgo/out.go#newcode649
src/cmd/cgo/out.go:649: if fntype.Results.NumFields() > 0 {
You should only declare a struct to hold results if there is more than one
result.  Otherwise gccgo will return a straight scalar value.  This matters for
ABIs in which small structs are not returned in registers.

That is, s/> 0/> 1/

http://codereview.appspot.com/5797046/diff/4001/src/cmd/cgo/out.go#newcode654
src/cmd/cgo/out.go:654: fmt.Fprintf(fgcch, "\t\t%s r%d;\n", t.C, i)
Why two tabs here?

http://codereview.appspot.com/5797046/diff/4001/src/cmd/cgo/out.go#newcode668
src/cmd/cgo/out.go:668: if !ast.IsExported(exp.Func.Name.Name) {
The not-exported case won't work because the function will be static and may not
even be emitted if it is never called or always inlined.

http://codereview.appspot.com/5797046/diff/4001/src/cmd/go/build.go
File src/cmd/go/build.go (right):

http://codereview.appspot.com/5797046/diff/4001/src/cmd/go/build.go#newcode1305
src/cmd/go/build.go:1305: return -1
This isn't the same mapping that gccgo applies to the -fgo-prefix option.  To
act the same as gccgo means turning "case r == '.':" to "default:".  This
matters when the prefix contains, e.g., '/' characters, as is normal, since the
docs encourage passing the installed name of the library as the -fgo-prefix
option.

[remyoudompheng, 2012-03-14 22:07:28]

http://codereview.appspot.com/5797046/diff/4001/src/cmd/cgo/out.go
File src/cmd/cgo/out.go (right):

http://codereview.appspot.com/5797046/diff/4001/src/cmd/cgo/out.go#newcode649
src/cmd/cgo/out.go:649: if fntype.Results.NumFields() > 0 {
On 2012/03/12 20:24:40, iant wrote:
> You should only declare a struct to hold results if there is more than one
> result.  Otherwise gccgo will return a straight scalar value.  This matters
for
> ABIs in which small structs are not returned in registers.
> 
> That is, s/> 0/> 1/

Done.

http://codereview.appspot.com/5797046/diff/4001/src/cmd/cgo/out.go#newcode654
src/cmd/cgo/out.go:654: fmt.Fprintf(fgcch, "\t\t%s r%d;\n", t.C, i)
On 2012/03/12 20:24:40, iant wrote:
> Why two tabs here?

Done.

http://codereview.appspot.com/5797046/diff/4001/src/cmd/cgo/out.go#newcode668
src/cmd/cgo/out.go:668: if !ast.IsExported(exp.Func.Name.Name) {
On 2012/03/12 20:24:40, iant wrote:
> The not-exported case won't work because the function will be static and may
not
> even be emitted if it is never called or always inlined.

Done.

http://codereview.appspot.com/5797046/diff/4001/src/cmd/go/build.go
File src/cmd/go/build.go (right):

http://codereview.appspot.com/5797046/diff/4001/src/cmd/go/build.go#newcode1305
src/cmd/go/build.go:1305: return -1
On 2012/03/12 20:24:40, iant wrote:
> This isn't the same mapping that gccgo applies to the -fgo-prefix option.  To
> act the same as gccgo means turning "case r == '.':" to "default:".  This
> matters when the prefix contains, e.g., '/' characters, as is normal, since
the
> docs encourage passing the installed name of the library as the -fgo-prefix
> option.

Done.

http://codereview.appspot.com/5797046/diff/4001/src/cmd/go/build.go#newcode1308
src/cmd/go/build.go:1308: case p.Name == "main":
On 2012/03/12 19:25:44, rsc wrote:
> Can we fix issue 3262 while we are here?  I think this should be
> 
> case p.build.IsCommand() && !p.forceLibrary:
> 
> and then we can add a forceLibrary bool to type Package,
>     forceLibrary bool  // this is a library, even if named package main
> 
> and then go test can set forceLibrary = true for the ptest package.
> 
> Russ

Done.

[remyoudompheng, 2012-03-14 22:07:42]

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

Please take another look.

[rsc, 2012-03-15 18:56:02]

LGTM but wait for iant

[iant, 2012-03-15 22:21:11]

LGTM

[iant, 2012-03-15 22:21:22]

LGTM

Thanks for tackling this.

[remyoudompheng, 2012-03-15 22:50:51]

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

cmd/cgo: add support for function export for gccgo.

A "gccgoprefix" flag is added and used by the go tool,
to mirror the -fgo-prefix flag for gccgo, whose value
is required to know how to access functions from C.

Trying to export Go methods or unexported Go functions
will not work.

Also fix go test on "main" packages.

Updates issue 2313.
Fixes issue 3262.

R=mpimenov, rsc, iant
CC=golang-dev
http://codereview.appspot.com/5797046