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

from_chars integer parser #231

Merged
merged 24 commits into from
Dec 14, 2023
Merged

from_chars integer parser #231

merged 24 commits into from
Dec 14, 2023

Conversation

mayawarrier
Copy link
Contributor

This PR implements the C++17 function:
from_chars_result from_chars( const char* first, const char* last, /* integer-type */& value, int base = 10);

  • Implementation is similar to that in fast_int #86.
  • Uses SIMD acceleration for base 10.

Tests contributed by @TheRandomGuy146275.
Resolves #86

Marvin and others added 23 commits December 10, 2023 23:40
…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]>
@@ -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
Copy link
Member

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?

Copy link
Contributor Author

@mayawarrier mayawarrier Dec 14, 2023

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.

@lemire
Copy link
Member

lemire commented Dec 14, 2023

@mayawarrier This is fantastic work and I am happy to merge. I have a few minor comments, can you have a look?

@mayawarrier
Copy link
Contributor Author

@lemire Thank you! I applied the suggestion. Awaiting your review.

@lemire
Copy link
Member

lemire commented Dec 14, 2023

Merging.

@lemire lemire merged commit ede1d6b into fastfloat:main Dec 14, 2023
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fast_int
3 participants