[libcxx-commits] [PATCH] D144277: [libc++][format] Addresses LWG3881.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/include/__format/container_adaptor.h:42
+  using __maybe_const_adaptor   = __maybe_const<is_const_v<__maybe_const_container>, _Adaptor>;
+  formatter<ranges::ref_view<__maybe_const_container>, _CharT> __underlying_;
 
----------------
Mordante wrote:
> tcanens wrote:
> > Mordante wrote:
> > > ldionne wrote:
> > > > Any reason why you're using `ref_view` instead of `all_t`? Those are not always equivalent -- is the difference not relevant?
> > > Mainly since that is what the wording in the LWG issue is. Victor's original proposed wording did use `views::all_t`.
> > > This is the rationale in the issue to make the change:
> > > ```
> > > [2023-02-10 Tim provides updated wording]
> > > 
> > > The container elements may not be const-formattable so we cannot use the const formatter unconditionally. Also the current wording is broken because an adaptor is not range and we cannot use fmt-maybe-const on the adaptor — only the underlying container.
> > > ```
> > This is for passing a slightly disguised container into a function, not for view composition. It does not need the sometimes-by-reference-sometimes-by-copy-sometimes-ill-formed behavior of `all_t`.
> Thanks for the information.
Ah thanks, I think I had missed that the original resolution was superseded.


================
Comment at: libcxx/test/std/containers/container.adaptors/container.adaptors.format/format.functions.tests.h:254
+
+  // LWG3881 fixes formatting characters with a string as container.
+  test_char_default<CharT>(check, check_exception, std::queue{std::basic_string<CharT>{input.begin(), input.end()}});
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144277