code review 5487058: cgo: support export for built-in types

cgo: support export for built-in types
This change doesn't pay attention to structs
so they still cannot be exported, see Issue 2552.

Fixes Issue 2462.

[mpimenov, 2011-12-13 17:13:21]

I'm not quite sure about aligning, sorting of goTypes's keys
and whether we should treat error separately from interface.

[dvyukov, 2011-12-13 17:16:45]

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

http://codereview.appspot.com/5487058/diff/5002/src/cmd/cgo/out.go#newcode585
src/cmd/cgo/out.go:585: "rune":       {Size: 4, Align: 4, C: c("int")},
Shouldn't it be int32?

[mpimenov, 2011-12-13 17:32:29]

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

http://codereview.appspot.com/5487058/diff/5002/src/cmd/cgo/out.go#newcode585
src/cmd/cgo/out.go:585: "rune":       {Size: 4, Align: 4, C: c("int")},
On 2011/12/13 17:16:46, dvyukov wrote:
> Shouldn't it be int32?
I think it should behave like Go's int32 which translates to C's int, see below.

[rsc, 2011-12-13 22:03:19]

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

http://codereview.appspot.com/5487058/diff/5002/src/cmd/cgo/out.go#newcode652
src/cmd/cgo/out.go:652: return &Type{Size: 3 * p.PtrSize, Align: p.PtrSize, C:
c("GoInterface")}
A Go interface is two pointers, not three.

[mpimenov, 2011-12-14 08:15:22]

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

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

[mpimenov, 2011-12-14 08:16:50]

Looks like this part of contribution guidelines is wrong:

You will probably revise your code in response to the reviewer comments. When
you have revised the code and are ready for another round of review, run

$ hg mail 99999



sorry for the noise, PTAL

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

http://codereview.appspot.com/5487058/diff/5002/src/cmd/cgo/out.go#newcode652
src/cmd/cgo/out.go:652: return &Type{Size: 3 * p.PtrSize, Align: p.PtrSize, C:
c("GoInterface")}
On 2011/12/13 22:03:20, rsc wrote:
> A Go interface is two pointers, not three.

Done.

[rsc, 2011-12-14 14:40:38]

On Wed, Dec 14, 2011 at 03:16,  <mpimenov@google.com> wrote:
> Looks like this part of contribution guidelines is wrong:
>
> You will probably revise your code in response to the reviewer comments.
> When you have revised the code and are ready for another round of
> review, run
>
> $ hg mail 99999

No, that's how it works.  Did you use clpatch to move
the CL from one machine to another?

[mpimenov, 2011-12-14 15:29:47]

No, I didn't. But iirc first time I mailed it from codereview site
and second time from the command prompt. Looks like it's the reason.


On Wed, Dec 14, 2011 at 6:40 PM, Russ Cox <rsc@golang.org> wrote:

> On Wed, Dec 14, 2011 at 03:16,  <mpimenov@google.com> wrote:
> > Looks like this part of contribution guidelines is wrong:
> >
> > You will probably revise your code in response to the reviewer comments.
> > When you have revised the code and are ready for another round of
> > review, run
> >
> > $ hg mail 99999
>
> No, that's how it works.  Did you use clpatch to move
> the CL from one machine to another?
>

[rsc, 2011-12-14 15:30:51]

On Wed, Dec 14, 2011 at 10:29, Maxim Pimenov <mpimenov@google.com> wrote:
> No, I didn't. But iirc first time I mailed it from codereview site
> and second time from the command prompt. Looks like it's the reason.

Indeed.  You should use hg mail for the first mail too.
It takes care of making sure golang-dev is cc'ed and
so on.

Russ

[mpimenov, 2011-12-16 15:31:54]

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

Please take another look.

[rsc, 2011-12-16 15:48:43]

Looks fine to me.  Ian wrote the export support
originally, so leaving for him.

[iant, 2011-12-20 17:28:30]

LGTM

[iant, 2011-12-20 17:28:48]

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

cgo: support export for built-in types
This change doesn't pay attention to structs
so they still cannot be exported, see Issue 2552.

Fixes Issue 2462.

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

Committer: Ian Lance Taylor <iant@golang.org>