This is an archive of the discontinued LLVM Phabricator instance.

Differential D97862

[libc++] Introduce __identity_t<T>
ClosedPublic

Authored by • Quuxplusone on Mar 3 2021, 8:20 AM.

Details

Reviewers
zoecarver
ldionne
curdeius
Group Reviewers
Restricted Project
Commits
rG09fa1d0e50a3: [libc++] Introduce __identity_t<T>. NFCI.
Summary

This is just a shorter synonym for __identity<T>::type.
Use it consistently throughout, where possible.

There is still some metaprogramming in <memory> and <variant>
where __identity is being used _without_ immediately calling
::type on it; but this is the unusual case, and it will become
even less usual as we start deliberately protecting certain types
against deduction (e.g. D97742).

(Btw, I've really looked at the instances in <memory> and I think they cannot trivially be adjusted to use __identity_t; it's using __dependent_type<__identity<T>, Dummy>::type and thus relying on the fact that __identity<T> is non-final even when T is final. I have not really looked at the instances in <variant>; I think it's using __identity<T> as an empty tag type for dispatch, similar to in_place_type_t<T>, but I'm not technically 100% sure.)

Diff Detail

Event Timeline

• Quuxplusone requested review of this revision.Mar 3 2021, 8:20 AM
• Quuxplusone created this revision.
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 3 2021, 8:20 AM
Herald added a subscriber: libcxx-commits. · View Herald Transcript
• Quuxplusone added inline comments.Mar 3 2021, 8:22 AM
libcxx/include/type_traits
4046

(The first diff here is to eliminate a "noise" use of __identity where IMHO it's not really serving any purpose. The second and third diffs are for consistency of style between __result_of_mp and __result_of_mdp.)

• Quuxplusone mentioned this in D97742: [C++23] [P1518R2] Better CTAD behavior for containers with allocators.Mar 3 2021, 8:24 AM
ldionne added a parent revision: D97742: [C++23] [P1518R2] Better CTAD behavior for containers with allocators.Mar 3 2021, 10:18 AM
Harbormaster completed remote builds in B91826: Diff 327806.Mar 3 2021, 10:19 AM
ldionne removed a parent revision: D97742: [C++23] [P1518R2] Better CTAD behavior for containers with allocators.Mar 3 2021, 10:19 AM
ldionne added a child revision: D97742: [C++23] [P1518R2] Better CTAD behavior for containers with allocators.
curdeius accepted this revision.Mar 3 2021, 12:58 PM
curdeius added a subscriber: curdeius.

LGTM given that all the compilers support alias templates in C++03 mode, which seems to be the case for clang at least.

ldionne accepted this revision.Mar 3 2021, 1:33 PM

LGTM too. I don't understand why those are needed at all though, but Clang does fail to pass the tests if I remove them. So ship it.

This revision is now accepted and ready to land.Mar 3 2021, 1:33 PM
zoecarver added a comment.Mar 3 2021, 2:31 PM

LGTM. I really liked _Ident<T> but I think your rationale in D97742 is correct.

zoecarver accepted this revision.Mar 3 2021, 2:31 PM
ldionne added a comment.Mar 3 2021, 2:43 PM

This is non-blocking, however I think it might be nicer to use __identity_t on the arguments of the constructors that need them instead of on the typedefs. Otherwise, it's really unclear *why* this convolution is needed. So for example, you could have

unordered_multiset(size_type __n, const __identity_t<allocator_type>& __a)
        : unordered_multiset(__n, hasher(), key_equal(), __a) {}

Feel free to do it now, do it later, or not do it at all.

• Quuxplusone added inline comments.Mar 3 2021, 7:22 PM
libcxx/include/map
908

@ldionne wrote:

This is non-blocking, however I think it might be nicer to use __identity_t on the arguments of the constructors that need them instead of on the typedefs. Otherwise, it's really unclear why this convolution is needed.

I agree, but when I started trying to do that, I immediately ran into existing implementation divergence; see https://godbolt.org/z/bxhn84 (libstdc++ rejects, libc++ and MSVC accept) and my tonight's LWG reflector thread "P1518 redux: CTAD deducing Allocator/Compare from random constructors". I definitely think the libc++/MSVC behavior is the "correct" behavior, so I'd want to preserve it, even if it means putting __identity_t on more ctor arguments than the paper standard currently mandates.

But now that's a feature-ish enough change that I'm gonna just land this "NFCI" commit as-is, and open a different PR if I decide it's worth doing anything else (which I still might, but it's no longer a slam-dunk).

