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

Add ParseFloat and replace stod with it #1227

Merged
merged 6 commits into from
Jan 17, 2023

Conversation

PragmaTwice
Copy link
Member

@PragmaTwice PragmaTwice commented Jan 16, 2023

We add ParseFloat to parse_util.h and replace most of std::stod with this newly added function in this PR.

torwig
torwig previously approved these changes Jan 16, 2023
Copy link
Contributor

@torwig torwig left a comment

Choose a reason for hiding this comment

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

LGTM

@PragmaTwice
Copy link
Member Author

PragmaTwice commented Jan 16, 2023

I notice that gcc implemented floating-point version std::from_chars since 11.1, which is a sad story. It means we cannot use std::from_chars because it comes too late and the gcc version is too new for some users. So I will implement it using strto* in c std library instead.

@PragmaTwice
Copy link
Member Author

I notice that gcc implemented floating-point version std::from_chars since 11.1, which is a sad story. It means we cannot use std::from_chars because it comes too late and the gcc version is too new for some users. So I will implement it using strto* in c std library instead.

done

@PragmaTwice PragmaTwice requested a review from torwig January 17, 2023 02:10
Copy link
Member

@git-hulk git-hulk left a comment

Choose a reason for hiding this comment

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

LGTM

@PragmaTwice
Copy link
Member Author

Thanks all. Merging...

@PragmaTwice PragmaTwice merged commit 84776a6 into apache:unstable Jan 17, 2023
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.

3 participants