Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement LWG-3545 std::pointer_traits should be SFINAE-friendly #3242

Merged
merged 3 commits into from
Dec 6, 2022
Merged

Implement LWG-3545 std::pointer_traits should be SFINAE-friendly #3242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
753969e
Implement LWG-3545
frederick-vs-ja Nov 21, 2022
d5e325a
Add test files for LWG-3545
frederick-vs-ja Nov 21, 2022
fb04d3f
Add `LWG3545_pointer_traits_sfinae` to list
frederick-vs-ja Nov 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions stl/inc/xutility
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,16 +210,6 @@ struct _Replace_first_parameter<_Newfirst, _Ty<_First, _Rest...>> { // given _Ty
using type = _Ty<_Newfirst, _Rest...>;
};

template <class _Ty, class = void>
struct _Get_element_type {
using type = typename _Get_first_parameter<_Ty>::type;
};

template <class _Ty>
struct _Get_element_type<_Ty, void_t<typename _Ty::element_type>> {
using type = typename _Ty::element_type;
};

template <class _Ty, class = void>
struct _Get_ptr_difference_type {
using type = ptrdiff_t;
Expand Down Expand Up @@ -281,23 +271,37 @@ void _Default_construct_in_place(_Ty& _Obj) noexcept(is_nothrow_default_construc
::new (_Voidify_iter(_STD addressof(_Obj))) _Ty;
}

_EXPORT_STD template <class _Ty>
struct pointer_traits {
template <class _Ty, class _Elem>
struct _Ptr_traits_base {
using pointer = _Ty;
using element_type = typename _Get_element_type<_Ty>::type;
using element_type = _Elem;
using difference_type = typename _Get_ptr_difference_type<_Ty>::type;

template <class _Other>
using rebind = typename _Get_rebind_alias<_Ty, _Other>::type;

using _Reftype = conditional_t<is_void_v<element_type>, char, element_type>&;
using _Reftype = conditional_t<is_void_v<_Elem>, char, _Elem>&;

_NODISCARD static _CONSTEXPR20 pointer pointer_to(_Reftype _Val) noexcept(
noexcept(_Ty::pointer_to(_Val))) /* strengthened */ { // Per LWG-3454
return _Ty::pointer_to(_Val);
}
};

template <class, class = void, class = void>
struct _Ptr_traits_sfinae_layer {};

template <class _Ty, class _Uty>
struct _Ptr_traits_sfinae_layer<_Ty, _Uty, void_t<typename _Get_first_parameter<_Ty>::type>>
: _Ptr_traits_base<_Ty, typename _Get_first_parameter<_Ty>::type> {};

template <class _Ty>
struct _Ptr_traits_sfinae_layer<_Ty, void_t<typename _Ty::element_type>, void>
: _Ptr_traits_base<_Ty, typename _Ty::element_type> {};
Comment on lines +291 to +300
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this seems to basically do as follows:

The third template specialization (right line 298) is the most specific of the three, so if it's valid, it'll be chosen, and our element_type will be _Ty::element_type.

If the third template specialization is not chosen, because _Ty::element_type is not a type name, then the second (right line 294) is still more specific than the first (right line 291), and if it is valid, we'll choose it.

Finally, otherwise, if _Ty is not a class template with only type arguments, we fall back to having no members.

This makes sense, and I think correctly implements the wording in the standard.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I believe it could have been written with both void_ts in the same position, but this way is fine too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't necessarily believe this is true, since if typename _Get_first_parameter<_Ty>::type and typename _Ty::element_type are both well formed (which they often are, like allocator<_Ty>), then both specializations are valid and equally specialized.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, what I'm envisioning is having the partial specializations be <_Ty, void_t<ONE>, _Uty> and <_Ty, void_t<TWO>, void>. In that case, when ONE and TWO are both well-formed, the partial specialization with void as the last argument is more specialized than the one with _Uty as the last argument.


_EXPORT_STD template <class _Ty>
struct pointer_traits : _Ptr_traits_sfinae_layer<_Ty> {};

template <class _Ty>
struct pointer_traits<_Ty*> {
using pointer = _Ty*;
Expand Down
1 change: 1 addition & 0 deletions tests/std/test.lst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ tests\LWG3146_excessive_unwrapping_ref_cref
tests\LWG3234_math_special_overloads
tests\LWG3422_seed_seq_ctors
tests\LWG3480_directory_iterator_range
tests\LWG3545_pointer_traits_sfinae
tests\LWG3610_iota_view_size_and_integer_class
tests\P0019R8_atomic_ref
tests\P0024R2_parallel_algorithms_adjacent_difference
Expand Down
4 changes: 4 additions & 0 deletions tests/std/tests/LWG3545_pointer_traits_sfinae/env.lst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

RUNALL_INCLUDE ..\usual_matrix.lst
92 changes: 92 additions & 0 deletions tests/std/tests/LWG3545_pointer_traits_sfinae/test.compile.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include <cstddef>
#include <memory>
#include <type_traits>

using namespace std;

#define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__);

template <class, class = void>
constexpr bool has_memtype_element_type = false;

template <class T>
constexpr bool has_memtype_element_type<T, void_t<typename T::element_type>> = true;

template <class, class = void>
constexpr bool has_memtype_pointer = false;

template <class T>
constexpr bool has_memtype_pointer<T, void_t<typename T::pointer>> = true;

template <class, class = void>
constexpr bool has_memtype_difference_type = false;

template <class T>
constexpr bool has_memtype_difference_type<T, void_t<typename T::difference_type>> = true;

STATIC_ASSERT(!has_memtype_element_type<pointer_traits<int>>);
STATIC_ASSERT(!has_memtype_pointer<pointer_traits<int>>);
STATIC_ASSERT(!has_memtype_difference_type<pointer_traits<int>>);

struct LackElementType {
using pointer = int;
using difference_type = int;
template <class>
using rebind = int;
};

STATIC_ASSERT(!has_memtype_element_type<pointer_traits<LackElementType>>);
STATIC_ASSERT(!has_memtype_pointer<pointer_traits<LackElementType>>);
STATIC_ASSERT(!has_memtype_difference_type<pointer_traits<LackElementType>>);

struct OnlyElementType {
using element_type = void;
};

STATIC_ASSERT(has_memtype_element_type<pointer_traits<OnlyElementType>>);
STATIC_ASSERT(has_memtype_pointer<pointer_traits<OnlyElementType>>);
STATIC_ASSERT(has_memtype_difference_type<pointer_traits<OnlyElementType>>);

STATIC_ASSERT(is_same_v<pointer_traits<OnlyElementType>::element_type, void>);
STATIC_ASSERT(is_same_v<pointer_traits<OnlyElementType>::pointer, OnlyElementType>);
STATIC_ASSERT(is_same_v<pointer_traits<OnlyElementType>::difference_type, ptrdiff_t>);

template <class I>
struct Templated {
using difference_type = I;
};

STATIC_ASSERT(has_memtype_element_type<pointer_traits<Templated<char>>>);
STATIC_ASSERT(has_memtype_pointer<pointer_traits<Templated<char>>>);
STATIC_ASSERT(has_memtype_difference_type<pointer_traits<Templated<char>>>);

STATIC_ASSERT(is_same_v<pointer_traits<Templated<char>>::element_type, char>);
STATIC_ASSERT(is_same_v<pointer_traits<Templated<char>>::pointer, Templated<char>>);
STATIC_ASSERT(is_same_v<pointer_traits<Templated<char>>::difference_type, char>);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change requested: Because this test case says using difference_type = I;, it's unable to distinguish whether pointer_traits is reading the template argument (as it did for element_type) or the difference_type typedef (as desired, which it is indeed doing). Using a unique type for the difference_type, like int, would make the test slightly stronger and clearer.

Below, CheckPriority's using element_type = T[42]; is a good example of detecting what pointer_traits is looking at.

I mention this only for future reference, as there is essentially zero risk in this specific case. This is related to my general recommendation for test data, which is "unique numbers for unique purposes".


template <class I, size_t N = sizeof(I)>
struct BadTemplated {
using difference_type = I;
};

STATIC_ASSERT(!has_memtype_element_type<pointer_traits<BadTemplated<int>>>);
STATIC_ASSERT(!has_memtype_pointer<pointer_traits<BadTemplated<int>>>);
STATIC_ASSERT(!has_memtype_difference_type<pointer_traits<BadTemplated<int>>>);

template <class T>
struct CheckPriority {
using element_type = T[42];
};

STATIC_ASSERT(has_memtype_element_type<pointer_traits<CheckPriority<char>>>);
STATIC_ASSERT(has_memtype_pointer<pointer_traits<CheckPriority<char>>>);
STATIC_ASSERT(has_memtype_difference_type<pointer_traits<CheckPriority<char>>>);

STATIC_ASSERT(is_same_v<pointer_traits<CheckPriority<char>>::element_type, char[42]>);
STATIC_ASSERT(is_same_v<pointer_traits<CheckPriority<char>>::pointer, CheckPriority<char>>);
STATIC_ASSERT(is_same_v<pointer_traits<CheckPriority<char>>::difference_type, ptrdiff_t>);

int main() {} // COMPILE-ONLY