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.

Sign up for GitHub

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

Jump to bottom

cmd/cgo: uint64_t used to permit C.ulong, but no longer does #31093

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

cmd/cgo: uint64_t used to permit C.ulong, but no longer does #31093

ianlancetaylor opened this issue Mar 27, 2019 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.13

Comments

@ianlancetaylor
Copy link
Member

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
@ianlancetaylor ianlancetaylor added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Mar 27, 2019
@ianlancetaylor ianlancetaylor added this to the Go1.13 milestone Mar 27, 2019
@ianlancetaylor ianlancetaylor mentioned this issue Apr 22, 2019
Closed
@rsc
Copy link
Contributor

rsc 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
Copy link
Member Author

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.

@ianlancetaylor ianlancetaylor mentioned this issue Jun 4, 2019
Open
@gopherbot
Copy link
Contributor

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

@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit that referenced this issue Jun 5, 2019
@ianlancetaylor
misc/cgo/test: add test for issue 31093
5f50914 
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>
@golang golang locked and limited conversation to collaborators Jun 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
3 participants
@rsc @ianlancetaylor @gopherbot