On 28/11/17 12:30 -0500, Glen Fernandes wrote:
On Tue, Nov 28, 2017 at 9:24 AM, Jonathan Wakely wrote:
Thanks, Glen, I've committed this to trunk, with one small change to
fix the copyright dates in the new test, to be just 2017.
Thanks!
Because my new hobby is finding uses for if-constexpr, I think we
could have used the detection idiom to do it in a single overload, but
I don't see any reason to prefer that over your implementation:
I was thinking about using if-constexpr with std::is_detected_v but
wondered if it wouldn't be appropriate to use the latter until it
transitions from TS to IS. (But now that you've pointed it out, I
guess an implementation detail like __detected_or_t can live on
forever, even if the detection idiom facilities do not get adopted).
However, more importantly, both this form and yours fails for the
following test case, in two ways:
struct P {
using F = void();
F* operator->() const noexcept { return nullptr; }
};
I'm not sure if this is a bug in our std::pointer_traits, or if the
standard requires the specialization of std::pointer_traits<P> to be
ill-formed (see [pointer.traits.types] p1). We have a problem if it
does require it, and either need to relax the requirements on
pointer_traits, or we need to alter the wording for to_address so that
it doesn't try to use pointer_traits when the specialization would be
ill-formed.
Could both be avoided? That is: I don't know if we need to relax it,
or make to_address tolerate it, if the intent is to require the user
to make P a valid pointer-like type such that pointer_traits<P> is
not ill-formed (by 1. providing an element_type member or 2.
specializing pointer_traits<P>, since P is not a template<class T,
class...> template). Current implementations of __to_address or
__to_raw_pointer that are in use by our library facilities already
have this requirement implicitly (those that use typename
pointer_traits<P>::element_type* as the return type, instead of C++14
auto), so users working with non-raw pointers would already be doing 1
or 2.
OK, that seems reasonable. In that case I think adding a note to the
standard might be useful, to clarify that for the first overload, even
if pointer_traits<Ptr>::to_address(p) is not well-formed, the
specialization pointer_traits<Ptr> must be well-formed (because the
check for to_address(p) can trigger errors outside the immediate
context).
So my test should be changed to have element_type (or specialize
pointer_traits), like so:
#include <memory>
struct P {
using element_type = void();
element_type* operator->() const noexcept { return nullptr; }
};
int main()
{
P p;
std::to_address(p);
}
That compiles, and the bug is that is should fail the static assertion.
Secondly, if I remove that static_assert from <bits/ptr_traits.h> then
the test compiles, which is wrong, because it calls std::to_address on
a function pointer type. That should be ill-formed. The problem is
that the static_assert(!is_function_v<_Ptr>) is in std::to_address and
the implementation actually uses std::__to_address. So I think we want
the !is_function_v<_Ptr> check to be moved to the __to_address(_Ptr*)
overload.
Ah, yes. I'll move the static_assert into that overload (enabled in
C++2a or higher mode, since it uses is_function_v).
Great, thanks.
(Using is_function<_Tp>::value as in your original patch would allow
the assertion to be done for all modes that define __to_address)