[libcxx-commits] [PATCH] D103583: [libcxx][gardening] Move all algorithms into their own headers.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/include/algorithm:648-665
+#include <__algorithm/adjacent_find.h>
+#include <__algorithm/all_of.h>
+#include <__algorithm/any_of.h>
+#include <__algorithm/count.h>
+#include <__algorithm/count_if.h>
+#include <__algorithm/equal.h>
+#include <__algorithm/find.h>
----------------
Quuxplusone wrote:
> (A) Why are these few headers alphabetized and then the bulk of them (below) done... below?
> 
> (B) @cjdb wrote:
> 
> > > Note: during this change, I burned down all the includes, so this follows "include only and exactly what you use."
> >
> > In D103330 @ldionne asked that this be done in a separate patch.
> 
> The important thing @ldionne was getting at in D103330 is Hyrum's Law. We need to make sure that we don't break any of libc++'s customers who might be relying on the fact e.g. "by including <algorithm>, I also receive all of <functional>."  @zoecarver's change doesn't actually affect those customers AFAICT, because he's leaving the existing includes alone here (e.g. <algorithm> does still include <functional>).
> 
> However, @zoecarver, please do triple-check this PR to make sure that the top-level <algorithm> header still includes all the facilities it used to, and that the same is true for all other top-level publicly visible headers (e.g. <functional>, <iterator>, anything else that might have changed).
> (A) Why are these few headers alphabetized and then the bulk of them (below) done... below?

Arg. Sorry this came out of merging Chris' patch with mine. Will fix. (Come to think of it, I also need to update the cmake list.)

> The important thing @ldionne was getting at in D103330 is Hyrum's Law. We need to make sure that we don't break any of libc++'s customers who might be relying on the fact e.g. "by including <algorithm>, I also receive all of <functional>." @zoecarver's change doesn't actually affect those customers AFAICT, because he's leaving the existing includes alone here (e.g. <algorithm> does still include <functional>).

> However, @zoecarver, please do triple-check this PR to make sure that the top-level <algorithm> header still includes all the facilities it used to, and that the same is true for all other top-level publicly visible headers (e.g. <functional>, <iterator>, anything else that might have changed).

Yes. I haven't removed anything from `<algorithm>`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103583