Page MenuHomePhabricator

[libc++] Granularize <type_traits> includes in <concepts>
ClosedPublic

Authored by philnik on Dec 2 2022, 3:47 AM.

Diff Detail

Event Timeline

philnik created this revision.Dec 2 2022, 3:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2022, 3:47 AM
philnik requested review of this revision.Dec 2 2022, 3:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2022, 3:47 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision.Dec 2 2022, 7:48 AM

LGTM once you have green CI.

This revision is now accepted and ready to land.Dec 2 2022, 7:48 AM

Thanks for the cleanup! One question.

libcxx/test/libcxx/ranges/range.utility.helpers/different_from.compile.pass.cpp
21

What is the benefit of using a private header here?

jloser added a subscriber: jloser.Dec 5 2022, 9:59 AM
jloser added inline comments.
libcxx/test/libcxx/ranges/range.utility.helpers/different_from.compile.pass.cpp
21

I assume it's because there is no std::different_from, it's just a useful private concept that we have unit tests for. I'm not sure this is the most appropriate home for these tests nested in ranges/range.utility.helpers as it's more generic than that (isomorphic to std::same_as).

philnik updated this revision to Diff 480865.Dec 7 2022, 5:19 AM
philnik marked 2 inline comments as done.

Try to fix CI

libcxx/test/libcxx/ranges/range.utility.helpers/different_from.compile.pass.cpp
21

We check that <__concpet/different_from.h> is self-contained. We might as well, since we test implementation details anyways.

Mordante accepted this revision.Dec 10 2022, 5:41 AM

One request other than that LGTM.

libcxx/test/libcxx/ranges/range.utility.helpers/different_from.compile.pass.cpp
21

We typically don't do that, so IMO it would be good to add a comment like Check that <__concpet/different_from.h> is self-contained.

philnik updated this revision to Diff 484302.Dec 20 2022, 9:30 AM
philnik marked an inline comment as done.

Address comment

philnik updated this revision to Diff 484324.Dec 20 2022, 10:51 AM

Try to fix CI

This revision was landed with ongoing or failed builds.Dec 20 2022, 12:37 PM
This revision was automatically updated to reflect the committed changes.
eaeltsin added inline comments.
libcxx/include/__type_traits/remove_cvref.h
15

Does the header include itself now?

Can we please remove this?

Mordante added inline comments.Dec 27 2022, 10:24 AM
libcxx/include/__type_traits/remove_cvref.h
15

Nice catch!

rtrieu added a subscriber: rtrieu.Dec 27 2022, 4:29 PM
rtrieu added inline comments.
libcxx/include/__type_traits/remove_cvref.h
15

I went ahead and removed this in 0c6e74fa744b0e6abe3614af55f4f9dd1fbc54b2.