This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][modularisation] splits `<utility>` into self-contained headers
ClosedPublic

Authored by cjdb on Jun 9 2021, 6:12 PM.

Details

Summary
  • moves std::hash and std::unary_function into __functional
  • Everything else goes into __utility/${NAME}.h

Diff Detail

Event Timeline

cjdb created this revision.Jun 9 2021, 6:12 PM
cjdb requested review of this revision.Jun 9 2021, 6:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 9 2021, 6:12 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
cjdb updated this revision to Diff 351026.Jun 9 2021, 6:15 PM

undoes deletion of __utility/__decay_copy.h

Quuxplusone requested changes to this revision.Jun 9 2021, 8:02 PM
Quuxplusone added a subscriber: Quuxplusone.

Moving unary_function without also moving binary_function is unusual.
Anyway, please hold off on this until after D103753 is landed.

This revision now requires changes to proceed.Jun 9 2021, 8:02 PM
cjdb added a comment.Jun 9 2021, 8:28 PM

Moving unary_function without also moving binary_function is unusual.

I suggest you carefully read the changes to learn why I didn't move binary_function out of <utility>. Seriously, I have a good reason.

cjdb updated this revision to Diff 353021.Jun 18 2021, 8:57 AM

rebases to get new <algorithm> split

ldionne accepted this revision.Jun 21 2021, 9:15 AM

Moving unary_function without also moving binary_function is unusual.

I suggest you carefully read the changes to learn why I didn't move binary_function out of <utility>. Seriously, I have a good reason.

I assume you're going to move binary_function out of __functional_base eventually, when we break up that header?

Did you make any changes to the code itself? Assuming you did not, this LGTM with the two indentation nitpicks fixed and the CI passing.

libcxx/test/std/utilities/intseq/intseq.make/make_integer_seq.fail.cpp
35

Why this weird indentation change? Please leave as previously.

libcxx/test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
20–21

Same, please don't change indentation here. It's idiomatic to put the expected-error on the same line as the thing that triggers it.

cjdb updated this revision to Diff 354307.Jun 24 2021, 11:07 AM
cjdb marked 2 inline comments as done.

reverts accidental formatting

cjdb added a comment.Jun 24 2021, 11:07 AM

Moving unary_function without also moving binary_function is unusual.

I suggest you carefully read the changes to learn why I didn't move binary_function out of <utility>. Seriously, I have a good reason.

I assume you're going to move binary_function out of __functional_base eventually, when we break up that header?

Yes. It would be strange to start splitting up __functional_base in a commit concerned with splitting <utility>.

Did you make any changes to the code itself? Assuming you did not, this LGTM with the two indentation nitpicks fixed and the CI passing.

No changes to the code.

cjdb updated this revision to Diff 354309.Jun 24 2021, 11:11 AM

rebases to activate CI

cjdb updated this revision to Diff 354310.Jun 24 2021, 11:12 AM

rebases to reactivate CI without parent revision

cjdb updated this revision to Diff 354313.Jun 24 2021, 11:18 AM

rebases with parent revision properly removed this time

cjdb updated this revision to Diff 354338.Jun 24 2021, 1:05 PM
cjdb edited the summary of this revision. (Show Details)

removes "dependency" from commit message to see if this changes anything re CI

cjdb updated this revision to Diff 354339.Jun 24 2021, 1:07 PM

should hopefully work this time

cjdb updated this revision to Diff 354365.Jun 24 2021, 2:45 PM

fixes missing include

This revision was not accepted when it landed; it landed in state Needs Review.Jun 24 2021, 5:29 PM
This revision was automatically updated to reflect the committed changes.