Re: [boost] Formal review: Optional library

From: Fernando Cacciola (fernando_cacciola_at_[hidden])
Date: 2002-12-10 18:23:04

----- Original Message -----
From: "William E. Kempf" <wekempf_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Tuesday, December 10, 2002 4:18 PM
Subject: Re: [boost] Formal review: Optional library

> [snipped]
>
> But I do see reasons to want to keep with a smart pointer design and am
> not (necessarily) trying to convince you to switch to value semantics.
>
Good.

> All I'm saying is that if we retain a smart pointer design, we should be
> faithful to that decision.
>
Fair enough.
In another post, I'll summarize the 'distance' between the current
'smart-pointer-likeness' and a full smart-pointer-like interface
so we can focus on it specifically.
I need the opinion of other reviewers.

>
> > That's why it hands you a pointer to the value, because you can test the
> > pointer
> > for NULL; but that's it.
> > IOWs, the pointer returned by optional is conceptually a handle, not a
> > pointer.
> > That's why I like the member function peek() much better than get().
>
> I still much prefer the standard get(), even if I bought into the rest of
> the rationale.  It's what people expect because of std::auto_ptr.
>
I see.
I counted 2(two) in favor of get(), and one (me :-) in favor of peek().
I'll be democratic on this one, though I hope I can get more counts.

>
> >> As long as there's a
> >> constructor taking a value, it seems logical to me there should be an
> >> assignment taking a value.
> >
> > There is an asymmetry here, certainly.
> > Originally, optional<> did not have a constructor taking a value,
> > precisely in order to keep it symmetry, but it was argued that it
> > cluttered the interface too much, so I eventually bent my mind and added
> > the constructor.
>
> As did I in the interface I proposed elsewhere.  My complaint wasn't
> actually the lack of an operator=(), but of the lack of any way to
> reassign the value in an efficient and uniform manner.
>
But what's wrong with: *opt = new_value?
This is as efficient and as uniform as anything else.

btw: your next response arrived so I can see that you think that *opt=x only
works if opt is initialized...
But it isn't the case!
*opt=x actually initializes the optional if it not initialized. And this is
why the proxy is needed...

But then of course you will say:
why don't you have *opt=x work *only* for initialized optionals, and use
reset() to initialize it.

OK. This could work... I'll think about it further.

> But that's not the case here.  The current interface provides no way to
> _efficiently_ change the value.  (That is, with out regard for whether or
> not the optional has been initialized.)
>
Actually, it does.
*opt = new_value.
changes the value even if it isn't initialized.
But since you didn't expected such a behaviour it turns out that it might be
confusing.

Clearly, if *opt=x only works for initialized optionals, reset() is needed.

So we need to discuss this:  should *opt=x assign even uninitialized
optionals? (as it does now)
Or should reset() do that?

>
> > I know that since you construct an optional with a value, and since you
> > can turn an optional into the uninitialized state, it is actually
> > 'similar' to a container, but there is a difference: when you construct
> > an optional<> with a value, the optional uses the value to initialize
> > its own value, it is not 'takinging it' (it isn't containing it).
> > Similarly, when you 'uninitialize' an optional, it is destroying its
> > value, but it is not taking it out (otherwise uninitialize would return
> > the previosu value).
>
> I think you're trying to convey too much with the naming, with the result
> being that the interface is actually _less_ understandable.
>
Notice that a smart pointer has a reset, but it takes a new pointer, not a
new object value.
If optional<> had a resest, it would be *similar* to that of a smart
pointer,
but just similar because it would not take a pointer.
It would have to be assymetric with get(), for example.
This is why I said that extending the analogy to a smart pointer too much
might lead to confusion.

> >> > If I manage to make the idiom known enough, the user will know that
> >> he can't delete the pointer and that the pointer can be used only as
> >> long as the 'source' (the optional<> object in this case) remains
> >> alive.
> >>
> >> OK, that rationale makes some sense, though I don't think it's that
> >> necessary to deviate the names here to indicate the unusual ownership
> >> semantics.  I think the class name and purpose do this well enough.
> >>
> > Perhaps.
> > But think about the rationale above about optional<> not being a
> > smart-pointer.
>
> I don't agree with that rationale.  If it walks like a duck, and quacks
> like a duck...
>
But does it walk and quack like a duck? or just looks like? :-))
We need to figure this out, and we could use some external input for this...

