Allow cgo to accept multiple .go inputs for a package

Allow cgo to accept multiple .go inputs for a package

Fixes issue 342.

[rsc, 2009-12-15 21:26:26]

looks pretty good.

i'm a bit hesitant to drop down to just one gcc input file.
i think it's possible that we'd end up needing
to compile different files with different
mutually exclusive #includes or #defines.
so probably the gcc input still needs to be per-file
instead of aggregated.

http://codereview.appspot.com/179062/diff/1/2
File src/Make.pkg (right):

http://codereview.appspot.com/179062/diff/1/2#newcode94
src/Make.pkg:94: # and y.go into a package called xy.
s/into a package called xy/when building the package./

(don't mention package name)

http://codereview.appspot.com/179062/diff/1/2#newcode113
src/Make.pkg:113: echo > /dev/null
instead of echo >/dev/null

@true

http://codereview.appspot.com/179062/diff/1/4
File src/cmd/cgo/main.go (right):

http://codereview.appspot.com/179062/diff/1/4#newcode21
src/cmd/cgo/main.go:21: fmt.Fprint(os.Stderr, "usage: cgo [compiler options]
file.go [file.go [file.go...]]\n")
usual syntax is 

[compiler options] file.go ...\n

http://codereview.appspot.com/179062/diff/1/4#newcode47
src/cmd/cgo/main.go:47: // Find first arg that looks like a go file and assume
everything before
Please run the loop backward, just for more safety.

// Assume arguments ending in .go at the end of the list
// are Go files; everything else is gcc command line.

once you find it,

gccOptions, goFiles = args[0:i], args[i:]

[dho, 2009-12-15 21:56:11]

2009/12/15  <rsc@golang.org>:
> looks pretty good.
>
> i'm a bit hesitant to drop down to just one gcc input file.
> i think it's possible that we'd end up needing
> to compile different files with different
> mutually exclusive #includes or #defines.
> so probably the gcc input still needs to be per-file
> instead of aggregated.

Hm, the reason I did this is because of the massive amount of
boilerplate code, but also because duplicated types end up going into
the C files as well. I guess I could keep track o the types that were
written, but part of the change makes it so that all the files use the
same Prog struct, so the Typedef et. al. fields are going to be shared
within files. Perhaps I misunderstood though; is the stuff that goes
in there just from comments?

> http://codereview.appspot.com/179062/diff/1/2
> File src/Make.pkg (right):
>
> http://codereview.appspot.com/179062/diff/1/2#newcode94
> src/Make.pkg:94: # and y.go into a package called xy.
> s/into a package called xy/when building the package./
>
> (don't mention package name)

Done

> http://codereview.appspot.com/179062/diff/1/2#newcode113
> src/Make.pkg:113: echo > /dev/null
> instead of echo >/dev/null
>
> @true

See, I knew there was a better way. Still an ugly target though :(

> http://codereview.appspot.com/179062/diff/1/4
> File src/cmd/cgo/main.go (right):
>
> http://codereview.appspot.com/179062/diff/1/4#newcode21
> src/cmd/cgo/main.go:21: fmt.Fprint(os.Stderr, "usage: cgo [compiler
> options] file.go [file.go [file.go...]]\n")
> usual syntax is
>
> [compiler options] file.go ...\n

Done

> http://codereview.appspot.com/179062/diff/1/4#newcode47
> src/cmd/cgo/main.go:47: // Find first arg that looks like a go file and
> assume everything before
> Please run the loop backward, just for more safety.
>
> // Assume arguments ending in .go at the end of the list
> // are Go files; everything else is gcc command line.
>
> once you find it,
>
> gccOptions, goFiles = args[0:i], args[i:]

I don't think I can do this specifically because args[0] is going to
be /path/to/cgo, and gccOptions may not exist. I guess I can do that
assignment if i > 1 (like I do now), but is keeping the make([]string,
0, 0) still alright for the case where there are no options passed?

--dho

[rsc, 2009-12-15 22:07:50]

On Tue, Dec 15, 2009 at 13:56, Devon H. O'Dell <devon.odell@gmail.com> wrote:
> 2009/12/15  <rsc@golang.org>:
>> looks pretty good.
>>
>> i'm a bit hesitant to drop down to just one gcc input file.
>> i think it's possible that we'd end up needing
>> to compile different files with different
>> mutually exclusive #includes or #defines.
>> so probably the gcc input still needs to be per-file
>> instead of aggregated.
>
> Hm, the reason I did this is because of the massive amount of
> boilerplate code, but also because duplicated types end up going into
> the C files as well. I guess I could keep track o the types that were
> written, but part of the change makes it so that all the files use the
> same Prog struct, so the Typedef et. al. fields are going to be shared
> within files. Perhaps I misunderstood though; is the stuff that goes
> in there just from comments?

I think you might want separate Prog structs too.
At the least you want to clear the C #includes etc. from
one file to the next.  I don't believe there is a massive
amount of boilerplate code.  What are you referring to?

>> http://codereview.appspot.com/179062/diff/1/4#newcode47
>> src/cmd/cgo/main.go:47: // Find first arg that looks like a go file and
>> assume everything before
>> Please run the loop backward, just for more safety.
>>
>> // Assume arguments ending in .go at the end of the list
>> // are Go files; everything else is gcc command line.
>>
>> once you find it,
>>
>> gccOptions, goFiles = args[0:i], args[i:]
>
> I don't think I can do this specifically because args[0] is going to
> be /path/to/cgo, and gccOptions may not exist. I guess I can do that
> assignment if i > 1 (like I do now), but is keeping the make([]string,
> 0, 0) still alright for the case where there are no options passed?

i think you can always use

gccOptions, goFiles := args[0:i], args[i:]

if args[0] is path/to/cgo, then change 0:i to 1:i.

[dho, 2009-12-15 22:17:55]

2009/12/15 Russ Cox <rsc@golang.org>:
> On Tue, Dec 15, 2009 at 13:56, Devon H. O'Dell <devon.odell@gmail.com> wrote:
>> 2009/12/15  <rsc@golang.org>:
>>> looks pretty good.
>>>
>>> i'm a bit hesitant to drop down to just one gcc input file.
>>> i think it's possible that we'd end up needing
>>> to compile different files with different
>>> mutually exclusive #includes or #defines.
>>> so probably the gcc input still needs to be per-file
>>> instead of aggregated.
>>
>> Hm, the reason I did this is because of the massive amount of
>> boilerplate code, but also because duplicated types end up going into
>> the C files as well. I guess I could keep track o the types that were
>> written, but part of the change makes it so that all the files use the
>> same Prog struct, so the Typedef et. al. fields are going to be shared
>> within files. Perhaps I misunderstood though; is the stuff that goes
>> in there just from comments?
>
> I think you might want separate Prog structs too.
> At the least you want to clear the C #includes etc. from
> one file to the next.  I don't believe there is a massive
> amount of boilerplate code.  What are you referring to?

I was referring to gccProlog / builtinProlog, which does define some
types, causing multiple definition issues (though I guess it's not
*that* massive)  I suppose what I could do is have an additional file
for gcc to contain the boilerplate prologues, and have the rest
generated per-file.

I can play with using separate Prog structs if I keep track of which
per-file types are emitted globally with a map. Perhaps that's the way
to go.

>>> http://codereview.appspot.com/179062/diff/1/4#newcode47
>>> src/cmd/cgo/main.go:47: // Find first arg that looks like a go file and

Done

[rsc, 2009-12-15 22:27:56]

> I was referring to gccProlog / builtinProlog, which does define some
> types, causing multiple definition issues (though I guess it's not
> *that* massive)  I suppose what I could do is have an additional file
> for gcc to contain the boilerplate prologues, and have the rest
> generated per-file.

those are C typedefs, which don't make it into the objects.
it's completely fine to duplicate that in each C output file.

russ

[dho, 2009-12-15 22:35:28]

2009/12/15 Russ Cox <rsc@golang.org>:
>> I was referring to gccProlog / builtinProlog, which does define some
>> types, causing multiple definition issues (though I guess it's not
>> *that* massive)  I suppose what I could do is have an additional file
>> for gcc to contain the boilerplate prologues, and have the rest
>> generated per-file.
>
> those are C typedefs, which don't make it into the objects.
> it's completely fine to duplicate that in each C output file.

Alright. Thanks!

> russ
>

[dho, 2009-12-15 23:22:25]

PTAL :)

[rsc, 2009-12-15 23:31:24]

IMPORTANT NOTE:

We just switched gofmt and the tree over to no semicolons.
To avoid a big merge conflict, copy all your changed
files elsewhere, hg revert them, hg sync, and then copy
them back.

You'll want to re-run all.bash too, to get a new gofmt,
and then re-gofmt them.

Sorry for the trouble.

[gri, 2009-12-15 23:43:35]

All relevant CLs are submitted now.
- gri

On Tue, Dec 15, 2009 at 3:31 PM, <rsc@golang.org> wrote:

> IMPORTANT NOTE:
>
> We just switched gofmt and the tree over to no semicolons.
> To avoid a big merge conflict, copy all your changed
> files elsewhere, hg revert them, hg sync, and then copy
> them back.
>
> You'll want to re-run all.bash too, to get a new gofmt,
> and then re-gofmt them.
>
> Sorry for the trouble.
>
>
> http://codereview.appspot.com/179062
>

[dho, 2009-12-16 00:03:56]

No problem, thanks for the heads-up. I'll get this re-submitted later tonight.

--dho

2009/12/15 Robert Griesemer <gri@golang.org>:
> All relevant CLs are submitted now.
> - gri
>
> On Tue, Dec 15, 2009 at 3:31 PM, <rsc@golang.org> wrote:
>>
>> IMPORTANT NOTE:
>>
>> We just switched gofmt and the tree over to no semicolons.
>> To avoid a big merge conflict, copy all your changed
>> files elsewhere, hg revert them, hg sync, and then copy
>> them back.
>>
>> You'll want to re-run all.bash too, to get a new gofmt,
>> and then re-gofmt them.
>>
>> Sorry for the trouble.
>>
>>
>> http://codereview.appspot.com/179062
>
>

[dho, 2009-12-16 02:09:21]

PTAL

[dho, 2009-12-16 15:24:58]

PTAL; updated with new Enum changes.

[rsc, 2009-12-17 20:49:53]

http://codereview.appspot.com/179062/diff/1038/40
File src/Make.pkg (right):

http://codereview.appspot.com/179062/diff/1038/40#newcode43
src/Make.pkg:43: GCC_OFILES=$(patsubst %.go,%.cgo1.o,$(CGOFILES))
Just for my sanity, can we call the c files cgo2.c ?
If Go files compile into .o files too at some point
(like they do under gccgo) then these will collide.

http://codereview.appspot.com/179062/diff/1038/40#newcode148
src/Make.pkg:148: $(TARG).so: $(GCC_OFILES)
I don't believe this will work if TARG is, say, mypackage/stdio.
That's what elem was for.

But this could change to being

_cgo_.so: $(GCC_OFILES)

and the install rule below would be

$(pkgdir)/$(TARG).so: _cgo_.so

[dho, 2009-12-17 20:55:21]

Done and done.

[rsc, 2009-12-17 21:07:56]

LGTM

thanks

[rsc, 2009-12-17 21:20:58]

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

Allow cgo to accept multiple .go inputs for a package

Fixes issue 342.

R=rsc
CC=golang-dev
http://codereview.appspot.com/179062

Committer: Russ Cox <rsc@golang.org>