[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:36
+  typename fputil::FPBits<T>::UIntType mantissa = 0;
+  int32_t exponent = 0;
+  int error = 0;
----------------
sivachandra wrote:
> Can you combine `mantissa` and `exponent` into a `FloatPair` value? Can it be `cpp::optional<FloatPair>`?
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.


================
Comment at: libc/src/__support/str_to_float.h:28
 
+template <class T> struct FloatPair {
+  typename fputil::FPBits<T>::UIntType mantissa;
----------------
lntue wrote:
> sivachandra wrote:
> > michaelrj wrote:
> > > sivachandra wrote:
> > > > Bikeshed: s/`FloatPair`/`FloatComponents` ?
> > > I like `FloatPair` better, personally, but it doesn't really matter to me.
> > My concern is that `FloatPair` sounds like it holds two floating point values.
> 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,
I've changed it to `ExpandedFloat` since that seems more accurate to what it is and does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144597