[libcxx-commits] [PATCH] D114001: [libc++][format] Adds formatter floating-point.

Mark de Wever via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sun Jan 2 08:03:01 PST 2022


Mordante marked an inline comment as done.
Mordante added inline comments.


================
Comment at: libcxx/include/__format/parser_std_format_spec.h:761
+   */
+  _LIBCPP_HIDE_FROM_ABI constexpr auto __parse(auto& __parse_ctx)
+      -> decltype(__parse_ctx.begin()) {
----------------
vitaut wrote:
> The separation of concerns between `parse` and `__parse` is unclear, they even have identical apidoc comments. I think `__parse` better be merged into `parse`.
`__parse` does the low parsing, and `parse` does the validation. I've used this methods for other parser. For now I like to keep it that way.

On a side note I noticed that since a formatter is-a parser and not has-a parser there are a lot of instantiations of the same parser code. I've already experimented with an "has-a" approach. But before moving that patch further I like to see what the resolution for LWG3576 "Clarifying fill character in std::format" will be.

That improvement will remove parts of the code duplication and probably improve the binary size.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114001



More information about the libcxx-commits mailing list