The internal implementation of the string to float function previously
used pointer arguments for returning several values. Additionally it
set errno in several unexpected places. Now all of that goes through
return structs. For readability I also moved the function away from raw
pointer arithmetic towards proper indexing. I also added support for
rounding modes.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libc/test/src/__support/str_to_float_test.cpp | ||
---|---|---|
322–325 | I think ASSERT_NE( 0, ...) for these would be better? |
Can you also update the new dependency in the bazel layout?
@llvm-project//libc/test/src/stdlib:atof_test FAILED TO BUILD @llvm-project//libc/test/src/stdlib:strtod_test FAILED TO BUILD @llvm-project//libc/test/src/stdlib:strtof_test FAILED TO BUILD @llvm-project//libc/test/src/stdlib:strtold_test FAILED TO BUILD
libc/src/__support/str_to_float.h | ||
---|---|---|
29 | Bikeshed: s/FloatPair/FloatComponents ? | |
37 | Looks like there are two types of uses for this type:
| |
40 | s/FloatParseReturn/StrToFloatResult? Also, why can't we use StrToNumResult directly? |
libc/src/__support/str_to_float.h | ||
---|---|---|
29 | I like FloatPair better, personally, but it doesn't really matter to me. | |
37 | For 1 you're right, we can use optional instead. |
libc/src/__support/high_precision_decimal.h | ||
---|---|---|
390–391 | nit: for performance purpose, this could be changed to: return result + this->should_round_up(this->decimal_point, round) which is well-defined behavior and branchless. | |
libc/src/__support/str_to_float.h | ||
29 | I agree that FloatPair name is a bit confusing, but other name that I can come up with is ExpMant, which might not be better, |
reformat structs somewhat
libc/src/__support/str_to_float.h | ||
---|---|---|
29 | I've changed it to ExpandedFloat since that seems more accurate to what it is and does. | |
36 | I can combine them into a FloatPair, but I don't see any benefit to making it an optional FloatPair. Once we get to a function that passes back a FloatConvertResult the mantissa and exponent are well defined. |
nit: for performance purpose, this could be changed to:
which is well-defined behavior and branchless.