This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Eliminate <compare>'s dependency on <array>
ClosedPublic

Authored by Quuxplusone on Mar 24 2021, 4:39 PM.

Details

Summary

This refactor is not only a good idea, but is in fact required by the standard, in the sense that <array> is mandated to include <compare>. So <compare> shouldn't have a circular dependency on <array>!

According to "graph_header_deps.py", this also removes <compare>'s dependencies on everything-except-type-traits, and drops its cumulative cost from ~40KLOC to ~8KLOC.

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Mar 24 2021, 4:39 PM
Quuxplusone created this revision.
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
libcxx/include/compare
725

Happy to remove the sizeof...(_Ts) here if anyone cares; this was just a very mechanical transformation.

cjdb accepted this revision.Mar 24 2021, 8:45 PM
zoecarver accepted this revision as: zoecarver.Mar 24 2021, 10:03 PM

This LGTM once the CI is fixed. Thanks!

libcxx/include/compare
703

Looks like the CI wants this to be const.

A missing const, and also deal with zero-length arrays. Buildkite should be happier with this revision.

curdeius accepted this revision.Mar 25 2021, 1:11 AM
curdeius added a subscriber: curdeius.

LGTM!

This revision is now accepted and ready to land.Mar 25 2021, 1:11 AM

According to "graph_header_deps.py", this also removes <compare>'s dependencies on everything-except-type-traits, and drops its cumulative cost from ~40KLOC to ~8KLOC.

Perhaps the output of this should be generated by the build, and committed to git, so it becomes obvious in a given commit when it changes drastically?

@grandinj wrote:

Perhaps the output of this should be generated by the build, and committed to git, so it becomes obvious in a given commit when it changes drastically?

Well, we do have a bunch of things that are generated by scripts and it'd be nice to make that happen automatically. However, the thing about this graph is that the cumulative line counts would change with every line added or removed, which means it would be changing all the time and making ugly extra git diffs. (Or we could check in only the graph-without-line-counts, but the graph-without-line-counts is significantly less interesting/useful/satisfying IMO.)
(And this is leaving aside the separate question of how we'd "check in the graph" anyway. git isn't great with binary files. We could check in the .dot file and let people render the graph into an image locally.)

I am planning to PR a family of tests for discrete propositions — such as "<compare> does not pull in std::array" or "<algorithm> does not pull in std::type_info" or "<memory> does not pull in std::ostream" — which I think will be better at the goal of making sure improvements like this are not regressed accidentally in the future.