[libcxx-commits] [PATCH] D109668: [libc++][test] Fix iterator assertion in span.cons/deduct.pass.cpp

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/containers/views/span.cons/deduct.pass.cpp:50
     ASSERT_SAME_TYPE(S, std::span<int, 3>);
     assert((std::equal(std::begin(arr), std::end(arr), s.begin(), s.end())));
     }
----------------
jloser wrote:
> Quuxplusone wrote:
> > For another, the right assertion here and throughout is actually
> > ```
> > assert(s.begin() == arr.begin());
> > assert(s.end() == arr.end());
> > ```
> > That is, we should verify that the span gets the right settings for its members, in terms of the original array, //not// just that it's viewing onto //some// array with the same contents as the original.
> > As a bonus, this eliminates the test's dependency on `<algorithm>`.
> I like that idea, but I don't think that "just works" because of how span's iterators are `__wrap_iter` depending on `_LIBCPP_DEBUG_LEVEL`, so there won't be a `operator==` to make that valid. Example error:
> 
> ```
> error: invalid operands to binary expression ('int *' and 'std::span<int, 3>::iterator' (aka '__wrap_iter<int *>'))
>     assert(std::begin(arr) == s.begin());
> ```
> 
> Is there something I'm missing to make this work regardless of whether the span's iterator type is `pointer` or `__wrap_iter<pointer>`?
Ah, gross. Well, since this is C++20, we can certainly do
```
assert(std::to_address(s.begin()) == arr.begin());
assert(std::to_address(s.begin()) == std::to_address(str.begin()));
```
etc. I guess this does explain why it wasn't written that way to begin with; we implemented `span` probably long before we had a working `std::to_address`. But these days we could use `std::to_address`, unless @ldionne objects for any reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109668