> > Now I want peek() even more than before, precisely to stress out that is
> > not containing
> > a poiner.
>
> I don't think that peek() conveys that information.  But let's assume it
> does.  Doesn't the name and purpose of optional<> already convey that?
>
That?
Do you understad what that the name peek() is intended to convey?
I will drop it if people want it that way, but I'd like it to be understood
what peek() is about.
It has nothing to do with optional<>, really, is about drawing a distinction
between 'peeking' inside an object as opposed to 'extracting' from it
(with a complete transfer or distribution of ownership).

> And from the point of view of the interface, why does that matter any way?
>  There's a reason why the differences between a list and a vector were
> blurred by the STL, and those reasons make me wonder why you're trying so
> hard to differentiate the differences in this interface.  I see no gain,
> and a loss in readability/understandability of the interface.
>
I'm trying to install a new idiom, but it might be a good idea to try that
somewhere else and not along optional<>.

> >> >> * I'm unsure about the need for "value()".  I'd be happy with just
> >> the normal smart pointer interfaces.
> >> >>
> >> > value() is there just for those contexts in which the conversion
> >> from the proxy (returned by operator*()) to 'operator T&' is not
> >> pick up by the compiler.
> >> > For example, when you pass (*opt) to a template function.
> >> > You could use: *peek(opt), but peek() doesn't test for the
> >> initialize state first,
> >> > as does value().
> >>
> >> OK, I'm not understanding something here.  In lay terms, what's the
> >> difference between:
> >>
> >> o.value() = some_value;
> >>
> >> and
> >>
> >> *o = some_value;
> >>
> >> They seem to serve the same purpose to me, and if so, why does
> >> operator*() need to return a proxy at all?
> >>
> > There are two questions here.
> >
> > (1)
> > operator*() needs to return a proxy in order to allow for assignment:
> > "operator*() const" returns "T const&"
> > but
> > "operator*()" returns a proxy with "operator=(T const&)" defined.
>
> Why is this needed?  You can only call operator*() if the value has been
> initialized, and if it has been there seems no reason to need to detect an
> assignment.
The proxy is needed to allow you to initialize an unintialized optional via:
*opt=x;

