Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/cgo: Treat typedefs of primitives as identical types in the same compilation unit. #21809

Closed
asottile opened this issue Sep 8, 2017 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@asottile
Copy link
Contributor

asottile commented Sep 8, 2017

This is a spinoff from #13467

Here's a "minimal" reproduction, I'll include my concrete usecase below.

Consider two vendors which provide equivalent implementations of ~*~something~*~ -- in this toy example, they are providing a function called My_foo

vendora/include/foo.h

typedef long My_ssize_t;

void My_foo(My_ssize_t);

vendora/src/foo.c

#include "foo.h"

void My_foo(My_ssize_t x) {}

vendorb/include/foo.h

typedef long My_ssize_t;

void My_foo(long);

vendorb/src/foo.c

#include "foo.h"

void My_foo(long x) {}

differences

So you don't have to play "spot the differences", both vendors provide a typedef for My_ssize_t as an alias to long. The two vendors define equivalent functions, but one vendor chose to use long in the function definition and the other chose to use the typedef My_ssize_t.

compiling the c bits to shared libraries:

gcc -shared -Ivendora/include -o vendora/libfoo.so vendora/src/foo.c
gcc -shared -Ivendorb/include -o vendorb/libfoo.so vendorb/src/foo.c

main.go

package main

// #include "foo.h"
import "C"
import "fmt"

func main() {
	C.My_foo(C.My_ssize_t(9001))
	fmt.Println("It worked!")
}

Running (compiling) against vendora

$ LD_LIBRARY_PATH=$PWD/vendora \
>     CGO_CFLAGS=-I$PWD/vendora/include \
>     CGO_LDFLAGS="-L$PWD/vendora -lfoo" \
>     go run main.go
It worked!

Running (compiling) against vendorb

$ LD_LIBRARY_PATH=$PWD/vendorb \
>     CGO_CFLAGS=-I$PWD/vendorb/include \
>     CGO_LDFLAGS="-L$PWD/vendorb -lfoo" \
>     go run main.go
# command-line-arguments
./main.go:8: cannot use C.My_ssize_t(9001) (type C.My_ssize_t) as type C.long in argument to _Cfunc_My_foo

Here I expect the same output as running (compiling) against vendora

My description of the problem

Despite My_ssize_t and long being identical C types in the same module, they are treated as different go types.

My concrete problem

My concrete problem actually has to do with interfacing with CPython / pypy, you can read more about it in the other issue: #13467 (comment)

My thoughts

I'm not a seasoned go developer, but my initial inclination that would at least solve my problem would be to make typedef primitive x into type aliases in cgo -- is this reasonable? where do I start hacking 😄

@AlexRouSg
Copy link
Contributor

The quickest way to get this working now would be to add a wrapper function. Something like...

package main
// #include "foo.h"
// void Wrapper_foo(long x) {
//     My_foo(x);
// }
import "C"
import "fmt"

func main() {
	C.Wrapper_foo(C.long(9001))
	fmt.Println("It worked!")
}

@bcmills
Copy link
Contributor

bcmills commented Sep 8, 2017

my initial inclination that would at least solve my problem would be to make typedef primitive x into type aliases in cgo -- is this reasonable?

Yep!

where do I start hacking 😄

https://golang.org/doc/contribute.html has instructions for setting up. The cgo implementation is in src/cmd/cgo and the integration tests are in misc/cgo.

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Sep 8, 2017
@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 8, 2017
@asottile
Copy link
Contributor Author

I think I did this right? https://go-review.googlesource.com/62670

One oddity of my change that I'm not sure if it's ok:

  • Simple c types that map directly to go primitives (such as C.uchar and uint8) are now aliases

This (imo) seems like a good thing as it reduces the amount of boilerlate for writing C interop, but it did change some of the error messages produced in the incorrect case (see the test I had to modify).

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/62670 mentions this issue: cmd/cgo: Treat simple C typedefs as go aliases

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/63276 mentions this issue: misc/cgo/errors: port test.bash to Go

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/63277 mentions this issue: cmd/cgo: use type aliases for primitive types

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants