[libcxx-commits] [PATCH] D140653: [libc++][format] Implements range_formatter

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__format/range_formatter.h:164
+    // When the range is contiguous use a basic_string_view instead to avoid a
+    // copy of the underlaying data. The basic_string_view formatter
+    // specialization is the "basic" string formatter in libc++.
----------------
Here and elsewhere.


================
Comment at: libcxx/include/__format/range_formatter.h:188
+      std::formatter<basic_string_view<_CharT>, _CharT> __formatter;
+      __formatter.set_debug_format();
+      return __formatter.format(basic_string_view<_CharT>{__range.data(), __range.size()}, __ctx);
----------------
Can you see a way to de-duplicate this function and `__format_as_string` by e.g. passing the `__specs` into it?


================
Comment at: libcxx/include/__format/range_formatter.h:223
+private:
+  _LIBCPP_HIDE_FROM_ABI constexpr void __parse_type(const _CharT*& __begin, const _CharT* __end) {
+    switch (*__begin) {
----------------
I think you're assuming that `string::iterator` is a `char*` here.


================
Comment at: libcxx/test/std/utilities/format/format.range/format.range.formatter/underlaying.pass.cpp:1
+//===----------------------------------------------------------------------===//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
----------------
Typo in the filename


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140653