Re: [boost] shared_ptr and std::less (was Re: Smart pointer status, directions, thoughts)

From: Peter Dimov (pdimov_at_[hidden])
Date: 2001-05-26 06:41:47

From: "Darin Adler" <darin_at_[hidden]>

> On Thursday, May 24, 2001, at 10:36  AM, Peter Dimov wrote:
>
> >> An alternative choice would be to define the < operator and the other
> >> comparison operators too, being sure to use std::less on the underlying
> >> pointer rather than using the < operator. But I don't think this would
be
> >> an improvement.
> >
> > Why?
>
> Well, this is only my opinion (and apparently Andrei's too, given what he
> said in his book). I'll try to be clear here. I'm not trying to win a
> debate, but just present my thinking in as transparent a way as possible.

It's not about winning a debate; I wanted to hear the rationale behind the
decision.

> When I was looking to make a change to ensure that shared_ptr's could be
> used in associative containers, I sought the minimum change that would
> accomplish this. I think that it's nice to retain the feature that this
> code:
>
> shared_ptr<C> a;
> shared_ptr<C> b;
> // ...
> if (a < b)
>
> still fails to compile, since this can help a little for cases where the
> programmer meant:
>
> if (*a < *b)

I'm with you so far, but I want to point out that when a programmer writes

if(a < b)

and (s)he really meant 'if (a < b)', this programmer's first reaction when
faced with the error would be to rewrite this as:

if(a.get() < b.get())

which is not what you'd want - although it's most probably going to work!
The programmer would never consider the correct alternative

if(std::less<shared_ptr<X> >()(a, b))

> I think this helps make clear the distinction between a meaningful
> ordering, and one which is "good enough to build an associative container
> on".

I'm familiar with this argument, yes. Its proponents usually provide a
separate comparison predicate when they don't deem an ordering "worthy" to
be called operator<. Specializing std::less violates 20.3.3/5 + 17.4.3.1/1.

> I see the std::less function as being analogous to the hash
> computation for a hashed associative collection: a way to pay the price of
> admission to the world of indexing, without necessarily changing anything
> else about the objects involved.

I understand this point of view; I think it's justified; my point is not
that it's flawed in some way, but that it's not currently reflected by the
Standard.

> I guess this would be even more clear if
> the standard provided an additional level of indirection for associative
> container key comparison -- another standard function template that calls
> through to std::less by default.

Exactly. But this is not the case.

> But I think this is a subject on which reasonable people can disagree,
> with no clear "right" and "wrong".

Yes, I think so, too. :-) It's amazing how these little details generate
such violent discussions. :-)

My opinion on the matter is that when you have two alternatives and one of
them introduces undefined behavior, this alternative must be clearly
superior to the other for being chosen. Moreover, the rationale behind the
decision must be clearly documented, because programmers often write code by
example; so specializing std::less in such a way indicates that this
approach has the 'boost approval seal' upon it and this kind of undefined
behavior can be safely ignored; in effect, this means that boost either
thinks there is a defect in the Standard, or places itself above the other
programmers.

> Having a < operator defined with a different semantic from the one
> implemented by std::less would be a big mistake. But not having a <
> operator declared means that people who want to use smart pointer
> comparison need to be a bit more explicit, which could be a good thing.
> The presence of std::less means we don't unduly harm generic programming.

Unfortunately we do harm generic programming. Non-generic programming
doesn't care whether a std::set<shared_ptr> is spelled std::set<shared_ptr,
shared_ptr_less>. Only generic algorithms - potentially - rely on the
knowledge that std::less is operator<.

For example, you can have a std::set of shared_ptrs,but the set_ algorithms
don't work on it.

> The question seems to boil down to whether the ability to use standard
> algorithms like std::sort in the simplest way, being able to use other
> generic code that uses the < operator rather than std::less, and being
> able to use the comparison operators directly on shared_ptr's outweigh the
> benefit of catching errors in cases where shared_ptr's are unintentionally
> compared.

Yes, this is one aspect of the issue.

There's also the question that some might want to legitimately compare
shared_ptrs.

> In my opinion, the important issue is being able to use shared_ptr's in
> associative containers in the simplest way when the desired semantic is
> matching by pointer. That's why I added the std::less overload (not
> specialization, I guess) for shared_ptr. Whether shared_ptr's can also be
> compared in other contexts seemed less important to me, but I get the
> impression that it's important to you, so perhaps we should consider a
> change.

No, the ability to compare shared_ptr's in other contexts is not important
to me, although it might be important to others.

The important thing to me is that the Standard currently says that std::less
is operator<, and responds with undefined behavior when this constraint is
not met.

I am also of the opinion that tying associative containers to std::less by
default is a mistake, but I think that this deficiency should be addressed
by a defect report or a redesign, not by introducing a precedent via
[std::]shared_ptr.

--
Peter Dimov
Multi Media Ltd.