[libcxx-commits] [PATCH] D128785: [libc++][format] Improve floating-point formatters.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/include/__format/formatter_floating_point.h:586-591
+      &("infnanINFNAN"
+            [6 * (__specs.__std_.__type_ == __format_spec::__type::__hexfloat_upper_case ||
+                  __specs.__std_.__type_ == __format_spec::__type::__scientific_upper_case ||
+                  __specs.__std_.__type_ == __format_spec::__type::__fixed_upper_case ||
+                  __specs.__std_.__type_ == __format_spec::__type::__general_upper_case) +
+             3 * __isnan]),
----------------
I know this was simply moved, however I think we should at least refactor the uppercase determination into a separate variable like this:

```
bool __uppercase = __specs.__std_.__type_ == __format_spec::__type::__hexfloat_upper_case ||
                  __specs.__std_.__type_ == __format_spec::__type::__scientific_upper_case ||
                  __specs.__std_.__type_ == __format_spec::__type::__fixed_upper_case ||
                  __specs.__std_.__type_ == __format_spec::__type::__general_upper_case;
```

I'm not a big fan of the way this is written because it's a bit too clever, but I can't think of an easy way to keep it branchless with another implementation, so that's fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128785