[libc-commits] [PATCH] D144597: [libc] Refactor string to float return values

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/__support/str_to_float.h:28
 
+template <class T> struct FloatPair {
+  typename fputil::FPBits<T>::UIntType mantissa;
----------------
Bikeshed: s/`FloatPair`/`FloatComponents` ?


================
Comment at: libc/src/__support/str_to_float.h:36
+  int32_t exponent = 0;
+  int error = 0;
+};
----------------
Looks like there are two types of uses for this type:

1. The `eisel_lemire` function returns `fail_output` with `error` set to `-1`. Would `cpp::optional<FloatPair>` be more appropriate?
2. The `simple_decimal_conversion` function returns actual error numbers in `error`. Would `__llvm_libc::ErrorOr<FloatPair>` be more appropriate?


================
Comment at: libc/src/__support/str_to_float.h:39
+
+template <class T> struct FloatParseReturn {
+  FloatConvertReturn<T> convert_result;
----------------
s/`FloatParseReturn`/`StrToFloatResult`? Also, why can't we use `StrToNumResult` directly?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144597/new/

https://reviews.llvm.org/D144597