[libcxx-commits] [PATCH] D117044: [libc++] [ranges] Implement ranges::cdata
Konstantin Varlamov via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Jan 11 19:19:30 PST 2022
var-const accepted this revision.
var-const added a comment.
LGTM (modulo the one agreed-upon style nitpick).
================
Comment at: libcxx/include/__ranges/data.h:76
+
+namespace ranges {
+namespace __cdata {
----------------
Quuxplusone wrote:
> var-const wrote:
> > Is there any reason to close the `ranges` namespace above and then reopen it here?
> https://reviews.llvm.org/D89057#inline-1117677
> (Also consistency with all the other CPOs at this point, but then, I wrote them ;))
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.
================
Comment at: libcxx/include/__ranges/data.h:84
+ noexcept(noexcept(ranges::data(static_cast<const remove_reference_t<_Tp>&>(__t))))
+ -> decltype( ranges::data(static_cast<const remove_reference_t<_Tp>&>(__t)))
+ { return ranges::data(static_cast<const remove_reference_t<_Tp>&>(__t)); }
----------------
Quuxplusone wrote:
> var-const wrote:
> > Question: would `decltype(auto)` work here?
> 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
Thanks for the link!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117044/new/
https://reviews.llvm.org/D117044
More information about the libcxx-commits
mailing list