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: map sized integer types to C equivalents #29878

Closed
phst opened this issue Jan 22, 2019 · 8 comments

Comments

Projects
None yet
5 participants
@phst
Copy link
Contributor

commented Jan 22, 2019

(Removing the template since this is an enhancement proposal)

The type mappings at https://golang.org/cmd/cgo/#hdr-Go_references_to_C don't state which Go types the sized C integer types int8_t etc. are mapped to. In C, these types have the same requirements as the corresponding Go types: they are not allowed to have padding bits and have to use two's complement representation, see https://en.cppreference.com/w/c/types/integer. Therefore, these types are guaranteed to have the same domain and representation as the corresponding Go types. So I propose to always represent these types with each other and formally document that fact. It would reduce the need for casts and would make it more obvious that there's no chance of data loss.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

Fine with me.

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Jan 22, 2019

phst added a commit to phst/go that referenced this issue Jan 23, 2019

cmd/cgo: use C exact-width integer types to represent Go types
The exact-width integer types are required to use two’s complement
representation and may not have padding bits, cf. §7.20.1.1/1 in the C11
standard or https://en.cppreference.com/w/c/types/integer.  This ensures that
they have the same domain and representation as the corresponding Go types.

Fixes golang#29878
@gopherbot

This comment has been minimized.

Copy link

commented Jan 23, 2019

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

@dpinela

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2019

I'm looking into this, is updating the (*typeConv).Type method in src/cmd/cgo/gcc.go sufficient to implement the equivalence? A quick experiment seems to show that it is.

edit: ninja'd. (And the answer turns out to be no.)

phst added a commit to phst/go that referenced this issue Jan 24, 2019

cmd/cgo: use C exact-width integer types to represent Go types
The exact-width integer types are required to use two’s complement
representation and may not have padding bits, cf. §7.20.1.1/1 in the C11
standard or https://en.cppreference.com/w/c/types/integer.  This ensures that
they have the same domain and representation as the corresponding Go types.

Fixes golang#29878

phst added a commit to phst/go that referenced this issue Mar 13, 2019

cmd/cgo: use C exact-width integer types to represent Go types
The exact-width integer types are required to use two’s complement
representation and may not have padding bits, cf. §7.20.1.1/1 in the C11
standard or https://en.cppreference.com/w/c/types/integer.  This ensures that
they have the same domain and representation as the corresponding Go types.

Fixes golang#29878

phst added a commit to phst/go that referenced this issue Mar 13, 2019

cmd/cgo: use C exact-width integer types to represent Go types
The exact-width integer types are required to use two’s complement
representation and may not have padding bits, cf. §7.20.1.1/1 in the C11
standard or https://en.cppreference.com/w/c/types/integer.  This ensures that
they have the same domain and representation as the corresponding Go types.

Fixes golang#29878

phst added a commit to phst/go that referenced this issue Mar 18, 2019

cmd/cgo: use C exact-width integer types to represent Go types
The exact-width integer types are required to use two’s complement
representation and may not have padding bits, cf. §7.20.1.1/1 in the C11
standard or https://en.cppreference.com/w/c/types/integer.  This ensures that
they have the same domain and representation as the corresponding Go types.

Fixes golang#29878

@gopherbot gopherbot closed this in 0875125 Mar 18, 2019

@gopherbot

This comment has been minimized.

Copy link

commented Mar 19, 2019

Change https://golang.org/cl/168337 mentions this issue: cmd/cgo: accept __uint8_t as the uint8_t type

gopherbot pushed a commit that referenced this issue Mar 19, 2019

cmd/cgo: accept __uint8_t as the uint8_t type
This works around the NetBSD <stdint.h> which defines the type using
"#define" rather than typedef.

Fixes #30918
Updates #29878

Change-Id: I8998eba52139366ae46762bdad5fcae85f9b4027
Reviewed-on: https://go-review.googlesource.com/c/go/+/168337
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 4, 2019

This seemed like a reasonable idea, but it breaks existing code (see #31093) and I don't see any way to fix that without losing good error reporting. So I'm going to roll back the two changes and leave this issue closed. Sorry. Perhaps this would have been a good approach if we had started that way, but we didn't.

@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 pushed a commit that referenced this issue Jun 5, 2019

cmd/cgo: roll back "use C exact-width integer types to represent Go t…
…ypes"

Roll back CL 159258 and CL 168337. Those changes broke existing
code. I can't see any way to keep existing code working while also
producing good error messages for types like C.ulong (such as the ones
already tested for in misc/cgo/errors).

This is not an exact roll back because parts of the code have changed
since those CLs.

Updates #29878
Fixes #31093

Change-Id: I56fe76c167ff0ab381ed273b9ca4b952402e1434
Reviewed-on: https://go-review.googlesource.com/c/go/+/180357
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@bcmills

This comment has been minimized.

Copy link
Member

commented Jun 5, 2019

I don't see any way to fix that without losing good error reporting.

Could you elaborate? It seems like we could define ulong, ushort, etc. as type aliases for the corresponding exact-width C types, but I haven't thought through the full implications of that.

(See also #13467.)

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 5, 2019

I tried exactly that--defining them as type aliases--and the effect was that the misc/cgo/error err2.go test failed. It failed because expressions like C.ushort("x") gave an error referring to an error converting to the type uint16, rather than the type C.ushort. I don't think this change gives us enough of an improvement to merit making the error messages that much more confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.