[libcxx-commits] [PATCH] D103357: [libc++][format] Add __format_arg_store.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__format/format_context.h:59
+    basic_format_args<basic_format_context<_OutIt, _CharT>> __args,
+    optional<_VSTD::locale>&& __loc = nullopt) {
+  return basic_format_context(_VSTD::move(__out_it), __args,
----------------
Re your comment on D106567 — It would be cleaner here to write "one function declaration per function signature," i.e. get rid of the defaulted function argument ( https://quuxplusone.github.io/blog/2020/04/18/default-function-arguments-are-the-devil ), and then I bet you wouldn't run into any modules issues with the 2-argument overload (because it wouldn't mention `optional` in its signature) nor the 3-argument overload (because it wouldn't mention `optional` in its signature, either). The next step would be to give the two things not-the-same name.
I actually talk about the "ooh, I'll take an optional that defaults to nullopt!" antipattern in //[Back to Basics: Algebraic Data Types](https://www.youtube.com/watch?v=OJzmWqCCZaM&t=2792s)// (CppCon 2020), and obviously I've talked about When to Give Two Things the Same Name (CppCon 2021) as well. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103357