This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][nfc] splices non-modifying algorithms into their own headers
AbandonedPublic

Authored by cjdb on May 28 2021, 10:59 AM.

Details

Reviewers
ldionne
zoecarver
Mordante
jdoerfert
Group Reviewers
Restricted Project
Summary
  • Moves all of [alg.nonmodifying] into <__algorithm/${ALGO_NAME}.h>.
  • Moves __search from <functional> into <__functional/__search.h> so we can stop including <functional> in <algorithm>.

Depends on D103329.

Diff Detail

Event Timeline

cjdb created this revision.May 28 2021, 10:59 AM
cjdb requested review of this revision.May 28 2021, 10:59 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision.May 31 2021, 8:09 AM

LGTM, but can you please keep including <functional> in <algorithm> for now? Add a comment saying // TODO: Remove this include when we're sure it won't break people.

You can then perform the removal immediately after, but in a separate commit. The reason is that I suspect it's going to break a lot of code (which does not include-what-they-use), and so having a separate commit just to toggle that include is going to be helpful for vendors. For example, I might have to temporarily revert that commit for shipping it on Apple platforms until some internal stuff has been fixed - I suspect other vendors will be in the same boat.

This revision is now accepted and ready to land.May 31 2021, 8:09 AM

As usual, I'll leave my for-the-record comment that I really don't like this direction. It makes tons of little headers for no benefit... and I smell the upcoming PR where you add a bunch of C++20 Ranges dependencies into all of these little headers, making compilation slower for C++17 programs and complexifying the dependency graph.

Notice the failing Modules test; that's being discussed in the comments on D102781.

cjdb updated this revision to Diff 349416.Jun 2 2021, 4:58 PM

rebases so Zoe can use this

cjdb abandoned this revision.Jun 11 2021, 9:53 AM