[libcxx-commits] [PATCH] D116278: [libc++][ranges] Add ranges::in_in_result

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/algorithms/algorithms.results/no_unique_address.compile.pass.cpp:17
+// [[no_unique_address]] for [algorithms.results]
+
+#include <algorithm>
----------------
Mordante wrote:
> Since this test is in `std` instead of `libcxx` I think it would be good to mention the class may only have the two members specified in the standard.
> http://eel.is/c++draft/algorithms.results#1
@Mordante: Are you basically asking to replace line 16 with
```
// [algorithms.results]/1
//   Each of the class templates specified in this subclause has the template parameters,
//   data members, and special members specified below, and has no base classes or members
//   other than those specified.
```
? (If so: I like that idea!)

@philnik: The fact that the standard defines exactly two members, both public, probably means that we're mandated to support structured binding:

    [[maybe_unused]] auto [in1, in2] = std::ranges::in_in_result<int, int>{1, 2};

Consider adding that simple test case to `in_in_result.pass.cpp`. (Attn: @var-const, re `in_out_result.pass.cpp`, for consistency.)


================
Comment at: libcxx/test/std/algorithms/algorithms.results/no_unique_address.compile.pass.cpp:24
+static_assert(sizeof(std::ranges::in_in_result<int, Empty>) == sizeof(int));
+static_assert(sizeof(std::ranges::in_in_result<Empty, Empty>) == 2 * sizeof(Empty));
----------------
Mordante wrote:
> Why `2 * sizeof(Empty)`?
Because of the ["empty-base exclusion principle"](https://quuxplusone.github.io/blog/2021/05/07/std-iterator-as-a-base-class/#so-it-contains-two-empty-base-cl).
FWIW, I would have just said `== 2);` since `sizeof`-an-empty-class is `1` by definition AFAIK.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116278/new/

https://reviews.llvm.org/D116278