[libcxx-commits] [PATCH] D112017: [libcxx][format] Fix how we handle char traits in formatter<string> and formatter<string_view>

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision.
Quuxplusone added inline comments.


================
Comment at: libcxx/include/__format/formatter_string.h:148
+  _LIBCPP_HIDE_FROM_ABI auto
+  format(basic_string_view<_CharT, _Traits> __str, auto& __ctx)
+      -> decltype(__ctx.out()) {
----------------
Mordante wrote:
> Quuxplusone wrote:
> > Aargh, I was gonna say "ship it" and then I noticed that... none of these `format` methods are marked `const`?! What's up with that?
> > https://en.cppreference.com/w/cpp/named_req/Formatter says:
> > > `formatter.format(arg, format_context)` [should work with] `formatter`, a **value** of type `Formatter`
> > 
> > What does "value" mean in this context? It probably doesn't mean "lvalue" or it would have said "lvalue," right? I suspect it means "(possibly const) lvalue or rvalue," in which case `format` should always be const-qualified. But then how does any of this //ever// work, today, given that we're not const-qualifying these `format` methods? Does this suddenly need further investigation?
> > 
> > Ping @Mordante — clarification please? (If @Mordante says "yeah this is all working as intended," then you can land this AFAIC.)
> I'm not entirely seeing what exactly the issue is. But there's no requirement the format member function is a `const` member function. 
> http://eel.is/c++draft/format#formatter.spec-6 contains a sample of a user defined formatter, which has a non-`const` member function.
> 
> When a formatter is used it:
> - first calls the parse member function, this function should store the state of the parsing, so shouldn't be `const`,
> - then calls format.
> http://eel.is/c++draft/format#formatter.spec-6 contains a sample of a user defined formatter, which has a non-const member function.

Okay, works for me!

FWIW: Sounds like the situation is analogous to how it works with the distributions from <random>: some of the <random> distributions are stateless and thus could //physically// be `const`-qualified, but that would actually be worse QoI because it would harm portability-to-other-vendors and/or refactorability-to-other-distribution-types-that-happen-to-be-stateful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112017