Skip to content

Conversation

mcspr
Copy link
Collaborator

@mcspr mcspr commented Apr 6, 2022

Update itoa to be the same as newlib, also fixing edgecase of abs(INT_MIN)
Update WString.cpp:toString() integer conversions to use noniso funcs
Update WString to use C++ limits lib
Remove legacy gcc versions from Makefile and allow overrides
Don't fallback to c11 and c++11, source cannot support that

Additionally, allow test binary to accept arguments

~/d/e/t/host> make TEST_ARGS=[core][String] test Makefile:31: using g++ Makefile:46: Cannot compile in 32 bit mode (g++-multilib is missing?), switching to native mode Makefile:57: compiling in native mode /home/runner/dev/esp8266-Arduino/tests/host/bin/host_tests [core][String] =============================================================================== All tests passed (288 assertions in 20 test cases) 

As mentioned by @d-a-v in the Matrix channel
Not really sure if it's the best idea feature-wise, but it keeps this weird Arduino legacy output throughout the class. Like, the way -100 with base 16 gets converted into ffffff9c instead of -64 (which what I'd expect here, with any other sane API :). Main reason for that is Arduino API does printf equivalent of %X. But, it is consistent with other implementations, and there's no branching required to the printf
ref. https://github.com/arduino/ArduinoCore-API/blob/master/test/src/itoa.cpp, https://github.com/espressif/arduino-esp32/blob/6cfe4613e4b4846e1ab08c7f78b7ea241f52c7da/cores/esp32/WString.cpp#L80-L89

Just using the noniso funcs saves a little bit of code. For example checking on the nm output for int ctor when building the test binary

- 0000000000000150 T String::String(int, unsigned char) + 0000000000000121 T String::String(int, unsigned char)

I'd like to run these on the board before pushing though, since I have only tried the host test

mcspr added 2 commits April 6, 2022 08:28
Update itoa to be the same as newlib, fixing edgecase of abs(INT_MIN) Update WString.cpp:toString() integer conversions to use noniso funcs Remove legacy gcc versions from Makefile and allow overrides Don't fallback to c11 and c++11, source cannot support that
Copy link
Collaborator

@d-a-v d-a-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without actually trying, it looks good.
Thanks !

@mcspr mcspr merged commit 520233f into esp8266:master Apr 11, 2022
@mcspr mcspr deleted the noniso-is-weird branch April 11, 2022 10:53
hasenradball pushed a commit to hasenradball/Arduino that referenced this pull request Nov 18, 2024
…ile (esp8266#8531) * Test: fixing itoa implementation, clean-up of tests and test runner Update itoa to be the same as newlib, fixing edgecase of abs(INT_MIN) Update WString.cpp:toString() integer conversions to use noniso funcs Remove legacy gcc versions from Makefile and allow overrides Don't fallback to c11 and c++11, source cannot support that * CXX and CC are make predefined values, assuming ?= does not work (?)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants