code review 9574043: cgo: do not output empty struct

cmd/cgo: do not output empty struct for -cdefs

[brainman, 2013-05-20 07:37:34]

Hello golang-dev@googlegroups.com,

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

[iant, 2013-05-22 18:26:36]

What empty struct is avoided in the regenerated output files?

[brainman, 2013-05-23 02:42:50]

These are the diffs for defs_windows_386.h (and defs_windows_amd64.h) files
generated with current cgo command and proposed changed one:

# diff defs_windows_amd64.h ~/cl9574043/src/pkg/runtime/defs_windows_amd64.h    
                                                                               
64,65d63                                                                        
     
< struct FloatingSaveArea {                                                     
     
< };                                                                            
     
# diff defs_windows_386.h ~/cl9574043/src/pkg/runtime/defs_windows_386.h  
74,75d73                                                                        
     
< struct M128a {                                                                
     
< };                                                                            
     
#                                                                             

It seems FloatingSaveArea and M128a are ARCH specific. They are not used
directly by our code, but they are embeded in other structs. They are present in
defs_windows.go, because cgo doesn't do recursion to discover embeded structs.

I suppose, FloatingSaveArea and M128a can be moved into ARCH specific file like
defs_windows_386.go and defs_windows_amd64.go, but I think it just complicates
things more.

I can just abandon this CL altogether. But I need more windows C structs for net
poller, and it bothers me that auto-generation does not work.

Alex

[iant, 2013-05-23 04:40:47]

Are the empty structs a problem?

[brainman, 2013-05-23 04:41:32]

On 2013/05/23 04:40:47, iant wrote:
> Are the empty structs a problem?

Yes. These fail to compile.

Alex

[iant, 2013-05-23 04:50:42]

On 2013/05/23 04:41:32, brainman wrote:
> On 2013/05/23 04:40:47, iant wrote:
> > Are the empty structs a problem?
> 
> Yes. These fail to compile.

GCC permits empty structs, although I don't think they are in standard C.  Are
they failing with a non-GCC compiler?

[iant, 2013-05-23 04:51:19]

On a separate topic, the changes to the defs files are large and in general have
nothing to do with the change to cgo.  Let's separate this CL from the CL that
regenerates those files.

[brainman, 2013-05-23 04:55:20]

On 2013/05/23 04:50:42, iant wrote:
> 
> GCC permits empty structs, although I don't think they are in standard C.  Are
> they failing with a non-GCC compiler?

Yes. They fail with 8c:

C:\go\root\src\pkg\runtime>go tool cgo -cdefs defs_windows.go >
defs_windows_386.h

C:\go\root\src\pkg\runtime>go install -v
runtime
# runtime
C:\DOCUME~1\brainman\LOCALS~1\Temp\go-build165623463\runtime\_obj\/defs_GOOS_GOARCH.h:75
c:\go\root\src\pkg\runtime\callback_windows_386.c:8 syntax error, lastname:
M128a

Alex

[brainman, 2013-05-23 04:56:05]

On 2013/05/23 04:51:19, iant wrote:
> On a separate topic, the changes to the defs files are large and in general
have
> nothing to do with the change to cgo.  Let's separate this CL from the CL that
> regenerates those files.

Fair enough. I will remove them. But let's decide what to do first.

Alex

[iant, 2013-05-23 04:58:43]

On 2013/05/23 04:56:05, brainman wrote:
> On 2013/05/23 04:51:19, iant wrote:
> > On a separate topic, the changes to the defs files are large and in general
> have
> > nothing to do with the change to cgo.  Let's separate this CL from the CL
that
> > regenerates those files.
> 
> Fair enough. I will remove them. But let's decide what to do first.

I'm fine with omitting empty structs from the generated C code.  They are
unlikely to be useful.

[brainman, 2013-05-23 07:06:31]

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

Please take another look.

[brainman, 2013-05-23 07:07:24]

On 2013/05/23 04:51:19, iant wrote:
> ... Let's separate this CL from the CL that
> regenerates those files.

Done https://codereview.appspot.com/9679046/

Alex

[iant, 2013-05-24 20:48:08]

LGTM

Before submitting please change CL description to

cmd/cgo: do not output empty struct for -cdefs

[brainman, 2013-05-25 10:54:04]

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

cmd/cgo: do not output empty struct for -cdefs

R=golang-dev, iant
CC=golang-dev
https://codereview.appspot.com/9574043