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

[Victor Zverovich via Phabricator via libcxx-commits]

vitaut added inline comments.


================
Comment at: libcxx/include/__format/parser_std_format_spec.h:100
+    __scientific_upper_case,
+    __fixed,
+    __general_lower_case,
----------------
Mordante wrote:
> 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.
Not quite, NaN and infinity are formatted differently.


================
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;
+  }
----------------
Mordante wrote:
> 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.
In other words `INT32_MAX` is already handled correctly with the current approach or have I misunderstood your comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103368