[libcxx-commits] [PATCH] D103368: [libc++][format] Adds parser std-format-spec.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 2 inline comments as done.
Mordante added a comment.

Thanks for the review!



================
Comment at: libcxx/include/__format/parser_std_format_spec.h:100
+    __scientific_upper_case,
+    __fixed,
+    __general_lower_case,
----------------
vitaut wrote:
> Fixed should have lower and uppercase variants too (`f` and `F`).
The upper and lower case are formatted the same. So the the type function maps both `f` and `F` to fixed.


================
Comment at: libcxx/include/__format/parser_std_format_spec.h:388
+  [[nodiscard]] constexpr bool __has_precision_field() const noexcept {
+    return __precision_as_arg == 0 || __precision != __format::__number_max;
+  }
----------------
vitaut wrote:
> So if the user passes a precision of `INT32_MAX` it will be treated as "no precision" (and asserted on in debug mode) which is somewhat arbitrary and may lead to the same problems I mentioned earlier. To avoid this you could use two values above `INT32_MAX` (e.g. `INT32_MAX + 1` and `INT32_MAX + 2` to denote "no precision" and `__precision_as_arg` respectively.
I guess I need more comment for this code ;-)
When the format-spec has a value `__precision_as_arg == 0`.
When the format-spec has an arg-id all arg-id's except for `INT32_MAX` are a valid arg-id. If the arg-id gets resolved to `INT32_MAX` this value is considered a limit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103368