[libcxx-commits] [PATCH] D110497: [libc++][format][3/6] Adds a __container_buffer.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante planned changes to this revision.
Mordante marked an inline comment as done.
Mordante added a comment.

Thanks for the feedback, I'll look at these issues when Buildkite is working properly again.



================
Comment at: libcxx/test/libcxx/utilities/format/enable_insertable.compile.pass.cpp:93
+static_assert(std::__format::__insertable<std::vector<char>>);
+static_assert(std::__format::__insertable<std::vector<wchar_t>>);
+static_assert(std::__format::__insertable<std::deque<char>>);
----------------
Quuxplusone wrote:
> What do you expect to happen for `std::vector<int>` or `std::vector<std::string>`? On the one hand, `int` and `string` are not "char types," right? But on the other hand, `vec.insert(vec.end(), charptr, charendptr)` will work as expected, so we //do// want to do the optimization, right?
The format specific functions require a `char` or `wchar_t`. So I expect them not to pass this trait. When we want to use this more generic, we can add an `std::__insertable` and rename the `typedef` in the containers. I didn't restrain the `typedef` in the classes; I looked at it but it became quite ugly quite fast. Instead it's constrained in the `std::__format::__insertable` `concept`. So it would be quite trivial to add `std::__insertable`, but I don't have a usecase for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110497