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: uint64_t used to permit C.ulong, but no longer does #31093

Closed
ianlancetaylor opened this issue Mar 27, 2019 · 4 comments

Comments

@ianlancetaylor
Copy link
Contributor

commented Mar 27, 2019

This program works fine with Go 1.12.

package main

// #include <stdint.h>
// static uint64_t foo(uint64_t v) { return v; }
import "C"

import "fmt"

func main() {
	i := 0
	j := C.foo(C.ulong(i))
	fmt.Printf("%T %d\n", j, j)
}

With current tip, it fails to build with

# command-line-arguments
foo.go:11:20: cannot use _Ctype_ulong(i) (type _Ctype_ulong) as type uint64 in argument to _Cfunc_foo

This is a consequence of the change for #29878. We need to address it one way or another.

CC @phst

@rsc

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2019

I am having a similar problem in a cgo program of mine that built in Go 1.12.
The C code contains

typedef uint8_t other_t;

and I used to be able to pass a Go *C.uchar to a C function expecting other_t*.
Now that code does not compile.

The good news is that if I fix the code to use *C.other_t then it compiles
both with current master and Go 1.12.

I guess the first question is whether we think cgo should be this picky.
Maybe if we were starting from scratch.
Ten years in, I am reluctant to break old code.

Should we tell cgo the go version from the go.mod file
and only do this for explicit go1.13 or later?

Or should we roll back #29878?
I mean, there are zero systems where
char, short, int, and long long are not 8, 16, 32, and 64 bits.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2019

I agree that cgo should not be this picky. I haven't thought about whether there is a way to fix this, or whether we need to roll back. If rolling back is the only way to let existing code continue to build, then I think we should roll back.

@gopherbot

This comment has been minimized.

Copy link

commented Jun 4, 2019

Change https://golang.org/cl/180357 mentions this issue: cmd/cgo: roll back "use C exact-width integer types to represent Go types"

@gopherbot

This comment has been minimized.

Copy link

commented Jun 4, 2019

Change https://golang.org/cl/180377 mentions this issue: misc/cgo/test: add test for issue 31093

@gopherbot gopherbot closed this in ac921da Jun 5, 2019

gopherbot pushed a commit that referenced this issue Jun 5, 2019
misc/cgo/test: add test for issue 31093
Updates #31093

Change-Id: I7962aaca0b012de01768b7b42dc2283d5845eeea
Reviewed-on: https://go-review.googlesource.com/c/go/+/180377
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.