[libcxx-commits] [PATCH] D120466: [libc++] Granularize <utility> includes

Wed Mar 2 08:44:28 PST 2022

EricWF added a comment.

In D120466#3352007 <https://reviews.llvm.org/D120466#3352007>, @ldionne wrote:

> In D120466#3351831 <https://reviews.llvm.org/D120466#3351831>, @EricWF wrote:
>
>> Do we have a document describing why we're doing this?
>
> Eric, we've been doing this for roughly 1 year now. Arthur has already explained some of the reasons to you, like reducing/removing circular dependencies and being easier to understand (now you don't have to wonder why in the world is `std::swap` defined in `<type_traits>`. As a side benefit, it also makes our headers include fewer things transitively, which enforces slightly better IWYU for users.

Cool. I'm suprised we don't have a document describing why we're doing it. And I would like one. I was unaware of circular dependencies prior to this change. And I personally don't find it easier to understand. The "side benefit" of including fewer things will break our users, and at Google IWYU has been adding includes for internal headers (which is not good).

> Regarding compile-times: the right way to speed things up is to use modules and have more minimal headers, not to try to reduce the number of filesystem accesses by reducing the number of files.

The right way to speed up compile times is the way that speeds up compile times. Measurement is key.

> Regarding diagnostics, I'm not sure how they are really made worse by this. If we wanted, we could also piggy-back on the `include_instead` pragma to tweak error messages. Clang fix-it should definitely listen to `include_instead`, at the very least.

When the compiler tells you to include the wrong header, or diagnostics expose implementation details (and hide what user-visible component is responsible for the breakage); it's not good for our users.

I'm not interested in litigating this. I am interested in documenting the rational for doing it. With concrete examples of how it makes the code better. I'm going to stand firm on this. We NEED to document large design decisions like this.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120466