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

Comments

@mdempsky
Copy link
Member

commented Nov 25, 2015

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

This comment has been minimized.

Copy link
Contributor

commented Nov 25, 2015

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

This comment has been minimized.

Copy link
Member Author

commented Nov 25, 2015

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

This comment has been minimized.

Copy link
Member Author

commented Nov 25, 2015

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

This comment has been minimized.

Copy link

commented Nov 25, 2015

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

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Nov 25, 2015

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 mdempsky closed this in 10cb39a Nov 25, 2015

mdempsky added a commit that referenced this issue Nov 25, 2015
doc: update go1.6.txt for cmd/cgo's C.complexfloat and C.complexdoubl…
…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
3 participants
You can’t perform that action at this time.