Skip to content

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Feb 7, 2021

time_t is now 64 bits.
This PR resolves String(time_t) ambiguity.

Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@mcspr mcspr left a comment

Choose a reason for hiding this comment

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

Also note of the charconv in the stdlib (and it's source as well)
https://en.cppreference.com/w/cpp/utility/to_chars
https://en.cppreference.com/w/cpp/string/basic_string/to_string

lgtm as-is, but String is inherently weird with it's approach to conversions :/

@d-a-v
Copy link
Collaborator Author

d-a-v commented Feb 10, 2021

@mcspr I tried with charconv that I didn't know. That's very handy !
That one is faster (1.6x) but has a cost (+880B/flash+256B/rodata) (compared against lltoa)*
It has to be noted that these functions charconv(std::to_chars) or lltoa are used only when radix is not 10 (in which case sprintf is used). It should be quite rare.
Without further comments, I will push a newer commit keeping only lltoa. These functions will anyway not be linked in the general case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants