Page MenuHomePhabricator

[libcxx][type_traits] Implement C++20 common_ref
Needs ReviewPublic

Authored by miscco on Feb 10 2020, 11:22 AM.

Details

Reviewers
mclow.lists
EricWF
CaseyCarter
cjdb
ldionne
Group Reviewers
Restricted Project
Summary

This implements the new type trait common_ref that is needed for library concepts support

Event Timeline

miscco created this revision.Feb 10 2020, 11:22 AM
cjdb added inline comments.Feb 10 2020, 7:58 PM
libcxx/include/type_traits
2306–2307

Perhaps we should hold off on merging this so we can use std::convertible_to instead.

miscco added a comment.EditedFeb 10 2020, 11:53 PM

@cjdb I have to disagree. That would create a ciruclar dependency between concepts and type_traits, which is in my opinion not worth it.

Also note that std::convertible_to essentially falls back to is_convertible_v

miscco marked an inline comment as done.Feb 11 2020, 12:06 AM
miscco added inline comments.
libcxx/include/type_traits
2292–2318

I think I should add the respective wording here so that one understands what it is actually testing.

miscco updated this revision to Diff 243786.Feb 11 2020, 3:19 AM

Simplified code and improved comments

miscco added inline comments.Feb 11 2020, 3:24 AM
libcxx/include/type_traits
2306–2307

I would add that std::convertible_to has slightly different semantics too as it has some addtional requires clauses

cjdb added inline comments.Feb 12 2020, 11:22 AM
libcxx/include/type_traits
2306–2307

Fair. I'd at least encourage the use of a _requires-clause_, since it's demonstrably faster to compile than SFINAE and is being commit for the first time to a concepts-aware compiler.

miscco marked an inline comment as done.Feb 12 2020, 12:06 PM
miscco added inline comments.
libcxx/include/type_traits
2306–2307

That sounds reasonable. However, someone could theoretically use a C++20 compiler (aka clang 9) without concept support and that would lead to <type_traits> not compiling.

On the other hand I think that is a very bad use case that should not be suported. Maintainers thoughts?

cjdb added inline comments.Feb 12 2020, 2:33 PM
libcxx/include/type_traits
2306–2307

Since common_reference is really only useful with ranges in mind, we could section this code off with feature-test macros?

Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 8 2020, 11:41 AM