This revision was landed with ongoing or failed builds.Mar 3 2021, 7:24 PM
Closed by commit rG09fa1d0e50a3: [libc++] Introduce __identity_t<T>. NFCI. (authored by • arthur.j.odwyer). · Explain Why
This revision was automatically updated to reflect the committed changes.
• arthur.j.odwyer added a commit: rG09fa1d0e50a3: [libc++] Introduce __identity_t<T>. NFCI..
ldionne added inline comments.Mar 4 2021, 4:49 AM
libcxx/include/map
908

Okay, I agree with that. Thanks for keeping changes small and separated!

• Quuxplusone removed a child revision: D97742: [C++23] [P1518R2] Better CTAD behavior for containers with allocators.Jun 17 2021, 9:43 AM

Revision Contents

PathSize
libcxx/
include/
8 lines
8 lines
9 lines
12 lines
12 lines

Diff 327989

libcxx/include/map

Show First 20 Lines • Show All 898 Lines • ▼ Show 20 Linestemplate <class _Key, class _Tp, class _Compare = less<_Key>,
class _Allocator = allocator<pair<const _Key, _Tp> > > class _Allocator = allocator<pair<const _Key, _Tp> > >
class _LIBCPP_TEMPLATE_VIS map class _LIBCPP_TEMPLATE_VIS map
{ {
public: public:
// types: // types:
typedef _Key key_type; typedef _Key key_type;
typedef _Tp mapped_type; typedef _Tp mapped_type;
typedef pair<const key_type, mapped_type> value_type; typedef pair<const key_type, mapped_type> value_type;
typedef typename __identity<_Compare>::type key_compare; typedef __identity_t<_Compare> key_compare;
typedef typename __identity<_Allocator>::type allocator_type; typedef __identity_t<_Allocator> allocator_type;
QuuxplusoneAuthorUnsubmitted
Done
ReplyInline Actions

@ldionne wrote:

This is non-blocking, however I think it might be nicer to use __identity_t on the arguments of the constructors that need them instead of on the typedefs. Otherwise, it's really unclear why this convolution is needed.

I agree, but when I started trying to do that, I immediately ran into existing implementation divergence; see https://godbolt.org/z/bxhn84 (libstdc++ rejects, libc++ and MSVC accept) and my tonight's LWG reflector thread "P1518 redux: CTAD deducing Allocator/Compare from random constructors". I definitely think the libc++/MSVC behavior is the "correct" behavior, so I'd want to preserve it, even if it means putting __identity_t on more ctor arguments than the paper standard currently mandates.

But now that's a feature-ish enough change that I'm gonna just land this "NFCI" commit as-is, and open a different PR if I decide it's worth doing anything else (which I still might, but it's no longer a slam-dunk).

Quuxplusone: @ldionne wrote: > This is non-blocking, however I think it might be nicer to use `__identity_t`…
ldionneUnsubmitted
Not Done
ReplyInline Actions

Okay, I agree with that. Thanks for keeping changes small and separated!

ldionne: Okay, I agree with that. Thanks for keeping changes small and separated!
typedef value_type& reference; typedef value_type& reference;
typedef const value_type& const_reference; typedef const value_type& const_reference;
static_assert((is_same<typename allocator_type::value_type, value_type>::value), static_assert((is_same<typename allocator_type::value_type, value_type>::value),
"Allocator::value_type must be same type as value_type"); "Allocator::value_type must be same type as value_type");
class _LIBCPP_TEMPLATE_VIS value_compare class _LIBCPP_TEMPLATE_VIS value_compare
: public binary_function<value_type, value_type, bool> : public binary_function<value_type, value_type, bool>
▲ Show 20 Lines • Show All 752 Lines • ▼ Show 20 Linestemplate <class _Key, class _Tp, class _Compare = less<_Key>,
class _Allocator = allocator<pair<const _Key, _Tp> > > class _Allocator = allocator<pair<const _Key, _Tp> > >
class _LIBCPP_TEMPLATE_VIS multimap class _LIBCPP_TEMPLATE_VIS multimap
{ {
public: public:
// types: // types:
typedef _Key key_type; typedef _Key key_type;
typedef _Tp mapped_type; typedef _Tp mapped_type;
typedef pair<const key_type, mapped_type> value_type; typedef pair<const key_type, mapped_type> value_type;
typedef typename __identity<_Compare>::type key_compare; typedef __identity_t<_Compare> key_compare;
typedef typename __identity<_Allocator>::type allocator_type; typedef __identity_t<_Allocator> allocator_type;
typedef value_type& reference; typedef value_type& reference;
typedef const value_type& const_reference; typedef const value_type& const_reference;
static_assert((is_same<typename allocator_type::value_type, value_type>::value), static_assert((is_same<typename allocator_type::value_type, value_type>::value),
"Allocator::value_type must be same type as value_type"); "Allocator::value_type must be same type as value_type");
class _LIBCPP_TEMPLATE_VIS value_compare class _LIBCPP_TEMPLATE_VIS value_compare
: public binary_function<value_type, value_type, bool> : public binary_function<value_type, value_type, bool>
▲ Show 20 Lines • Show All 569 Lines • Show Last 20 Lines

libcxx/include/set

Show First 20 Lines • Show All 444 Lines • ▼ Show 20 Lines
class _LIBCPP_TEMPLATE_VIS set class _LIBCPP_TEMPLATE_VIS set
{ {
public: public:
// types: // types:
typedef _Key key_type; typedef _Key key_type;
typedef key_type value_type; typedef key_type value_type;
typedef _Compare key_compare; typedef _Compare key_compare;
typedef key_compare value_compare; typedef key_compare value_compare;
typedef typename __identity<_Allocator>::type allocator_type; typedef __identity_t<_Allocator> allocator_type;
typedef value_type& reference; typedef value_type& reference;
typedef const value_type& const_reference; typedef const value_type& const_reference;
static_assert((is_same<typename allocator_type::value_type, value_type>::value), static_assert((is_same<typename allocator_type::value_type, value_type>::value),
"Allocator::value_type must be same type as value_type"); "Allocator::value_type must be same type as value_type");
private: private:
typedef __tree<value_type, value_compare, allocator_type> __base; typedef __tree<value_type, value_compare, allocator_type> __base;
▲ Show 20 Lines • Show All 507 Lines • ▼ Show 20 Lines
#endif #endif
template <class _Key, class _Compare = less<_Key>, template <class _Key, class _Compare = less<_Key>,
class _Allocator = allocator<_Key> > class _Allocator = allocator<_Key> >
class _LIBCPP_TEMPLATE_VIS multiset class _LIBCPP_TEMPLATE_VIS multiset
{ {
public: public:
// types: // types:
typedef _Key key_type; typedef _Key key_type;
typedef key_type value_type; typedef key_type value_type;
typedef _Compare key_compare; typedef _Compare key_compare;
typedef key_compare value_compare; typedef key_compare value_compare;
typedef typename __identity<_Allocator>::type allocator_type; typedef __identity_t<_Allocator> allocator_type;
typedef value_type& reference; typedef value_type& reference;
typedef const value_type& const_reference; typedef const value_type& const_reference;
static_assert((is_same<typename allocator_type::value_type, value_type>::value), static_assert((is_same<typename allocator_type::value_type, value_type>::value),
"Allocator::value_type must be same type as value_type"); "Allocator::value_type must be same type as value_type");
private: private:
typedef __tree<value_type, value_compare, allocator_type> __base; typedef __tree<value_type, value_compare, allocator_type> __base;
▲ Show 20 Lines • Show All 510 Lines • Show Last 20 Lines

libcxx/include/type_traits

Show First 20 Lines • Show All 519 Lines • ▼ Show 20 Lines
using _IsValidExpansion _LIBCPP_NODEBUG_TYPE = decltype(__sfinae_test_impl<_Templ, _Args...>(0)); using _IsValidExpansion _LIBCPP_NODEBUG_TYPE = decltype(__sfinae_test_impl<_Templ, _Args...>(0));
template <class> template <class>
struct __void_t { typedef void type; }; struct __void_t { typedef void type; };
template <class _Tp> template <class _Tp>
struct __identity { typedef _Tp type; }; struct __identity { typedef _Tp type; };
template <class _Tp>
using __identity_t _LIBCPP_NODEBUG_TYPE = typename __identity<_Tp>::type;
template <class _Tp, bool> template <class _Tp, bool>
struct _LIBCPP_TEMPLATE_VIS __dependent_type : public _Tp {}; struct _LIBCPP_TEMPLATE_VIS __dependent_type : public _Tp {};
template <bool _Bp, class _If, class _Then> template <bool _Bp, class _If, class _Then>
struct _LIBCPP_TEMPLATE_VIS conditional {typedef _If type;}; struct _LIBCPP_TEMPLATE_VIS conditional {typedef _If type;};
template <class _If, class _Then> template <class _If, class _Then>
struct _LIBCPP_TEMPLATE_VIS conditional<false, _If, _Then> {typedef _Then type;}; struct _LIBCPP_TEMPLATE_VIS conditional<false, _If, _Then> {typedef _Then type;};
▲ Show 20 Lines • Show All 3,481 Lines • ▼ Show 20 Lines
template <class _MP, class _Tp, bool _IsMemberFunctionPtr> template <class _MP, class _Tp, bool _IsMemberFunctionPtr>
struct __result_of_mp; struct __result_of_mp;
// member function pointer // member function pointer
template <class _MP, class _Tp> template <class _MP, class _Tp>
struct __result_of_mp<_MP, _Tp, true> struct __result_of_mp<_MP, _Tp, true>
: public __identity<typename __member_pointer_traits<_MP>::_ReturnType>
{ {
using type = typename __member_pointer_traits<_MP>::_ReturnType;
}; };
// member data pointer // member data pointer
template <class _MP, class _Tp, bool> template <class _MP, class _Tp, bool>
struct __result_of_mdp; struct __result_of_mdp;
template <class _Rp, class _Class, class _Tp> template <class _Rp, class _Class, class _Tp>
struct __result_of_mdp<_Rp _Class::*, _Tp, false> struct __result_of_mdp<_Rp _Class::*, _Tp, false>
{ {
typedef typename __apply_cv<decltype(*_VSTD::declval<_Tp>()), _Rp>::type& type; using type = typename __apply_cv<decltype(*_VSTD::declval<_Tp>()), _Rp>::type&;
}; };
template <class _Rp, class _Class, class _Tp> template <class _Rp, class _Class, class _Tp>
struct __result_of_mdp<_Rp _Class::*, _Tp, true> struct __result_of_mdp<_Rp _Class::*, _Tp, true>
{ {
typedef typename __apply_cv<_Tp, _Rp>::type& type; using type = typename __apply_cv<_Tp, _Rp>::type&;
QuuxplusoneAuthorUnsubmitted
Done
ReplyInline Actions

(The first diff here is to eliminate a "noise" use of __identity where IMHO it's not really serving any purpose. The second and third diffs are for consistency of style between __result_of_mp and __result_of_mdp.)

Quuxplusone: (The first diff here is to eliminate a "noise" use of `__identity` where IMHO it's not really…
}; };
template <class _Rp, class _Class, class _Tp> template <class _Rp, class _Class, class _Tp>
struct __result_of_mp<_Rp _Class::*, _Tp, false> struct __result_of_mp<_Rp _Class::*, _Tp, false>
: public __result_of_mdp<_Rp _Class::*, _Tp, : public __result_of_mdp<_Rp _Class::*, _Tp,
is_base_of<_Class, typename remove_reference<_Tp>::type>::value> is_base_of<_Class, typename remove_reference<_Tp>::type>::value>
{ {
}; };
▲ Show 20 Lines • Show All 456 Lines • Show Last 20 Lines

libcxx/include/unordered_map

Show First 20 Lines • Show All 914 Lines • ▼ Show 20 Lines
template <class _Key, class _Tp, class _Hash = hash<_Key>, class _Pred = equal_to<_Key>, template <class _Key, class _Tp, class _Hash = hash<_Key>, class _Pred = equal_to<_Key>,
class _Alloc = allocator<pair<const _Key, _Tp> > > class _Alloc = allocator<pair<const _Key, _Tp> > >
class _LIBCPP_TEMPLATE_VIS unordered_map class _LIBCPP_TEMPLATE_VIS unordered_map
{ {
public: public:
// types // types
typedef _Key key_type; typedef _Key key_type;
typedef _Tp mapped_type; typedef _Tp mapped_type;
typedef typename __identity<_Hash>::type hasher; typedef __identity_t<_Hash> hasher;
typedef typename __identity<_Pred>::type key_equal; typedef __identity_t<_Pred> key_equal;
typedef typename __identity<_Alloc>::type allocator_type; typedef __identity_t<_Alloc> allocator_type;
typedef pair<const key_type, mapped_type> value_type; typedef pair<const key_type, mapped_type> value_type;
typedef value_type& reference; typedef value_type& reference;
typedef const value_type& const_reference; typedef const value_type& const_reference;
static_assert((is_same<value_type, typename allocator_type::value_type>::value), static_assert((is_same<value_type, typename allocator_type::value_type>::value),
"Invalid allocator::value_type"); "Invalid allocator::value_type");
private: private:
typedef __hash_value_type<key_type, mapped_type> __value_type; typedef __hash_value_type<key_type, mapped_type> __value_type;
▲ Show 20 Lines • Show All 918 Lines • ▼ Show 20 Lines
template <class _Key, class _Tp, class _Hash = hash<_Key>, class _Pred = equal_to<_Key>, template <class _Key, class _Tp, class _Hash = hash<_Key>, class _Pred = equal_to<_Key>,
class _Alloc = allocator<pair<const _Key, _Tp> > > class _Alloc = allocator<pair<const _Key, _Tp> > >
class _LIBCPP_TEMPLATE_VIS unordered_multimap class _LIBCPP_TEMPLATE_VIS unordered_multimap
{ {
public: public:
// types // types
typedef _Key key_type; typedef _Key key_type;
typedef _Tp mapped_type; typedef _Tp mapped_type;
typedef typename __identity<_Hash>::type hasher; typedef __identity_t<_Hash> hasher;
typedef typename __identity<_Pred>::type key_equal; typedef __identity_t<_Pred> key_equal;
typedef typename __identity<_Alloc>::type allocator_type; typedef __identity_t<_Alloc> allocator_type;
typedef pair<const key_type, mapped_type> value_type; typedef pair<const key_type, mapped_type> value_type;
typedef value_type& reference; typedef value_type& reference;
typedef const value_type& const_reference; typedef const value_type& const_reference;
static_assert((is_same<value_type, typename allocator_type::value_type>::value), static_assert((is_same<value_type, typename allocator_type::value_type>::value),
"Invalid allocator::value_type"); "Invalid allocator::value_type");
private: private:
typedef __hash_value_type<key_type, mapped_type> __value_type; typedef __hash_value_type<key_type, mapped_type> __value_type;
▲ Show 20 Lines • Show All 721 Lines • Show Last 20 Lines

libcxx/include/unordered_set

Show First 20 Lines • Show All 405 Lines • ▼ Show 20 Lines
template <class _Value, class _Hash = hash<_Value>, class _Pred = equal_to<_Value>, template <class _Value, class _Hash = hash<_Value>, class _Pred = equal_to<_Value>,
class _Alloc = allocator<_Value> > class _Alloc = allocator<_Value> >
class _LIBCPP_TEMPLATE_VIS unordered_set class _LIBCPP_TEMPLATE_VIS unordered_set
{ {
public: public:
// types // types
typedef _Value key_type; typedef _Value key_type;
typedef key_type value_type; typedef key_type value_type;
typedef typename __identity<_Hash>::type hasher; typedef __identity_t<_Hash> hasher;
typedef typename __identity<_Pred>::type key_equal; typedef __identity_t<_Pred> key_equal;
typedef typename __identity<_Alloc>::type allocator_type; typedef __identity_t<_Alloc> allocator_type;
typedef value_type& reference; typedef value_type& reference;
typedef const value_type& const_reference; typedef const value_type& const_reference;
static_assert((is_same<value_type, typename allocator_type::value_type>::value), static_assert((is_same<value_type, typename allocator_type::value_type>::value),
"Invalid allocator::value_type"); "Invalid allocator::value_type");
private: private:
typedef __hash_table<value_type, hasher, key_equal, allocator_type> __table; typedef __hash_table<value_type, hasher, key_equal, allocator_type> __table;
▲ Show 20 Lines • Show All 679 Lines • ▼ Show 20 Lines
template <class _Value, class _Hash = hash<_Value>, class _Pred = equal_to<_Value>, template <class _Value, class _Hash = hash<_Value>, class _Pred = equal_to<_Value>,
class _Alloc = allocator<_Value> > class _Alloc = allocator<_Value> >
class _LIBCPP_TEMPLATE_VIS unordered_multiset class _LIBCPP_TEMPLATE_VIS unordered_multiset
{ {
public: public:
// types // types
typedef _Value key_type; typedef _Value key_type;
typedef key_type value_type; typedef key_type value_type;
typedef typename __identity<_Hash>::type hasher; typedef __identity_t<_Hash> hasher;
typedef typename __identity<_Pred>::type key_equal; typedef __identity_t<_Pred> key_equal;
typedef typename __identity<_Alloc>::type allocator_type; typedef __identity_t<_Alloc> allocator_type;
typedef value_type& reference; typedef value_type& reference;
typedef const value_type& const_reference; typedef const value_type& const_reference;
static_assert((is_same<value_type, typename allocator_type::value_type>::value), static_assert((is_same<value_type, typename allocator_type::value_type>::value),
"Invalid allocator::value_type"); "Invalid allocator::value_type");
private: private:
typedef __hash_table<value_type, hasher, key_equal, allocator_type> __table; typedef __hash_table<value_type, hasher, key_equal, allocator_type> __table;
▲ Show 20 Lines • Show All 654 Lines • Show Last 20 Lines