[libcxx-commits] [PATCH] D149543: [libc++][format] Fixes vector<bool> requirements.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D149543#4459956 <https://reviews.llvm.org/D149543#4459956>, @ldionne wrote:

> In D149543#4456976 <https://reviews.llvm.org/D149543#4456976>, @hans wrote:
>
>>> In D149543#4456497 <https://reviews.llvm.org/D149543#4456497>, @Mordante wrote:
>>>
>>>> 
>>>
>>> Interesting. Am I right to assume most of the new size is due to including `<string>`?
>>
>> I don't think it's `<string>`; that's already widely included. I think it's `__format/formatter_output.h` where most of it gets added.

Thanks for the info @hans.

> @Mordante What's the very very minimum amount of stuff we need to implement `formatter<bool>`? Are we somehow including a lot more than we strictly need? Can we split up some of those `format` headers to avoid including some of this stuff transitively?

At the moment we include the minimum set of granularized headers we need. I had a look yesterday and I found some unneeded includes in our granularized format headers, I'll create a patch for that after testing. I think I can slice up `formatter_output.h` and reduce the number includes that way. I'll look into that later this week.

>> Part of me thinks maybe `<format>` could be gated behind a macro like is was while it was experimental, since we're not currently using it in Chromium. But that would only delay the issue until we do want to use it of course.
>
> IMO that's just sweeping the issue under the carpet.

I agree. However I think the performance of compiling `vector` is important. So I'm not entirely against it as long as users (chromium) explicitly needs to opt out of it, so by default you get the the current behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149543