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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante 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>
----------------
Quuxplusone wrote:
> 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.)
Yes and adding a structured bindings test is a good idea!


================
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));
----------------
Quuxplusone wrote:
> 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.
Seems there's always a blog post :-)
I thought `[[no_unique_address]]` allows it to be 1. Reading your blog and looking in the standard I see
the answer in [intro.object]/9 "Two objects with overlapping lifetimes that are not bit-fields may have the same address if one is nested within the other, or if at least one is a subobject of zero size and they are of different types; otherwise, they have distinct addresses and occupy disjoint bytes of storage."

Thanks.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116278