[libc++][ranges][NFC] Refactor tests for `ranges::{begin,end}`.

[var-const]

Summary

• add some test cases for cbegin/cend;
• make class definitions generally follow the order in which they are used;
• add a missing include.

[• Quuxplusone]

LGTM % comments (thanks for the cleanup!) but the merge-conflict comment is significant.

libcxx/test/std/ranges/range.access/begin.pass.cpp

32–41

Looks like you had a merge conflict here. Please take what's on the left-hand side.
Your new lines 39 and 41 are the IFNDR case (right?), so we can't static_assert anything about them; we must LIBCPP_STATIC_ASSERT as seen on the left-hand side.

210–215

FWIW, there's no difference between BeginFunctionReturnsInt and BeginFunctionReturnsEmpty, so I'd be perfectly happy to remove one or the other (and all else being equal, I'd remove BeginFunctionReturnsEmpty because then I could perhaps also remove Empty).
(I do think it's worth testing the pointer-but-not-iterator type void* separately from a not-even-pointer non-iterator type like int or Empty.)

libcxx/test/std/ranges/range.access/end.pass.cpp

155

FWIW, here the main problem is that iterator_t<EmptyEndMember> is ill-formed because Empty is not an iterator type; we never even really get around to checking whether t.end() is well-formed. So I question the usefulness of this test case. But it's harmless.

257

Twice now I've looked at this PR, and twice I've been fooled by this "clever" line — "What? There's no private member here, just two friends!" Consider eliminating the inheritance relationship here, so that this code will more closely resemble BeginFunctionWithPrivateBeginMember above.

[var-const]

Address feedback and rebase on main.

libcxx/test/std/ranges/range.access/begin.pass.cpp

32–41

Thanks! Sorry, yeah, I accidentally overwrote the changes on main.

libcxx/test/std/ranges/range.access/end.pass.cpp

155

I agree. It's a preexistent test case, but I'm quite open to deleting it -- just let me know if you'd prefer to do it in this patch.

[var-const]

Rebase

[var-const]

@Quuxplusone I'll go ahead and merge based on the understanding that this patch was LGTM to you modulo the comments I've addressed. My rbegin patch depends on this one and is approved, so I'd rather not let it go stale. If you have any additional feedback, I'll be happy to address in a follow-up. Thanks!

libcxx/include/__ranges/access.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #ifndef _LIBCPP___RANGES_ACCESS_H
 #define _LIBCPP___RANGES_ACCESS_H
 
 #include <__concepts/class_or_enum.h>
 #include <__config>
 #include <__iterator/concepts.h>
 #include <__iterator/readable_traits.h>
 #include <__ranges/enable_borrowed_range.h>
-#include <__utility/as_const.h>
 #include <__utility/auto_cast.h>
 #include <concepts>
 #include <type_traits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 

libcxx/test/std/ranges/range.access/begin.pass.cpp

 // UNSUPPORTED: libcpp-has-no-incomplete-ranges
 
 // std::ranges::begin
 // std::ranges::cbegin
 
 #include <ranges>
 
 #include <cassert>
+#include <utility>
 #include "test_macros.h"
 #include "test_iterators.h"
 
 using RangeBeginT = decltype(std::ranges::begin);
 using RangeCBeginT = decltype(std::ranges::cbegin);
 
 static int globalBuff[8];
 
 static_assert(!std::is_invocable_v<RangeBeginT, int (&&)[10]>);
 static_assert( std::is_invocable_v<RangeBeginT, int (&)[10]>);
 static_assert(!std::is_invocable_v<RangeBeginT, int (&&)[]>);
 static_assert( std::is_invocable_v<RangeBeginT, int (&)[]>);
+static_assert(!std::is_invocable_v<RangeCBeginT, int (&&)[10]>);
+static_assert( std::is_invocable_v<RangeCBeginT, int (&)[10]>);
+static_assert(!std::is_invocable_v<RangeCBeginT, int (&&)[]>);
+static_assert( std::is_invocable_v<RangeCBeginT, int (&)[]>);
 
 struct Incomplete;
 static_assert(!std::is_invocable_v<RangeBeginT, Incomplete(&&)[]>);
 static_assert(!std::is_invocable_v<RangeBeginT, const Incomplete(&&)[]>);
 static_assert(!std::is_invocable_v<RangeCBeginT, Incomplete(&&)[]>);
 static_assert(!std::is_invocable_v<RangeCBeginT, const Incomplete(&&)[]>);
 
 static_assert(!std::is_invocable_v<RangeBeginT, Incomplete(&&)[10]>);
 static_assert(!std::is_invocable_v<RangeBeginT, const Incomplete(&&)[10]>);
 static_assert(!std::is_invocable_v<RangeCBeginT, Incomplete(&&)[10]>);
 static_assert(!std::is_invocable_v<RangeCBeginT, const Incomplete(&&)[10]>);
 
 // This case is IFNDR; we handle it SFINAE-friendly.
 LIBCPP_STATIC_ASSERT(!std::is_invocable_v<RangeBeginT, Incomplete(&)[]>);
constexpr bool testArray() {
 
   BeginMember c[2];
   assert(std::ranges::begin(c) == c);
   assert(std::ranges::cbegin(c) == c);
 
   return true;
 }
 
-struct BeginMemberFunction {
-  int x;
-  constexpr const int *begin() const { return &x; }
-  friend int *begin(BeginMemberFunction const&);
-};
-
 struct BeginMemberReturnsInt {
   int begin() const;
 };
 static_assert(!std::is_invocable_v<RangeBeginT, BeginMemberReturnsInt const&>);
 
 struct BeginMemberReturnsVoidPtr {
   const void *begin() const;
 };
 static_assert(!std::is_invocable_v<RangeBeginT, BeginMemberReturnsVoidPtr const&>);
 
 struct EmptyBeginMember {
   struct iterator {};
   iterator begin() const;
 };
 static_assert(!std::is_invocable_v<RangeBeginT, EmptyBeginMember const&>);
 
-struct EmptyPtrBeginMember {
-  struct Empty {};
-  Empty x;
-  constexpr const Empty *begin() const { return &x; }
-};
-
 struct PtrConvertibleBeginMember {
   struct iterator { operator int*() const; };
   iterator begin() const;
 };
 static_assert(!std::is_invocable_v<RangeBeginT, PtrConvertibleBeginMember const&>);
 
 struct NonConstBeginMember {
   int x;
   constexpr int *begin() { return &x; }
 };
 static_assert( std::is_invocable_v<RangeBeginT,  NonConstBeginMember &>);
 static_assert(!std::is_invocable_v<RangeBeginT,  NonConstBeginMember const&>);
 static_assert(!std::is_invocable_v<RangeCBeginT, NonConstBeginMember &>);
 static_assert(!std::is_invocable_v<RangeCBeginT, NonConstBeginMember const&>);
 
 struct EnabledBorrowingBeginMember {
   constexpr int *begin() const { return &globalBuff[0]; }
 };
 template<>
 inline constexpr bool std::ranges::enable_borrowed_range<EnabledBorrowingBeginMember> = true;
 
+struct BeginMemberFunction {
+  int x;
+  constexpr const int *begin() const { return &x; }
+  friend int *begin(BeginMemberFunction const&);
+};
+
+struct EmptyPtrBeginMember {
+  struct Empty {};
+  Empty x;
+  constexpr const Empty *begin() const { return &x; }
+};
+
 constexpr bool testBeginMember() {
   BeginMember a;
   assert(std::ranges::begin(a) == &a.x);
   assert(std::ranges::cbegin(a) == &a.x);
   static_assert(!std::is_invocable_v<RangeBeginT, BeginMember&&>);
   static_assert(!std::is_invocable_v<RangeCBeginT, BeginMember&&>);
 
   NonConstBeginMember b;
struct BeginFunction {
   friend constexpr const int *begin(BeginFunction const& bf) { return &bf.x; }
 };
 static_assert( std::is_invocable_v<RangeBeginT,  BeginFunction const&>);
 static_assert(!std::is_invocable_v<RangeBeginT,  BeginFunction &&>);
 static_assert(!std::is_invocable_v<RangeBeginT,  BeginFunction &>);
 static_assert( std::is_invocable_v<RangeCBeginT, BeginFunction const&>);
 static_assert( std::is_invocable_v<RangeCBeginT, BeginFunction &>);
 
-struct BeginFunctionWithDataMember {
-  int x;
-  int begin;
-  friend constexpr const int *begin(BeginFunctionWithDataMember const& bf) { return &bf.x; }
+struct BeginFunctionReturnsInt {
+  friend int begin(BeginFunctionReturnsInt const&);
 };
+static_assert(!std::is_invocable_v<RangeBeginT, BeginFunctionReturnsInt const&>);
 
-struct BeginFunctionWithPrivateBeginMember {
-  int y;
-  friend constexpr const int *begin(BeginFunctionWithPrivateBeginMember const& bf) { return &bf.y; }
-private:
-  const int *begin() const;
+struct BeginFunctionReturnsVoidPtr {
+  friend void *begin(BeginFunctionReturnsVoidPtr const&);
 };
+static_assert(!std::is_invocable_v<RangeBeginT, BeginFunctionReturnsVoidPtr const&>);
 
-struct BeginFunctionReturnsEmptyPtr {
-  struct Empty {};
-  Empty x;
-  friend constexpr const Empty *begin(BeginFunctionReturnsEmptyPtr const& bf) { return &bf.x; }
+struct BeginFunctionReturnsPtrConvertible {
+  struct iterator { operator int*() const; };
+  friend iterator begin(BeginFunctionReturnsPtrConvertible const&);
 };
+static_assert(!std::is_invocable_v<RangeBeginT, BeginFunctionReturnsPtrConvertible const&>);
 
 struct BeginFunctionByValue {
   friend constexpr int *begin(BeginFunctionByValue) { return &globalBuff[1]; }
 };
 static_assert(!std::is_invocable_v<RangeCBeginT, BeginFunctionByValue>);
 
 struct BeginFunctionEnabledBorrowing {
   friend constexpr int *begin(BeginFunctionEnabledBorrowing) { return &globalBuff[2]; }
 };
 template<>
 inline constexpr bool std::ranges::enable_borrowed_range<BeginFunctionEnabledBorrowing> = true;
 
-struct BeginFunctionReturnsInt {
-  friend int begin(BeginFunctionReturnsInt const&);
-};
-static_assert(!std::is_invocable_v<RangeBeginT, BeginFunctionReturnsInt const&>);
-
-struct BeginFunctionReturnsVoidPtr {
-  friend void *begin(BeginFunctionReturnsVoidPtr const&);
+struct BeginFunctionReturnsEmptyPtr {
+  struct Empty {};
+  Empty x;
+  friend constexpr const Empty *begin(BeginFunctionReturnsEmptyPtr const& bf) { return &bf.x; }
 };
-static_assert(!std::is_invocable_v<RangeBeginT, BeginFunctionReturnsVoidPtr const&>);
 
-struct BeginFunctionReturnsEmpty {
-  struct Empty {};
-  friend Empty begin(BeginFunctionReturnsEmpty const&);
+struct BeginFunctionWithDataMember {
+  int x;
+  int begin;
+  friend constexpr const int *begin(BeginFunctionWithDataMember const& bf) { return &bf.x; }
 };
-static_assert(!std::is_invocable_v<RangeBeginT, BeginFunctionReturnsEmpty const&>);
 
-struct BeginFunctionReturnsPtrConvertible {
-  struct iterator { operator int*() const; };
-  friend iterator begin(BeginFunctionReturnsPtrConvertible const&);
+struct BeginFunctionWithPrivateBeginMember {
+  int y;
+  friend constexpr const int *begin(BeginFunctionWithPrivateBeginMember const& bf) { return &bf.y; }
+private:
+  const int *begin() const;
 };
-static_assert(!std::is_invocable_v<RangeBeginT, BeginFunctionReturnsPtrConvertible const&>);
 
 constexpr bool testBeginFunction() {
   BeginFunction a{};
   const BeginFunction aa{};
   static_assert(!std::invocable<RangeBeginT, decltype((a))>);
   assert(std::ranges::cbegin(a) == &a.x);
   assert(std::ranges::begin(aa) == &aa.x);
   assert(std::ranges::cbegin(aa) == &aa.x);

libcxx/test/std/ranges/range.access/end.pass.cpp

 // UNSUPPORTED: libcpp-has-no-incomplete-ranges
 
 // std::ranges::end
 // std::ranges::cend
 
 #include <ranges>
 
 #include <cassert>
+#include <utility>
 #include "test_macros.h"
 #include "test_iterators.h"
 
 using RangeEndT = decltype(std::ranges::end);
 using RangeCEndT = decltype(std::ranges::cend);
 
 static int globalBuff[8];
 
 static_assert(!std::is_invocable_v<RangeEndT, int (&&)[]>);
 static_assert(!std::is_invocable_v<RangeEndT, int (&)[]>);
 static_assert(!std::is_invocable_v<RangeEndT, int (&&)[10]>);
 static_assert( std::is_invocable_v<RangeEndT, int (&)[10]>);
+static_assert(!std::is_invocable_v<RangeCEndT, int (&&)[]>);
+static_assert(!std::is_invocable_v<RangeCEndT, int (&)[]>);
+static_assert(!std::is_invocable_v<RangeCEndT, int (&&)[10]>);
+static_assert( std::is_invocable_v<RangeCEndT, int (&)[10]>);
 
 struct Incomplete;
 static_assert(!std::is_invocable_v<RangeEndT, Incomplete(&&)[]>);
 static_assert(!std::is_invocable_v<RangeEndT, Incomplete(&&)[42]>);
 static_assert(!std::is_invocable_v<RangeCEndT, Incomplete(&&)[]>);
 static_assert(!std::is_invocable_v<RangeCEndT, Incomplete(&&)[42]>);
 
 struct EndMember {
constexpr bool testArray() {
 
   EndMember c[2];
   assert(std::ranges::end(c) == c + 2);
   assert(std::ranges::cend(c) == c + 2);
 
   return true;
 }
 
-struct EndMemberFunction {
-  int x;
-  constexpr const int *begin() const { return nullptr; }
-  constexpr const int *end() const { return &x; }
-  friend constexpr int *end(EndMemberFunction const&);
-};
-
 struct EndMemberReturnsInt {
   int begin() const;
   int end() const;
 };
-
 static_assert(!std::is_invocable_v<RangeEndT, EndMemberReturnsInt const&>);
 
 struct EndMemberReturnsVoidPtr {
   const void *begin() const;
   const void *end() const;
 };
-
 static_assert(!std::is_invocable_v<RangeEndT, EndMemberReturnsVoidPtr const&>);
 
-struct Empty { };
-struct EmptyEndMember {
-  Empty begin() const;
-  Empty end() const;
-};
-struct EmptyPtrEndMember {
-  Empty x;
-  constexpr const Empty *begin() const { return nullptr; }
-  constexpr const Empty *end() const { return &x; }
-};
-
-static_assert(!std::is_invocable_v<RangeEndT, EmptyEndMember const&>);
-
 struct PtrConvertible {
   operator int*() const;
 };
 struct PtrConvertibleEndMember {
   PtrConvertible begin() const;
   PtrConvertible end() const;
 };
-
 static_assert(!std::is_invocable_v<RangeEndT, PtrConvertibleEndMember const&>);
 
 struct NoBeginMember {
   constexpr const int *end();
 };
-
 static_assert(!std::is_invocable_v<RangeEndT, NoBeginMember const&>);
 
 struct NonConstEndMember {
   int x;
   constexpr int *begin() { return nullptr; }
   constexpr int *end() { return &x; }
 };
-
 static_assert( std::is_invocable_v<RangeEndT,  NonConstEndMember &>);
 static_assert(!std::is_invocable_v<RangeEndT,  NonConstEndMember const&>);
 static_assert(!std::is_invocable_v<RangeCEndT, NonConstEndMember &>);
 static_assert(!std::is_invocable_v<RangeCEndT, NonConstEndMember const&>);
 
 struct EnabledBorrowingEndMember {
   constexpr int *begin() const { return nullptr; }
   constexpr int *end() const { return &globalBuff[0]; }
 };
 
 template<>
 inline constexpr bool std::ranges::enable_borrowed_range<EnabledBorrowingEndMember> = true;
 
+struct EndMemberFunction {
+  int x;
+  constexpr const int *begin() const { return nullptr; }
+  constexpr const int *end() const { return &x; }
+  friend constexpr int *end(EndMemberFunction const&);
+};
+
+struct Empty { };
+struct EmptyEndMember {
+  Empty begin() const;
+  Empty end() const;
+};
+static_assert(!std::is_invocable_v<RangeEndT, EmptyEndMember const&>);
+
+struct EmptyPtrEndMember {
+  Empty x;
+  constexpr const Empty *begin() const { return nullptr; }
+  constexpr const Empty *end() const { return &x; }
+};
+
 constexpr bool testEndMember() {
   EndMember a;
   assert(std::ranges::end(a) == &a.x);
   assert(std::ranges::cend(a) == &a.x);
 
   NonConstEndMember b;
   assert(std::ranges::end(b) == &b.x);
   static_assert(!std::is_invocable_v<RangeCEndT, decltype((b))>);

 static_assert(!std::is_invocable_v<RangeEndT, EndFunction &&>);
 
 static_assert( std::is_invocable_v<RangeEndT,  EndFunction const&>);
 static_assert(!std::is_invocable_v<RangeEndT,  EndFunction &&>);
 static_assert(!std::is_invocable_v<RangeEndT,  EndFunction &>);
 static_assert( std::is_invocable_v<RangeCEndT, EndFunction const&>);
 static_assert( std::is_invocable_v<RangeCEndT, EndFunction &>);
 
-struct EndFunctionWithDataMember {
-  int x;
-  int end;
-  friend constexpr const int *begin(EndFunctionWithDataMember const&) { return nullptr; }
-  friend constexpr const int *end(EndFunctionWithDataMember const& bf) { return &bf.x; }
-};
-
-struct EndFunctionWithPrivateEndMember : private EndMember {
-  int y;
-  friend constexpr const int *begin(EndFunctionWithPrivateEndMember const&) { return nullptr; }
-  friend constexpr const int *end(EndFunctionWithPrivateEndMember const& bf) { return &bf.y; }
-};
-
-struct EndFunctionReturnsEmptyPtr {
-  Empty x;
-  friend constexpr const Empty *begin(EndFunctionReturnsEmptyPtr const&) { return nullptr; }
-  friend constexpr const Empty *end(EndFunctionReturnsEmptyPtr const& bf) { return &bf.x; }
-};
-
-struct EndFunctionByValue {
-  friend constexpr int *begin(EndFunctionByValue) { return nullptr; }
-  friend constexpr int *end(EndFunctionByValue) { return &globalBuff[1]; }
-};
-
-static_assert(!std::is_invocable_v<RangeCEndT, EndFunctionByValue>);
-
-struct EndFunctionEnabledBorrowing {
-  friend constexpr int *begin(EndFunctionEnabledBorrowing) { return nullptr; }
-  friend constexpr int *end(EndFunctionEnabledBorrowing) { return &globalBuff[2]; }
-};
-
-template<>
-inline constexpr bool std::ranges::enable_borrowed_range<EndFunctionEnabledBorrowing> = true;
-
 struct EndFunctionReturnsInt {
   friend constexpr int begin(EndFunctionReturnsInt const&);
   friend constexpr int end(EndFunctionReturnsInt const&);
 };
-
 static_assert(!std::is_invocable_v<RangeEndT, EndFunctionReturnsInt const&>);
 
 struct EndFunctionReturnsVoidPtr {
   friend constexpr void *begin(EndFunctionReturnsVoidPtr const&);
   friend constexpr void *end(EndFunctionReturnsVoidPtr const&);
 };
-
 static_assert(!std::is_invocable_v<RangeEndT, EndFunctionReturnsVoidPtr const&>);
 
 struct EndFunctionReturnsEmpty {
   friend constexpr Empty begin(EndFunctionReturnsEmpty const&);
   friend constexpr Empty end(EndFunctionReturnsEmpty const&);
 };
-
 static_assert(!std::is_invocable_v<RangeEndT, EndFunctionReturnsEmpty const&>);
 
 struct EndFunctionReturnsPtrConvertible {
   friend constexpr PtrConvertible begin(EndFunctionReturnsPtrConvertible const&);
   friend constexpr PtrConvertible end(EndFunctionReturnsPtrConvertible const&);
 };
-
 static_assert(!std::is_invocable_v<RangeEndT, EndFunctionReturnsPtrConvertible const&>);
 
 struct NoBeginFunction {
   friend constexpr const int *end(NoBeginFunction const&);
 };
-
 static_assert(!std::is_invocable_v<RangeEndT, NoBeginFunction const&>);
 
+struct EndFunctionByValue {
+  friend constexpr int *begin(EndFunctionByValue) { return nullptr; }
+  friend constexpr int *end(EndFunctionByValue) { return &globalBuff[1]; }
+};
+static_assert(!std::is_invocable_v<RangeCEndT, EndFunctionByValue>);
+
+struct EndFunctionEnabledBorrowing {
+  friend constexpr int *begin(EndFunctionEnabledBorrowing) { return nullptr; }
+  friend constexpr int *end(EndFunctionEnabledBorrowing) { return &globalBuff[2]; }
+};
+template<>
+inline constexpr bool std::ranges::enable_borrowed_range<EndFunctionEnabledBorrowing> = true;
+
+struct EndFunctionReturnsEmptyPtr {
+  Empty x;
+  friend constexpr const Empty *begin(EndFunctionReturnsEmptyPtr const&) { return nullptr; }
+  friend constexpr const Empty *end(EndFunctionReturnsEmptyPtr const& bf) { return &bf.x; }
+};
+
+struct EndFunctionWithDataMember {
+  int x;
+  int end;
+  friend constexpr const int *begin(EndFunctionWithDataMember const&) { return nullptr; }
+  friend constexpr const int *end(EndFunctionWithDataMember const& bf) { return &bf.x; }
+};
+
+struct EndFunctionWithPrivateEndMember {
+  int y;
+  friend constexpr const int *begin(EndFunctionWithPrivateEndMember const&) { return nullptr; }
+  friend constexpr const int *end(EndFunctionWithPrivateEndMember const& bf) { return &bf.y; }
+private:
+  const int *end() const;
+};
+
 struct BeginMemberEndFunction {
   int x;
   constexpr const int *begin() const { return nullptr; }
   friend constexpr const int *end(BeginMemberEndFunction const& bf) { return &bf.x; }
 };
 
 constexpr bool testEndFunction() {
   const EndFunction a{};