[libcxx-commits] [PATCH] D104980: [libcxx][NFC] replaces `<utility>` includes with specific headers

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D104980#2880792 <https://reviews.llvm.org/D104980#2880792>, @cjdb wrote:

> There might be a potential workaround for this effort that lets us make progress, while also not throwing the world under the bus. `<Windows.h>` has a `WIN32_LEAN_AND_MEAN` macro that users can opt into if they want a slimmer `<Windows.h>`. We could have a similar macro that users opt into if they want slimmer libc++ headers for the time being. Once we've gotten tooling to a spot where we're happy users have automated remediation, we can then remove our use of this macro, and deprecate `_LIBCPP_LEAN_AND_MEAN`.
>
> In summary: no one gets broken, anyone who wants slimmer headers gets them, and we eventually get to the "ideal" state. @ldionne, @ericwf, WDYT?

Part of me wants to say "no, let's not add yet another configuration that'll only make things more complicated", and part of me wants to say "yes, that will allow us to keep making progress on this header minimization without breaking users".

However, since there is very little benefit to minimizing the headers if nobody's going to turn on that `LEAN_AND_MEAN` flag, I'm not sure. Instead, here's what I think would be best:

1. Minimize the includes in all the detail headers, and in the public headers too.
2. However, for "backwards compatibility", keep the unnecessary includes in the public headers, but group them and put them at the end of all the `#includes`:

  #include <__utility/whatever.h>
  #include <__ranges/whatever.h>
  
  // Kept for backwards compatibility until we can remove them
  #include <utility>
  #include <whatever>

Eventually, when we're ready to make the move, we'll be able to very easily make the jump for everyone by just removing those stray includes from all public headers. We might need to do a few adjustments before we can drop them (because as time passes, we might sometimes start relying on a few transitive includes by mistake here and there), but that should be much easier to do than what we're doing now.

I guess I'm just somewhat reluctant to adding yet another user-visible knob.



================
Comment at: libcxx/include/algorithm:655
 #include <initializer_list>
-#include <utility> // needed to provide swap_ranges.
 #include <memory>
 #include <iterator>
----------------
Mordante wrote:
> Can you remove the duplicated includes and sort them. Not part of your change, but would be nice to do it along in this patch,
I would definitely wait for a separate NFC commit, since doing it here will obscure the changes we're making in this patch, which are all precisely `#include`-related.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104980