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

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/src/__support/str_to_float.h:28
 
+template <class T> struct FloatPair {
+  typename fputil::FPBits<T>::UIntType mantissa;
----------------
sivachandra wrote:
> Bikeshed: s/`FloatPair`/`FloatComponents` ?
I like `FloatPair` better, personally, but it doesn't really matter to me.


================
Comment at: libc/src/__support/str_to_float.h:36
+  int32_t exponent = 0;
+  int error = 0;
+};
----------------
sivachandra wrote:
> 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?
For 1 you're right, we can use optional instead. 
For 2, we need to return not just the error, but also some additional information (specifically if we've got an infinity or a zero for the out of range value). For this reason I've left this struct in place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144597