[libcxx-commits] [PATCH] D125606: [libc++][format] Improve string formatters

[Victor Zverovich via Phabricator via libcxx-commits]

vitaut added inline comments.


================
Comment at: libcxx/include/__format/parser_std_format_spec.h:1750-1751
+    __width_ = __format_spec::__substitute_arg_id(__arg);
+    if (__width_ == 0)
+      __throw_format_error("A format-spec width field replacement should have a positive value");
+  }
----------------
Mordante wrote:
> vitaut wrote:
> > This is a weird check. I think we should open an LWG issue to allow zero dynamic width and remove this unnecessary check.
> Currently width is specified as
> ```
> width:
>     positive-integer
>     { arg-idopt }
> ```
> 
> do you want to change that also to `nonnegative-integer`? When not it's the question what the width of `0` should do.
> Note that `__format_spec::__substitute_arg_id(__arg);` already throws on negative values.
> 
> Another option would be to give the minimum as an argument to `__substitute_arg_id` so there it can validate the proper boundary.
The grammar was done that way to avoid confusion between zero width and zero flag. I am not suggesting to change the grammar, just remove the runtime check for zero dynamic width since it's mostly harmless and only adds overhead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125606