code review 140740043: runtime: include constants and defs_*_*.h types in gene...

runtime: include constants and defs_*_*.h types in generated Go defs

I had to rename Kevent and Sigaction to avoid the functions of the
same (lowercase) name.

[rsc, 2014-08-29 19:20:50]

Hello golang-codereviews@googlegroups.com (cc: dvyukov, iant, khr, r),

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

[r, 2014-08-29 19:24:09]

this is going to put lots more symbols into godoc.

[r, 2014-08-29 19:24:26]

i think

[r, 2014-08-29 19:25:35]

let me rephrase

these items will be turned into Go, right? and then they will be
published as exported names.
perhaps we should figure out how to manage that first.

[iant, 2014-08-29 19:33:26]

LGTM

[aram2, 2014-08-29 19:33:42]

On Fri, Aug 29, 2014 at 9:25 PM, Rob Pike <r@golang.org> wrote:
> these items will be turned into Go, right? and then they will be
> published as exported names.
> perhaps we should figure out how to manage that first.

I used _Symbol for what I needed so far; manually written, so this
change is greatly welcomed.

-- 
Aram Hăvărneanu

[iant, 2014-08-29 19:33:58]

On 2014/08/29 19:25:35, r wrote:
> let me rephrase
> 
> these items will be turned into Go, right? and then they will be
> published as exported names.
> perhaps we should figure out how to manage that first.

The new names are not exported as far as I can see.

Names with a leading upper case letter are prefixed with underscore.

[r, 2014-08-29 19:38:25]

defs2_linux.go:140: type Sigaction C.struct_kernel_sigaction

no leading underscore

[iant, 2014-08-29 19:43:29]

On 2014/08/29 19:38:25, r wrote:
> defs2_linux.go:140: type Sigaction C.struct_kernel_sigaction
> 
> no leading underscore

True, but not affected by this CL.

[iant, 2014-08-29 19:45:09]

On 2014/08/29 19:43:29, iant wrote:
> On 2014/08/29 19:38:25, r wrote:
> > defs2_linux.go:140: type Sigaction C.struct_kernel_sigaction
> > 
> > no leading underscore
> 
> True, but not affected by this CL.

That is, this CL just affects generated const declarations, and it adds an
underscore prefix when a const declaration starts with an upper case letter.

(Except for renaming a couple of types.)

[rsc, 2014-08-29 19:47:16]

The process is a bit subtle, but no, there are no new symbols being added
to package runtime.

The defs_*.go files are +build ignore inputs to cgo and are used to
generate defs_linux_amd64.h etc. They are not compiled into the package at
all.

This change adds #include "defs_GOOS_GOARCH.h" to defs.c, so that 6c's "C
types to Go types" generator will generate definitions for all the things
in defs_GOOS_GOARCH.h. But when it does, it converts everything to lower
case.

The defs2_linux.go
    type Sigaction C.struct_kernel_sigaction
was used to generate defs_linux_amd64.h, which contains
    typedef struct Sigaction Sigaction;
    struct Sigaction {
       ...
    };
and then 6c is used by cmd/dist at all.bash time to turn that into a Go
definition in zdefs_linux_amd64.go:
    type sigaction struct {
        ...
    }
All these types have always been lower case. Sigaction is newly included,
but still lower case. I renamed Sigaction to SigactionT in this CL so that
the lowercase version does not conflict with the already existing function
named sigaction.

The 6c converter now also emits constants; cmd/dist inserts a leading _ on
the ones that are upper case (most of them). So _EINTR, etc.

In both cases, no new exported names.

[r, 2014-08-29 19:53:40]

LGTM other than malloc.go

https://codereview.appspot.com/140740043/diff/40001/src/pkg/runtime/malloc.go
File src/pkg/runtime/malloc.go (right):

https://codereview.appspot.com/140740043/diff/40001/src/pkg/runtime/malloc.go...
src/pkg/runtime/malloc.go:29: maxGCMask       = 64
is this file related?

[rsc, 2014-08-29 19:57:53]

https://codereview.appspot.com/140740043/diff/40001/src/pkg/runtime/malloc.go
File src/pkg/runtime/malloc.go (right):

https://codereview.appspot.com/140740043/diff/40001/src/pkg/runtime/malloc.go...
src/pkg/runtime/malloc.go:29: maxGCMask       = 64
On 2014/08/29 19:53:40, r wrote:
> is this file related?

Yes, the two lines being deleted are duplicating definitions that are now
automatically generated from C.

[rsc, 2014-08-29 20:00:33]

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

runtime: include constants and defs_*_*.h types in generated Go defs

I had to rename Kevent and Sigaction to avoid the functions of the
same (lowercase) name.

LGTM=iant, r
R=golang-codereviews, r, iant, aram.h
CC=dvyukov, golang-codereviews, khr
https://codereview.appspot.com/140740043

[gobot, 2014-08-29 20:01:35]

This CL appears to have broken the android-arm-crawshaw builder.
See http://build.golang.org/log/27448d314fbc31db3e1ad88ee604453b343f369c

[crawshaw, 2014-08-29 20:06:33]

Real:

/tmp/gobuilder/android-arm-crawshaw-91378329bf67/go/src/pkg/runtime/zruntime_defs_linux_amd64.go:972:
sigaltstack redeclared in this block
previous declaration at
/tmp/gobuilder/android-arm-crawshaw-91378329bf67/go/src/pkg/runtime/os_linux.go:12


On Fri, Aug 29, 2014 at 4:01 PM,  <gobot@golang.org> wrote:
> This CL appears to have broken the android-arm-crawshaw builder.
> See http://build.golang.org/log/27448d314fbc31db3e1ad88ee604453b343f369c
>
>
> https://codereview.appspot.com/140740043/
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.