Re: [boost] Formal review: Optional library

From: William E. Kempf (wekempf_at_[hidden])
Date: 2002-12-11 10:37:02

Fernando Cacciola said:
>> 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...

Then your documentation is wrong!  To quote:

"proxy optional<T>::operator* () ;

   If the optional object is initialized, returns a proxy object which
allows mutable access to its value (of type T).

   If it is uninitialized, the result is undefined (but BOOST_ASSERT is
used so the user can define this behavior in a debug build)."

I can not assign to an uninitialized optional through operator*() because
that results in undefined behavior.  Now I assume that this was the reason
for the proxy... so you could assign to an uninitialized optional.  But I
think that's a bad idea, because the assignment would be the only valid
thing you could do here, which is just plain confusing.  But even if I
thought we should keep the concept of the proxy, your documentation is
simply incorrect.

> 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.

Yes!

> 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.

I didn't expect the behavior mostly because the documentation clearly says
this isn't legal.  However, it would be surprising to include the single
use case of assignment to an uninitialized optional, yes.

> 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?

My vote is for reset(), as it provides the cleanest interface.

>> > 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.

So what?  Containers store values.  So do smart pointers, even though the
value stored is a pointer.

> 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.

Which bothers me less than using an interface that's mostly likely a smart
pointer except in these unecessary details (the detail that it doesn't
take a pointer is necessary, but the others are clearly not).

> 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.

I think it leads to a more understandable interface... but we'll have to
let others voice their opinions as well.

>> 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...

It provides operator*(), and operator->(), so it walks like a duck and
talks like a duck.

>> > 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 believe so, and I believe that the name optional conveys it already.

> 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).

But get() doesn't transfer ownership.  release() does.  And the name
optional conveys to me that the ownership can't be transfered.

>> 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.

What rest?

> 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.

Smart pointers need not take a pointer for the constructor, etc.  Many
valid smart pointers allocate their own memory internally, for instance. 
This _one_ quality you see in smart pointers is something I don't think is
at all part of what makes a smart pointer.

>> > 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,

That's what I was referring to.

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

It's not?  'b' is null, 'a' isn't, and 'a == b' yields false.

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

I think the analogy is accurate.

>> > 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.

Why not?

> The comparison just cannot be made, so the result is neither true nor
> false.

I don't agree.

> Think about the interval library for instance.

That's a totally different domain, with different rules and requirements.

>> > 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.
> }

That works when "*a OR *b" is uninitialized.  It may or may not work
depending on what you expect when "*a AND *b" are uninitialized... but I
think you can make that decision, document it, and have a useful
interface.  Again, pointers work this way.

a = b = NULL;
if (a == b)
{
}
else
{
}

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

That would help.  It seems like the only cases I can think of would be
where you were abusing optional<bool> as a tribool.

>> > 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<>.

You don't?

boost::shared_ptr<bool> pb(new bool(true));

if (pb)  // user meant if (*pb)

Granted, this is less likely, but it's still very valid.

And if we get a tristate bool in Boost, it's just as unlikely that anyone
would use optional<bool>, since the tristate bool would be more
appropriate for most use cases.

William E. Kempf