Page MenuHomePhabricator
Differential D99141 Diff 335200 libcxx/include/iterator

Changeset View

Standalone View

View Options

libcxx/include/iterator

// -*- C++ -*- // -*- C++ -*-
//===-------------------------- iterator ----------------------------------===// //===-------------------------- iterator ----------------------------------===//
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#ifndef _LIBCPP_ITERATOR #ifndef _LIBCPP_ITERATOR
#define _LIBCPP_ITERATOR #define _LIBCPP_ITERATOR
/* /*
iterator synopsis iterator synopsis
#include <concepts>
namespace std namespace std
MordanteUnsubmitted
Not Done
ReplyInline Actions

Can you add the required includes?
http://eel.is/c++draft/iterator.synopsis

#include <compare>              // see [compare.syn]
#include <concepts>             // see [concepts.syn]
Mordante: Can you add the required includes? http://eel.is/c++draft/iterator.synopsis ``` #include…
cjdbAuthorUnsubmitted
Done
ReplyInline Actions

I've added it for <concepts>, but will leave <compare> for the person who adds that functionality (probably me in a future patch).

cjdb: I've added it for `<concepts>`, but will leave `<compare>` for the person who adds that…
{ {
template<class> struct incrementable_traits; // since C++20
template<class Iterator> template<class Iterator>
struct iterator_traits struct iterator_traits
{ {
typedef typename Iterator::difference_type difference_type; typedef typename Iterator::difference_type difference_type;
typedef typename Iterator::value_type value_type; typedef typename Iterator::value_type value_type;
typedef typename Iterator::pointer pointer; typedef typename Iterator::pointer pointer;
typedef typename Iterator::reference reference; typedef typename Iterator::reference reference;
▲ Show 20 Lines • Show All 392 Lines • ▼ Show 20 Lines
#include <iosfwd> // for forward declarations of vector and string. #include <iosfwd> // for forward declarations of vector and string.
#include <__functional_base> #include <__functional_base>
#include <type_traits> #include <type_traits>
#include <cstddef> #include <cstddef>
#include <initializer_list> #include <initializer_list>
#include <__memory/base.h> #include <__memory/base.h>
#include <__memory/pointer_traits.h> #include <__memory/pointer_traits.h>
#include <version> #include <version>
#include <concepts>
MordanteUnsubmitted
Not Done
ReplyInline Actions

Maybe also add compare while you're at it.

Mordante: Maybe also add compare while you're at it.
cjdbAuthorUnsubmitted
Done
ReplyInline Actions

See above.

cjdb: See above.
#include <__debug> #include <__debug>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
#pragma GCC system_header #pragma GCC system_header
#endif #endif
_LIBCPP_BEGIN_NAMESPACE_STD _LIBCPP_BEGIN_NAMESPACE_STD
#if !defined(_LIBCPP_HAS_NO_RANGES)
// [incrementable.traits]
template<class> struct incrementable_traits {};
template<class _Tp>
requires is_object_v<_Tp>
thakisUnsubmitted
Not Done
ReplyInline Actions

This breaks using this header with -std=c++14 apparently: http://45.33.8.238/linux/43983/step_4.txt

Looks like _LIBCPP_HAS_NO_RANGES is supposed to be defined in that case, but maybe it doesn't work right?

thakis: This breaks using this header with `-std=c++14` apparently: http://45.33.8.
thakisUnsubmitted
Not Done
ReplyInline Actions

Oh wait, this might be a bot config problem. Please ignore for now.

thakis: Oh wait, this might be a bot config problem. Please ignore for now.
thakisUnsubmitted
Not Done
ReplyInline Actions

Yes, was a problem on my end. All good now, sorry for the noise.

thakis: Yes, was a problem on my end. All good now, sorry for the noise.
struct incrementable_traits<_Tp*> {
using difference_type = ptrdiff_t;
};
template<class _Ip>
struct incrementable_traits<const _Ip> : incrementable_traits<_Ip> {};
template<class _Tp>
concept __has_member_difference_type = requires { typename _Tp::difference_type; };
template<__has_member_difference_type _Tp>
struct incrementable_traits<_Tp> {
using difference_type = typename _Tp::difference_type;
};
QuuxplusoneUnsubmitted
Done
ReplyInline Actions

This is the only reason <iterator> needs to include <concepts> in C++20 mode, right?
@ldionne, how about we just use is_integral_v here instead?

template<class _Tp>
concept __has_integral_minus =
  !__has_difference_type_member<_Tp> &&
  requires(const _Tp& __x, const _Tp& __y) {
    requires is_integral_v<decltype( __x - __y )>;
  };

(And/or, wait for <concepts> to stop depending on <iterator>; D99041, D99124.)

I note that the name __has_integral_minus is slightly misleading; it currently means __has_integral_minus_without_difference_type. The paper standard doesn't factor out these concepts, so we have no exposition-only names to imitate here. Maybe __incrementable_traits_use_difference_type<_Tp> and __incrementable_traits_use_minus<_Tp> would be better? But I don't really object to the current name even if I do find it "slightly misleading."

Quuxplusone: This is the //only// reason `<iterator>` needs to include `<concepts>` in C++20 mode, right?
cjdbAuthorUnsubmitted
Done
ReplyInline Actions

This is the only reason <iterator> needs to include <concepts> in C++20 mode, right?

For this patch? Yes.

It's a part of the synopsis, so in general, no.

(And/or, wait for <concepts> to stop depending on <iterator>; D99041, D99124.)

That was my intention, hence "Depends on D99041".

The paper standard doesn't factor out these concepts, so we have no exposition-only names to imitate here. Maybe __incrementable_traits_use_difference_type<_Tp> and __incrementable_traits_use_minus<_Tp> would be better?

Right. I think I got the idea of naming these from readable.traits, so __has_member_difference_type (sic) is probably best for consistency. As for __has_integral_minus, I'm ambivalent, so long as it's consistent.

cjdb: > This is the only reason <iterator> needs to include <concepts> in C++20 mode, right? For…
MordanteUnsubmitted
Done
ReplyInline Actions

The synopsis requires concepts http://eel.is/c++draft/iterator.synopsis

Mordante: The synopsis requires concepts http://eel.is/c++draft/iterator.synopsis
template<class _Tp>
concept __has_integral_minus =
ldionneUnsubmitted
Not Done
ReplyInline Actions

IMO this name is misleading because you're really checking whether T has an integral minus AND no member difference type. I think it would be cleaner to take the !__has_member_difference_type out of this helper concept and use it directly below in incrementable_traits.

ldionne: IMO this name is misleading because you're really checking whether `T` has an integral minus…
!__has_member_difference_type<_Tp> &&
requires(const _Tp& __x, const _Tp& __y) {
{ __x - __y } -> integral;
};
template<__has_integral_minus _Tp>
struct incrementable_traits<_Tp> {
using difference_type = make_signed_t<decltype(declval<_Tp>() - declval<_Tp>())>;
QuuxplusoneUnsubmitted
Done
ReplyInline Actions

Just declval<_Tp>(), not std::declval<_Tp>(). (If it had any arguments, then definitely _VSTD::declval to prevent ADL — but it has no arguments. Actually we still have a ton of places that do _VSTD::declval, so I'm okay with that spelling too. Just not std::.)

Quuxplusone: Just `declval<_Tp>()`, not `std::declval<_Tp>()`. (If it had any arguments, then definitely…
cjdbAuthorUnsubmitted
Done
ReplyInline Actions

Eh, ADL isn't a concern here. Removed.

cjdb: Eh, ADL isn't a concern here. Removed.
};
// TODO(cjdb): add iter_difference_t once iterator_traits is cleaned up.
zoecarverUnsubmitted
Not Done
ReplyInline Actions

Hmm. I really feel like we should have a better way to track what is implemented, what is blocked on other patches, etc. TODOs might be OK for now, but I'd like to sort this out.

zoecarver: Hmm. I really feel like we should have a better way to track what is implemented, what is…
cjdbAuthorUnsubmitted
Done
ReplyInline Actions

In general: yes please.

cjdb: In general: yes please.
#endif // !defined(_LIBCPP_HAS_NO_RANGES)
template <class _Iter> template <class _Iter>
struct _LIBCPP_TEMPLATE_VIS iterator_traits; struct _LIBCPP_TEMPLATE_VIS iterator_traits;
struct _LIBCPP_TEMPLATE_VIS input_iterator_tag {}; struct _LIBCPP_TEMPLATE_VIS input_iterator_tag {};
struct _LIBCPP_TEMPLATE_VIS output_iterator_tag {}; struct _LIBCPP_TEMPLATE_VIS output_iterator_tag {};
struct _LIBCPP_TEMPLATE_VIS forward_iterator_tag : public input_iterator_tag {}; struct _LIBCPP_TEMPLATE_VIS forward_iterator_tag : public input_iterator_tag {};
struct _LIBCPP_TEMPLATE_VIS bidirectional_iterator_tag : public forward_iterator_tag {}; struct _LIBCPP_TEMPLATE_VIS bidirectional_iterator_tag : public forward_iterator_tag {};
struct _LIBCPP_TEMPLATE_VIS random_access_iterator_tag : public bidirectional_iterator_tag {}; struct _LIBCPP_TEMPLATE_VIS random_access_iterator_tag : public bidirectional_iterator_tag {};
▲ Show 20 Lines • Show All 1,613 Lines • Show Last 20 Lines