> >> >> * I'm unsure about the presence of "initialized()".  On the one
> >> hand, the duplication in features (compared to "get/peek() == 0")
> >> is something I think designs should generally avoid.  On the other
> >> hand, this name is more meaningful for what precisely "get/peek()
> >> == 0" signifies.  I guess I'm +0 on this one.
> >> >>
> >> > To be honest, I dislike it too :-)
> >> > But some people found the alternative spellings ugly,
> >> > so I figured that a member function would make them happy.
> >>
> >> That depends on how much the class follows smart pointer semantics.
> >> After all, std::auto_ptr doesn't contain a null() member, yet no one
> >> is confused by "p.get() == 0".  This is another one I won't argue too
> >> much, though.
> >>
> > optional<> follows pointer semantics just to the extent to which
> > it is convenient in order to handle the uninitialized state, but no
> > further.
>
> Which I think is a mistake.  In fact, it follows the full requirements for
> smart pointers by one definition (as in it supports operator*() and
> operator->(), which is all that's required to qualify as a smart pointer).
>  To not follow the rest of the conventions for the (one) smart pointer in
> the standard seems to be a mistake that just leads to a less
> understandable interface.
But it can't follow the rest really.
It doesn't deal with pointers so it can't have a ctor taking a pointer,
an assignment taking a pointer or a reset taking a pointer.
These methods would not be alike those for smart pointers in spite of their
appearance.

>
> >> >> * I'm a little uncomfortable with the free functions in
> >> >> <boost/optional.hpp>.  With out some rationale for them they seem
> >> to provide nothing and are useless duplications in the public
> >> interface, which will lead to confusion for users.
> >> >>
> >> > I generally dislike member functions for 'general' objects.
> >> > Onn several contexts, the optional<> objects can be replaced by true
> >> pointers directly
> >> > thanks to its pointer semantics (and correspoding syntax)
> >> > With the non-member interface, you can write code that works even if
> >> you change optional<>
> >> > with a true pointer.
> >> > The member function interface is not-recommended.
> >> > It is there only for those situations in which resolution problems
> >> prevent the user
> >> > from using the non-member functions.
> >>
> >> We come from different camps of design, then.  I prefer member
> >> functions for the _minimal_ set of operations, and free functions for
> >> everything else.  Providing both for the same functionality just lends
> >> itself to confusion on the part of the user (for instance, at the very
> >> least they are left wondering if one is more efficient than the other
> >> for some reason).
> >>
> > I actually agree with you.
> > As I said earlier, many member functions are there just in order to
> > circumbent lexical problems in cases were the other non-member functions
> > won't work.
>
> None of the free standing functions provide anything other than a wrapper
> around the member functions.  This violates the design criteria I stated.
>
I think I will remove initialized().
This leaves peek(), which most likely will be renamed get(), and value().
Theoretically, the non-member get() could be made a friend and get() could
be made private,
but unfortunately, lookup rules are such that it is possible that 'get(opt)'
fail to
compile, so opt.get() is required to be kept as a workaround.
Similarly, value() wouldn't exist if operator*() wouldn't return a proxy;
but it does
return a proxy in order to allow deep-constantness so value() is required as
a workaround.

> >> >> * I don't follow the rationale for not having relational
> >> comparisons. When would you ever get a "maybe" state when
> >> comparing?
> >> > Yes, whenever one of the optional<> is uninitialized.
> >>
> >> That's not a maybe.  That's a definate false.  If both were
> > uninitialized...
> >>
> > I disgaree that comparing against an uinitialized optional
> > should yield a definite false.
>
> Does NaN not compare unambiguously with 1?
>
It does compare as it is defined, but it is defined as such, IMO, becasue
it is complicated to implement and deal with a tristate boolean logic
at the processor level.
But optional<> doesn't has to follow NANs logic unless there is a good
reason for it;
and I find out -after having actually use that logic-, that there isn't.

> > Are you thinking of a concrete scenario where this
> > is the behaviour you want?
> > I ask because I originally had optional<> follow precisely this logic,
> > mostly because I initially followed NANs behavior, but it turned out
> > that it didn't work in practice.
>
> If that's the case, then you need to provide the rationale for why it
> doesn't work in practice.
Fair enough.

>  It eludes me how it wouldn't work, since this
> is truly precisely how pointers (which you're emulating) work.
I don't follow. How does pointers work in this way?
If you compare the 'values' of the objects being pointed to by two pointers
and
one of those pointers is NULL, the result is undefined just as in the case
of
optional<>:

int n = 2 ;
int* a = n;
int* b = NULL ;
if ( *a == *b ) // undefined.

unless you think about comparing pointer values as oposed to pointee values:

if ( a == b ) // defined,

but this, anyway, is not defined as 'comparing to NULL yields false'.

(I see again the confusion drawn from pushing the analogy with pointers)

>
> > That is, I never really wanted the comparison to return false; that was
> > just as wrong an answer as "true".
> > Therefore, I always had to make sure the optionals were initialized
> > before even trying to compare them.
> > There wasn't a single ocassion on which I could let an uninitialized
> > optional
> > slip through a comparison with no problems trusting the 'false' branch
> > to take care of it properly.
>
> Why not?  Why would an unitialized int ever be considered the same as an
> initialized int (no matter its value)?

Never.
And just as never an uninitialized int would be considered *different* from
an initilized
int.
The comparison just cannot be made, so the result is neither true nor false.

Think about the interval library for instance.

>
> > I'm convinced now that comparing against an uninitialized optional isn't
> > either true nor false, is undefined.
>
> I'm not.  Convince me.
>
I'll try.
Think about a *concrete* example on which the 'false' branch
in a piece of code is effectively capable of dealing with the fact
that the values were never really compared because one of them was missing.
Make the example such that you don't need to test for initialization first.
That is, something of the form:

if ( *a == *b )
{
}
else
{
  // doesn't matter that *a or *b is actually uninitialized.
}

In the cases I encounter this, it did matter.
I'll see if I found a real example to show you.

> >> >>I can see that
> >> >> when comparing two uninitialized optionals that a case could be
> >> made for either true or false, but that's not a "maybe" as to my
> >> mind you could pick one of them with out causing any ill effects to
> >> anyone, and if you do it seems clear to me that you'd want to
> >> choose true.  I'm not
> >> >> (necessarily) arguing for the comparisons, I'm just questioning the
> >> rationale, at least as it's worded in the document now.
> >> >>
> >> > optional<> is intended to be used as much as a pointer as possible;
> >> therefore
> >> > the syntaxis of its usage is intended to looks like that of a
> >> pointer; i.e., you can't access optional's "value" directly,
> >> > you need operator*() for that.
> >>
> >> That I have no problem with, but that's not the rationale provided!
> >>
> >> > Consider:
> >> >
> >> > int* a;
> >> > int* b;
> >> > if ( a == b ) ..
> >> >
> >> > Altough the comparison is well defined between pointers,
> >> > you are not comparing the "values of the objects" being pointed to,
> >> only the pointer values.
> >> > If you want to compare object values, you must dereference the
> >> pointers: if ( *a == *b ) ..
> >>
> >> Again, this is not the rationale provided in the document.  This
> >> rationale I can get behind and support.
> >>
> > OK.
> > The main reason why optional comparisons are left undefined
> > is what I explained above, but I incidentally came out with an
> > additional argument while answering you...
> > I'll add this to the documentation.
>
> Well, you've not convinced me of the first reason, only of this second.

At least... :-)

>
> >> >> * I also don't follow the rationale for leaving out the conversion
> >> to bool.  I see no conflict with including this conversion, and in
> >> fact inclusion would make this more consistent with other smart
> >> pointers. There's no confusion/conflict created for
> >> boost::smart_ptr<bool>, so I don't see how there could be for
> >> optional<bool> either.
> >> >>
> >> > The conversion to bool for a smart pointer unambiguously refers to
> >> the NULL state of the pointer wrapped.
> >> > In the case of optional, a conversion to bool will be ambiguous in
> >> those cases were the wrapped T in optional<> is bool itself.
> >>
> >> How?  You've made this assertion several times, but I don't follow the
> >> rationale.  The bool conversion for optional<> would, to me,
> >> "unambiguously refer to the UNINITIALIZED state of the value wrapped",
> >> to borrow your wording with the appropriate changes in terminology.  I
> >> see no way in which this can be considered "ambiguous in those case
> >> where the wrapped T in optional<> is bool itself".  I.e. these lines
> >> would not be equivalent:
> >>
> >> if (o)
> >>
> >> and
> >>
> >> if (*o)
> >>
> > OK, I was clear enough.
> > It would be strictly unambiguous, but it would be confusing.
> > The follwing will compile is optional<> had a safe_bool.
> >
> > void foo( bool );
> > void bar()
> > {
> >   optional<bool> opt;
> >
> >   foo(opt); // Oops!  Is this really what you wanted? Or was it:
> > foo(*opt);
> >
> >   bool b = opt ; // Or should this be: b = *opt ;
> >
> >   const int n = 0 ;
> >
> >   if ( opt == n )  // Should this be (*opt == n )?
> > }
> > bool baz()
> > {
> >   optional<bool> opt ;
> >   return opt ; // Oops! Shouldn't this be: return *opt ?
> > }
>
> Yes, and that would be a programmer error ;).  Seriously, this scenario
> would just be one of many relatively rare use cases in C++ that the
> programmer has to be wary of... the price of the power and expressibility
> allowed elsewhere.  I won't retract my yes vote based on this, but I DO
> NOT agree with the rationale and would lobby strongly for providing the
> safe bool.
>
I see.
I'll see what happens during the rest of the review.
If by the end, most of you prefer safe_bool even at the risk of allowing
potential mistakes as above, I'll add it.

> > I've noticed in another post that Peter suggest that preventing this
> > sort of potential mistake does not worth the trouble; I'm not sure.
> > The danger is that all the code above which would compile would have a
> > completely
> > different meaning that the one possibly intended (with *).
> > So I think that such a conversion falls into the realm of implicit
> > dangerous conversions.
>
> Of which we have several today any way.  Note that boost::shared_ptr<> has
> this exact same potential problem, but the user base has not had issue
> with it.  (Also note that raw pointers would have the same issue.)
>
It not is the same problem at all.
You do not store a bool inside a shared_ptr<>,
but you do store a bool inside an optional<>.

That is:

shared_ptr<X> p ;
bool x = p ;
// there is no doubt *p is not a bool because you wouldn't have
shared_ptr<bool>,
// that is, X wouldn't be bool unless shared_ptr<> is being used very very
oddly.
// If you forgot the *, you also forgot the correct pointee type (X instead
of bool)

optional<bool> p ;
bool x = p ;
// But here the line above could very well supposed to be: bool x = *p;

Fernando Cacciola