-
Notifications
You must be signed in to change notification settings - Fork 167
Fix wording #323
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
base: main
Are you sure you want to change the base?
Fix wording #323
Conversation
``` | ||
|
||
Though the C++17 standard has you do a comparison with `std::errc()` to check whether the conversion worked, you can avoid it by casting the result to a `bool` like so: | ||
You can simplify the conversion success check by casting the result of `from_chars` to `bool`, as shown below: |
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.
The original text sounds clunky but it makes an important point that this edit removes. The C++17 standard does not include this cast to bool. It's important for users to know that it is a fast_float extension.
The cast to bool
only became standard in C++26. https://en.cppreference.com/w/cpp/utility/from_chars_result.html
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.
Yes, there is still room for improvement. The point is that I did not understand the actual purpose of the original statement. Neither does it mention C++26.
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.
The original wording is valid English, but how about:
In C++17, to check whether the conversion succeeded, you must compare the ec
member to std::errc()
. As an extension, fast float supports the C++26 feature that allows you to cast the result to bool
, like so:
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.
Yes, that's easier to comprehend for non-native English speakers. So what about C++20 then?
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.
Everything else in that document talks about C++17 from_chars
, and I think it should be obvious that unless stated otherwise, the same API is present in C++20 and C++23.
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.
What about this?
Whereas before C++26 std::from_chars
needs to be explicitily compared with std::errc()
to check for conversion success, fast_float::from_chars
simplifies the check as shown below:
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.
from_chars
is a function, you don't compare it to std::errc()
, and you don't even compare its return value to std::errc()
, so I don't think that's good.
"to check for a successful conversion" would be better than "to check for conversion success".
I don't think there's a problem with simply referring to C++17 as the standard that defined the from_chars
feature, but if you insist, then maybe:
Prior to C++26, checking for a successful
std::from_chars
conversion requires comparing thefrom_chars_result::ec
member tostd::errc()
. As an extensionfast_float::from_chars
supports the improved C++26 API that allows checking the result by converting it tobool
, like so:
No description provided.