[libcxx-commits] [PATCH] D130835: [libc++] Fix a hard error in `contiguous_iterator<NoOperatorArrowIter>`.
Konstantin Varlamov via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Aug 4 04:22:53 PDT 2022
var-const added inline comments.
================
Comment at: libcxx/include/__memory/pointer_traits.h:197
template <class _Pointer, class = __enable_if_t<
- !is_pointer<_Pointer>::value && !is_array<_Pointer>::value && !is_function<_Pointer>::value
+ _And<is_class<_Pointer>, _IsFancyPointer<_Pointer> >::value
> >
----------------
ldionne wrote:
> var-const wrote:
> > var-const wrote:
> > > This is more to start a discussion. Would it be easier to list what the `_Pointer` could be rather than what it should *not* be? Are there any other alternatives rather than a class that we would accept here?
> > `_And` is necessary because `enable_if` doesn't short-circuit, so it would still try to evaluate `_IsFancyPointer` on e.g. an array, leading to a hard error when trying to instantiate `pointer_traits<array-type>`.
> Here's a refactoring suggestion:
>
> ```
> template <class _Pointer, class = __enable_if_t<
> _And<is_class<_Pointer>, _HasToAddress<_Pointer> >::value
> > >
> _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
> decay_t<decltype(pointer_traits<_Pointer>::to_address(declval<const _Pointer&>()))>
> __to_address(const _Pointer& __p) _NOEXCEPT {
> return pointer_traits<_Pointer>::to_address(__p);
> }
>
> template <class _Pointer, class = __enable_if_t<
> _And<is_class<_Pointer>, _HasArrow<_Pointer>, _Not<_HasToAddress<_Pointer> > >::value
> > >
> _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
> decltype(std::__to_address(declval<const _Pointer&>().operator->()))
> __to_address(const _Pointer& __p) _NOEXCEPT {
> return std::__to_address(__p.operator->());
> }
> ```
>
> Something like that. And then we could remove `__to_address_helper` altogether -- it feels like it's kind of unnecessary.
Hmm, it looks like the compiler is having trouble with
```
decltype(std::__to_address(declval<const _Pointer&>().operator->()))
```
when the type returned by `_Pointer::operator->` is another fancy pointer with `operator->` defined -- looks like the recursion is causing the issue. `decltype(auto)` works but we can't use it because of the C++03 mode. I wonder if that's the reason the `helper` struct was added in the first place?
================
Comment at: libcxx/include/__memory/pointer_traits.h:179
+struct _HasToAddress<_Pointer,
+ decltype((void)&pointer_traits<_Pointer>::to_address)
+> : true_type {};
----------------
ldionne wrote:
> I think you need to call `pointer_traits<_Pointer>::to_address(...)`, otherwise it could be a function taking no arguments, or an overloaded function (and we can't take the address of those), etc.
>
> It could even be a static data member.
Thanks, good point. This is also consistent with the check below in `__to_address_helper`.
================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.random.access/contiguous_iterator.compile.pass.cpp:237-238
+ self operator+(difference_type n) const;
+ template <bool B1, bool B2>
+ friend no_operator_arrow<B1, B2> operator+(difference_type n, no_operator_arrow<B1, B2> x);
+
----------------
huixie90 wrote:
> does it work without `template <bool B1, bool B2>` and just use `no_operator_arrow`
This is to prevent a GCC warning ("friend declaration declares a non-template function"). Added a comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130835/new/
https://reviews.llvm.org/D130835
More information about the libcxx-commits
mailing list