[libcxx-commits] [PATCH] D101638: [libc++] std::to_address mustn't depend on P::element_type.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp:32
+    assert(std::to_address(c.begin()) == c.data());
+    assert(std::to_address(c.end()) == c.data() + c.size());
+    assert(std::to_address(cc.begin()) == cc.data());
----------------
zoecarver wrote:
> zoecarver wrote:
> > @Quuxplusone this is the reason the test is failing in debug mode, we can't dereference end. Do we ever really want to be dereferencing end anyway, isn't that UB? I don't think we need to test this, at least not in debug mode. 
> Actually on second thought: we're not actually dereferencing anything here. I guess when the assertion was added to `operator->` they didn't expect the caller of the operator to be using the pointer itself. So I feel like there's a good argument to remove the assertion all together. @EricWF, thoughts?
FYI @zoecarver, I believe you've got it now. But I don't think "remove the assertion" is correct. I think, (well, as you know I think) `pointer_traits` was just a dumb place to put this customization point (it should have been ADL like `swap`), but, I think the whole intention of //letting// the user customize `to_address` was //precisely// to give them a way to solve this exact problem. They have an `operator->` that is unsafe to call on non-dereferenceable pointers, so they should customize `to_address`.

"They/the user" here means `__wrap_iter`, of course, which means us.

IOW, the answer is clearly to have libc++ provide a partial specialization of `pointer_traits<__wrap_iter<U>>` and maintain it forever. That's clearly the right thing to do. I resist it only because it's so, so dumb.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101638