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

[Daniel McIntosh via Phabricator via libcxx-commits]

DanielMcIntosh-IBM added inline comments.


================
Comment at: libcxx/include/__format/formatter_string.h:147
+
+  _LIBCPP_HIDE_FROM_ABI auto format(const basic_string_view<_CharT, _Traits>& __str, auto& __ctx)
+      -> decltype(__ctx.out()) {
----------------
DanielMcIntosh-IBM wrote:
> Quuxplusone wrote:
> > Style: Linebreak after `auto`, please.
> > 
> > Perf: Shouldn't `basic_string_view<_CharT, _Traits> __str` be passed by value, not by const-ref? Or is there a subtle reason we're passing by const-ref here in particular?
> I believe `formatter<basic_string_view<_CharT, _Traits>, _CharT>::format` needs to meet the [Formatter named requirements](https://en.cppreference.com/w/cpp/named_req/Formatter), which limits what the function signature can be.
Never mind, I realized passing by value won't change what we can accept as arguments.

However, I'm not quite sure why passing by value would be a performance benefit. When `_Traits == char_traits<_CharT>`, does the `basic_string_view<_CharT>(__str.data(), __str.size())` somehow get treated as a move constructor, allowing the re-use of `__str` as the argument to `_Base::format` if `__str` was passed by value? i.e. does using that specific constructor somehow still allow copy elision? I was under the impression it was only allowed if "the initializer expression is a [value] of the same class type". Or is this a case of "we know what the constructor does, so the optimizer can figure out that there are no side-effects, and remove it"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112017