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: broken C.complexfloat and C.complexdouble support? #13402

Closed
mdempsky opened this issue Nov 25, 2015 · 5 comments
Closed

cmd/cgo: broken C.complexfloat and C.complexdouble support? #13402

mdempsky opened this issue Nov 25, 2015 · 5 comments

Comments

@mdempsky
Copy link
Member

In cmd/cgo/gcc.go, I see there's code to support C.complexfloat and C.complexdouble, but it doesn't seem to work:

$ go build x.go
# command-line-arguments
could not determine kind of name for C.complexdouble
could not determine kind of name for C.complexfloat

$ cat x.go
package x
import "C"
var _ C.complexfloat
var _ C.complexdouble

CC @ianlancetaylor

@ianlancetaylor
Copy link
Contributor

The "float complex" and "double complex" in nameToC need to be __complex instead. Not sure about dwarfToName. Also probably the first case in the switch at line 1692 should check for dwarf.ComplexType as well.

@mdempsky
Copy link
Member Author

Looks like the DWARF names are "complex float" and "complex double" for both GCC and Clang, so the space stripping logic will map them to C.complexfloat and C.complexdouble even without any dwarfToName entries.

@mdempsky
Copy link
Member Author

Hm, fixing this leads to:

./issue8694.go:27: cannot use x (type complex64) as type C.complexfloat in argument to _Cfunc_complexFloatSquared
./issue8694.go:28: invalid operation: cx2 != x2 (mismatched types C.complexfloat and complex64)
./issue8694.go:34: cannot use y (type complex128) as type C.complexdouble in argument to _Cfunc_complexDoubleSquared
./issue8694.go:35: invalid operation: cy2 != y2 (mismatched types C.complexdouble and complex128)

Apparently the status quo is cgo treats C.complexfloat and complex64 as equivalent (and similarly for C.complexdouble and complex128). Should we be worried that fixing this issue could break Go code that relies on this (seemingly erroneous) behavior?

@gopherbot
Copy link

CL https://golang.org/cl/17208 mentions this issue.

@ianlancetaylor
Copy link
Contributor

It does seem like people should need an explicit type conversion to the C type, so I think it's OK to make this change. Even if this were covered by the Go 1 guarantee, it is a bug fix.

mdempsky added a commit that referenced this issue Nov 25, 2015
…e fix

Updates #13402.

Change-Id: Ia7b729d81fb78206d214444911f2e6573b88717a
Reviewed-on: https://go-review.googlesource.com/17240
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Nov 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants