Page MenuHomePhabricator
Differential D117044 Diff 399032 libcxx/include/__ranges/data.h

Changeset View

Standalone View

View Options

libcxx/include/__ranges/data.h

Show First 20 Lines • Show All 65 Lines • ▼ Show 20 Linesnamespace __data {
}; };
} }
inline namespace __cpo { inline namespace __cpo {
inline constexpr auto data = __data::__fn{}; inline constexpr auto data = __data::__fn{};
} // namespace __cpo } // namespace __cpo
} // namespace ranges } // namespace ranges
// [range.prim.cdata]
namespace ranges {
var-constUnsubmitted
Done
ReplyInline Actions

Is there any reason to close the ranges namespace above and then reopen it here?

var-const: Is there any reason to close the `ranges` namespace above and then reopen it here?
QuuxplusoneAuthorUnsubmitted
Done
ReplyInline Actions

https://reviews.llvm.org/D89057#inline-1117677
(Also consistency with all the other CPOs at this point, but then, I wrote them ;))

Quuxplusone: https://reviews.llvm.org/D89057#inline-1117677 (Also consistency with all the other CPOs at…
var-constUnsubmitted
Not Done
ReplyInline Actions

Personally, I'd rather have everything in a single contiguous namespace. When I see this reopening, it makes me wonder whether this file contains other nested namespaces (whatever they might be) and leads me to expand the file and start searching around.

var-const: Personally, I'd rather have everything in a single contiguous namespace. When I see this…
namespace __cdata {
struct __fn {
var-constUnsubmitted
Not Done
ReplyInline Actions

I think we agreed to only indent inside __cpo namespaces, so this struct should not have any extra indentation.

var-const: I think we agreed to only indent inside `__cpo` namespaces, so this struct should not have any…
QuuxplusoneAuthorUnsubmitted
Done
ReplyInline Actions

True, we did. But I guess I only took that as far as reverting the indentation in uninitialized_foo; I didn't actually go remove the existing indentation in begin/end/cbegin/etc. This was copied from cbegin. I'll go remove that existing indentation tomorrow, unless someone else objects before then (which will certainly give me enough of an excuse to avoid doing it, again ;))

Quuxplusone: True, we did. But I guess I only took that as far as reverting the indentation in…
template <class _Tp>
requires is_lvalue_reference_v<_Tp&&>
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI
constexpr auto operator()(_Tp&& __t) const
noexcept(noexcept(ranges::data(static_cast<const remove_reference_t<_Tp>&>(__t))))
-> decltype( ranges::data(static_cast<const remove_reference_t<_Tp>&>(__t)))
var-constUnsubmitted
Done
ReplyInline Actions

Question: would decltype(auto) work here?

var-const: Question: would `decltype(auto)` work here?
QuuxplusoneAuthorUnsubmitted
Done
ReplyInline Actions

No, because we need SFINAE.
This is the "you must write it three times" idiom; see Vittorio Romeo's 2017 lightning talk https://www.youtube.com/watch?v=I3T4lePH-yA

Quuxplusone: No, because we need SFINAE. This is the "you must write it three times" idiom; see Vittorio…
var-constUnsubmitted
Done
ReplyInline Actions

Thanks for the link!

var-const: Thanks for the link!
{ return ranges::data(static_cast<const remove_reference_t<_Tp>&>(__t)); }
philnikUnsubmitted
Done
ReplyInline Actions

The standard only says static_cast<const T&>(), but that doesn't make much sense, so I'm guessing that's a wording defect?

philnik: The standard only says `static_cast<const T&>()`, but that doesn't make much sense, so I'm…
QuuxplusoneAuthorUnsubmitted
Done
ReplyInline Actions

Serendipitously, Casey Carter emphasized the answer just today in D116991. The standard uses T to refer to the "type" of the expression, which is never a reference type (although it may be cv-qualified). On paper, you can have an lvalue expression of type const int, but you can't have an lvalue expression of type const int&. So when in libc++ we take a parameter of declared type _Tp&&, that corresponds to a paper expression E of "type" remove_reference_t<_Tp>.
TLDR the standard is correct as written; the trick is that its notion of "type T" is not the same as our _Tp here.

Quuxplusone: Serendipitously, Casey Carter emphasized the answer [just today in D116991](https://reviews.
template <class _Tp>
requires is_rvalue_reference_v<_Tp&&>
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI
constexpr auto operator()(_Tp&& __t) const
noexcept(noexcept(ranges::data(static_cast<const _Tp&&>(__t))))
-> decltype( ranges::data(static_cast<const _Tp&&>(__t)))
{ return ranges::data(static_cast<const _Tp&&>(__t)); }
};
}
inline namespace __cpo {
inline constexpr auto cdata = __cdata::__fn{};
} // namespace __cpo
} // namespace ranges
#endif // !defined(_LIBCPP_HAS_NO_RANGES) #endif // !defined(_LIBCPP_HAS_NO_RANGES)
_LIBCPP_END_NAMESPACE_STD _LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP___RANGES_DATA_H #endif // _LIBCPP___RANGES_DATA_H