On Fri, Jan 22, 2021 at 6:40 AM Giuseppe D'Angelo via Std-Proposals <
std-proposals_at_[hidden]> wrote:

> I've got a draft for a proposal that allows for comparisons between a
> smart pointer and a raw pointer.
>
[but deliberately doesn't touch std::hash]

Since your Tony Tables do mention std::hash, I'd like to see your paper
specifically in one place list the guarantees that we already have about
that stuff in C++20.
For example:

- Does `uptr.get() == p` already imply that `std::hash<>()(uptr) ==
std::hash<>()(p)`?
- Does `sptr.get() == p` imply that `std::hash<>()(sptr) ==
std::hash<>()(p)`?
- Does `uptr.get() == sptr.get()` imply that `std::hash<>()(uptr) ==
std::hash<>()(sptr)`?
- Does `wptr.lock() == sptr` imply that `std::hash<>()(wptr) ==
std::hash<>()(sptr)`?

(std::hash doesn't actually have a transparent std::hash<> specialization,
but you can just imagine that I was unlazy enough to write out the correct
types in all of those angle brackets.)

Coincidentally, I was just dealing with the same situation literally
yesterday with respect to hash<string> and hash<string_view>. We have our
own hasher to overcome the shortcomings of std::hash, but we delegate to
std::hash in a few places, notably

template<> struct hash<std::string> {
    size_t operator()(const std::string& s) const { return
std::hash<std::string_view>()(s); }
};
template<> struct hash<std::string_view> {
    size_t operator()(const std::string_view& s) const { return
std::hash<std::string_view>()(s); }
};

Notice that we delegate to hash<string_view> in both cases. This has two or
three benefits:
(1) It eliminates one unnecessary template instantiation; we never need to
instantiate hash<string>.
(2) It clarifies intent and assures the human reader that yes we really
intend these hashers to return compatible hash values.
(3?) It enforces our intent, just in the very unlikely case that the
standard library's hash<string> and hash<string_view> are non-conformingly
incompatible.

I recommend that you adopt the same strategy in your Tony Table code, both
before and after. Delegate to hash<T*> in every case.


As for the comparison operators that you *are* proposing, personally I'm
skeptical. Your Tony Table has three rows:
(1) The first row is definitely the most convincing. However, nits: Your
`isEqual` lambda should capture [&] instead of [input], since it's not
doing anything tricky that would require deviating from the usual practice.
Also, avoid ADL; see
https://quuxplusone.github.io/blog/2020/07/08/erase-if/#ah-so-should-i-maybe-treat-erase
. I would write something like
    std::erase_if(objects, [&](auto&& p) { return p.get() == input; });
or else factor it out into a helper function so you don't have to repeat
the body of the lambda multiple times:
    static auto isEqualTo(const Object *input) {
        return [=](auto&& p) { return p.get() == input; };
    }
    [...]
    std::erase_if(objects, isEqualTo(input));
Likewise, the right-hand side should use `std::erase`, not ADL `erase`.
(2) The second row, with `std::set<unique_ptr>`, is completely
unconvincing, because the *point* of unique_ptr is that *of course* you'll
never find `objects.contains(ptr)` to be true; that would be possible only
if `ptr` pointed to an element of the set already. So that row should be
rewritten and/or expanded to make it clearer what you're trying to
accomplish.
(3) The third row, with `smart_pointer_hasher`, is less convincing than I'd
like, because you're not trying to solve the hashing problem at the same
time. If I had a choice between "unordered_set with custom hasher and
custom equality predicate," versus "unordered_set with custom hasher and
*default* equality predicate," I feel like I'd have a hard time deciding
which one I preferred. Hashing and equality-comparison usually go together
as a package, to the point where I sometimes write them in a single struct
— operator()(const T&) for hashing, operator()(const T&, const T&) for
equality.

I am *very mildly* concerned that you're asking for this to compile. But
surprisingly-compiling-code is just a natural side-effect of any time we
weaken the type system:
    std::shared_ptr<char> sptr = std::make_shared<char>('a');
    assert(sptr != "hello world");

Your current proposed wording makes this ambiguous AFAICT, but (A) nobody
should write this, and (B) this can always be fixed by heroic fiddliness in
LWG, as is tradition:
    // https://godbolt.org/z/69nzMv
    std::shared_ptr<int> sptr = std::make_shared<int>(42);
    assert(sptr != NULL);

This proposal violates my personal sense of how value semantics ought to
work, in the sense that you're asking to permit
    x == y
even while disallowing both
    x = y;
    y = x;
I can't think off the top of my head of any existing pairs of types where
this is true.
We have plenty of pairs where the assignment (implicit conversion) is
permitted in one direction but not the other, e.g. string/string_view,
int/optional<int>, nullptr_t/int*.
There was a proposal several years ago to make vector<int>/vector<float>
such a pair, but it wasn't adopted. It would be a good idea for you to look
up that discussion and link it from your paper. I believe the proposer was
Marshall Clow.

You refer to an LEWG reflector discussion started by Marc Mutz. Could you
link to that thread in the paper, or at least give the committee *some*
hint about how to find that previous discussion? (Also, linking to this
std-proposals thread would be cool too.)

TLDR, I have nitpicks on your Tony Tables, and I personally wouldn't favor
this proposal because I think it is a half-solution to a very marginal
problem; but I don't see any *fatal* flaw in it.

–Arthur