-
Notifications
You must be signed in to change notification settings - Fork 136
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
from_chars integer parser #231
Conversation
…rs, invalid bases, and out of range bases
…ts, out of range errors for all bases (64 bit only), and fixed some test cases
merge fast_float with testing
- Werror=conversion,char-subscripts
- added: fix incorrect base for leading zeros test --------- Co-authored-by: Marvin <[email protected]> Co-authored-by: Maya Warrier <[email protected]>
- fix MSVC warning
@@ -173,7 +173,7 @@ using parse_options = parse_options_t<char>; | |||
// rust style `try!()` macro, or `?` operator | |||
#define FASTFLOAT_TRY(x) { if (!(x)) return false; } | |||
|
|||
#define FASTFLOAT_ENABLE_IF(...) typename std::enable_if<(__VA_ARGS__), int>::type = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not mind this change per se, but can you explain why this was changed? Is there a reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so that I can perform enable_if checks in this form:
typename = FASTFLOAT_ENABLE_IF(...)
instead of
FASTFLOAT_ENABLE_IF(...) = 0
The first form (default argument for a type parameter) is used in a template declaration in fast_float.h. Default arguments can only be specified once, so the template definition in parse_number.h cannot have it. With the first form, I can write the definition by just adding an extra 'typename', but with the second form I would have to repeat the FASTFLOAT_ENABLE_IF(...) again. I thought it best to keep it one place. With both, changing only one of them causes some not-so-obvious linker errors.
@mayawarrier This is fantastic work and I am happy to merge. I have a few minor comments, can you have a look? |
Co-authored-by: Daniel Lemire <[email protected]>
@lemire Thank you! I applied the suggestion. Awaiting your review. |
Merging. |
This PR implements the C++17 function:
from_chars_result from_chars( const char* first, const char* last, /* integer-type */& value, int base = 10);
Tests contributed by @TheRandomGuy146275.
Resolves #86