cgo: refactor Go types a little bit #3927
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Code like this is not allowed by the upstream Go CGo implementation, but
was allowed by TinyGo:
var _ int8 = C.int8_t(5)
The reason it shouldn't be allowed is a little bit complicated. While
it is true that C.int8_t is always the same underlying data type as Go
int8 (signed 8-bit integer), the C type is actually a typedef of one of
the base C types (usually unsigned char or signed char) which in turn do
_not_ map cleanly to Go types: the 'char' type is ambiguous (it may be
either signed or unsigned depending on the ABI) and types like 'int'
vary in size by ABI as well.
To make code more portable, I think it's better to match the upstream
implementation.
aykevl
added a commit
to tinygo-org/drivers
that referenced
this pull request
Sep 22, 2023
For details, see: tinygo-org/tinygo#3927 This change just means we need to be more careful to use the right type, now that types like C.uint32_t don't match to the Go equivalent (like uint32).
aykevl
added a commit
to tinygo-org/bluetooth
that referenced
this pull request
Sep 22, 2023
For details, see: tinygo-org/tinygo#3927 I ran the smoke tests and the binaries are exactly identical to what they were before, so this change cannot have had an effect on these smoke tests (which is expected, as this is mostly just changing some types without changing the machine data type).
This was referenced Sep 22, 2023
|
Size difference with the dev branch: Binary size differencenot the same command!
go: downloading golang.org/x/text v0.7.0
tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/ntpclient/main.go
not the same command!
go: downloading golang.org/x/text v0.7.0
tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/udpstation/main.go
drivers/sizes-dev.txt has more commands than drivers/sizes-pr.txt
tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/ws2812
before after diff
60688 60688 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adt7410/main.go
9700 9700 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adxl345/main.go
13260 13260 0 0.00% tinygo build -size short -o ./build/test.hex -target=pybadge ./examples/amg88xx
8776 8776 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/apa102/main.go
11716 11716 0 0.00% tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/apds9960/proximity/main.go
10392 10392 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/apa102/itsybitsy-m0/main.go
8196 8196 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/at24cx/main.go
8308 8308 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bh1750/main.go
7608 7608 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/blinkm/main.go
69540 69540 0 0.00% tinygo build -size short -o ./build/test.hex -target=pinetime ./examples/bma42x/main.go
63260 63260 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmi160/main.go
27860 27860 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmp180/main.go
63288 63288 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmp280/main.go
12420 12420 0 0.00% tinygo build -size short -o ./build/test.hex -target=trinket-m0 ./examples/bmp388/main.go
8116 8116 0 0.00% tinygo build -size short -o ./build/test.hex -target=bluepill ./examples/ds1307/sram/main.go
22008 22008 0 0.00% tinygo build -size short -o ./build/test.hex -target=bluepill ./examples/ds1307/time/main.go
68964 68964 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/ds3231/main.go
4708 4708 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/easystepper/main.go
24876 24876 0 0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/espconsole/main.go
25104 25104 0 0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/esphub/main.go
24860 24860 0 0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/espstation/main.go
68684 68684 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/flash/console/spi
64636 64636 0 0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/flash/console/qspi
7052 7052 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/gc9a01/main.go
67956 67956 0 0.00% tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/gps/i2c/main.go
68424 68424 0 0.00% tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/gps/uart/main.go
8424 8424 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/hcsr04/main.go
5712 5712 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hd44780/customchar/main.go
5756 5756 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hd44780/text/main.go
10616 10616 0 0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/hd44780i2c/main.go
14624 14624 0 0.00% tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/hts221/main.go
16904 16904 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hub75/main.go
10048 10048 0 0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/basic
10884 10884 0 0.00% tinygo build -size short -o ./build/test.hex -target=xiao ./examples/ili9341/basic
29044 29044 0 0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/pyportal_boing
10076 10076 0 0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/scroll
10960 10960 0 0.00% tinygo build -size short -o ./build/test.hex -target=xiao ./examples/ili9341/scroll
263524 263524 0 0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/slideshow
12092 12092 0 0.00% tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/lis3dh/main.go
14012 14012 0 0.00% tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/lps22hb/main.go
26132 26132 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/lsm303agr/main.go
12572 12572 0 0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/lsm6ds3/main.go
10952 10952 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mag3110/main.go
10180 10180 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp23017/main.go
10608 10608 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp23017-multiple/main.go
9868 9868 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp3008/main.go
66772 66772 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp2515/main.go
22952 22952 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/microbitmatrix/main.go
22944 22944 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit-v2 ./examples/microbitmatrix/main.go
8532 8532 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mma8653/main.go
8316 8316 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mpu6050/main.go
74988 74988 0 0.00% tinygo build -size short -o ./build/test.hex -target=p1am-100 ./examples/p1am/main.go
12144 12144 0 0.00% tinygo build -size short -o ./build/test.hex -target=pico ./examples/pca9685/main.go
6084 6084 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/pcd8544/setbuffer/main.go
5104 5104 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/pcd8544/setpixel/main.go
2681 2681 0 0.00% tinygo build -size short -o ./build/test.hex -target=arduino ./examples/servo
7944 7944 0 0.00% tinygo build -size short -o ./build/test.hex -target=pybadge ./examples/shifter/main.go
56368 56368 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/sht3x/main.go
56424 56424 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/sht4x/main.go
56340 56340 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/shtc3/main.go
6464 6464 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1306/i2c_128x32/main.go
5940 5940 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1306/spi_128x64/main.go
5704 5704 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1331/main.go
6392 6392 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/st7735/main.go
6292 6292 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/st7789/main.go
17212 17212 0 0.00% tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/thermistor/main.go
10328 10328 0 0.00% tinygo build -size short -o ./build/test.hex -target=circuitplay-bluefruit ./examples/tone
10728 10728 0 0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/tm1637/main.go
9404 9404 0 0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/touch/resistive/fourwire/main.go
12460 12460 0 0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/touch/resistive/pyportal_touchpaint/main.go
15772 15772 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/vl53l1x/main.go
13812 13812 0 0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/vl6180x/main.go
6460 6460 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd2in13/main.go
6012 6012 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd2in13x/main.go
6288 6288 0 0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd4in2/main.go
137184 137184 0 0.00% go: downloading golang.org/x/text v0.7.0
137176 137176 0 0.00% go: downloading golang.org/x/text v0.7.0
137500 137500 0 0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/tcpclient/main.go
137600 137600 0 0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/webclient/main.go
2429709 2429709 0 0.00% sum
|
deadprogram
pushed a commit
to tinygo-org/drivers
that referenced
this pull request
Oct 5, 2023
For details, see: tinygo-org/tinygo#3927 This change just means we need to be more careful to use the right type, now that types like C.uint32_t don't match to the Go equivalent (like uint32).
deadprogram
pushed a commit
to tinygo-org/bluetooth
that referenced
this pull request
Oct 5, 2023
For details, see: tinygo-org/tinygo#3927 I ran the smoke tests and the binaries are exactly identical to what they were before, so this change cannot have had an effect on these smoke tests (which is expected, as this is mostly just changing some types without changing the machine data type).
deadprogram
pushed a commit
to tinygo-org/drivers
that referenced
this pull request
Oct 17, 2023
For details, see: tinygo-org/tinygo#3927 This change just means we need to be more careful to use the right type, now that types like C.uint32_t don't match to the Go equivalent (like uint32).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Code like this is not allowed by the upstream Go CGo implementation, but was allowed by TinyGo:
The reason it shouldn't be allowed is a little bit complicated. While it is true that C.int8_t is always the same underlying data type as Go int8 (signed 8-bit integer), the C type is actually a typedef of one of the base C types (usually unsigned char or signed char) which in turn do not map cleanly to Go types: the 'char' type is ambiguous (it may be either signed or unsigned depending on the ABI) and types like 'int' vary in size by ABI as well.
To make code more portable, I think it's better to match the upstream implementation.
This is a backwards incompatible change, but I'd like to make it sometime in the future. I will send corresponding patches to tinygo.org/x/bluetooth and tinygo.org/x/drivers.