Re: [boost] Formal review: Optional library

From: William E. Kempf (wekempf_at_[hidden])
Date: 2002-12-10 14:18:53

Fernando Cacciola said:
>> Fernando Cacciola said:
>> > "William E. Kempf" <wekempf_at_[hidden]> wrote in message
>> >> * I believe there should be an "optional& operator=(T const& v)".
>> >>
>> > This would break the pointer-semantics:
>> >
>> > optional<int> opt ;
>> >
>> > opt = 3 ;
>> >
>> > instead of
>> >
>> > *opt = 3 ;
>> >
>> > as it is currently the required syntax.
>>
>> Things get a little confusing here since optional<> is a smart pointer
>> of sorts, but it's assigned values not pointers.
>
> This might lead to confusion.
> optional<> is not really a smart-pointer sort of thing.
> It has similarities, but *significant* differences as well.
> I think than shaping optional<> as a smart pointer blurs
> those important differences.
> The most important difference, as I see it, is that a smart pointer
> wraps 'a pointer' which 'points to an object'.

I'm not sure I agree with this.  A smart pointer to me is simply an object
whose interface mimics a pointer.  Granted, this is the first case I've
ever seen where the implementation doesn't wrap a real pointer, but I'm
not at all sure this makes any difference here.  If it does, then I don't
think the pointer interface should be used at all and you should consider
value semantics instead.

> optional<> is trying to model using C++ a concept that
> it is not really covered by the language, that of uninitialzed values.
> It uses pointer semantics *just* because pointers are the only sort of
> C++ objects which has a clear uninitialized state.

I think that's flawed logic.  There are a lot of C++ objects I've seen
(with out the generic reusable design we get with optional) that deal with
an "uninitialized state".  Granted, they fail to truly not initialize the
data like optional does, instead default initializing the data, but the
idea is still the same.  The value of the data is not gauranteed to
fullfill any invariants when the "uninitialized" flag is true.  So,
pointers aren't the only examples available.  And even if they were, that
still wouldn't necessarily mean they'd be the correct interface in this
case.

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. 
All I'm saying is that if we retain a smart pointer design, we should be
faithful to that decision.

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

>> 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 I won't argue this one too much.  But with
>> out the assignment I think it becomes more imperative to add a reset()
>> that allows you to modify the value with out the need for a temporary
>> optional<>.
>>
> I don't know.
> I'm generally not too fond of small-convenient-member-functions
> which offer a shortcut-functionality which blurs the conceptual
> uniformity of the design.

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

> Currently, optional<> is not a designed as a container,
> it wraps a value, true, but it does not 'contain it'
> in the sense that you cannot put it in and take it out.
> reset() would actually shape optional<> more like a container
> because it would amount to 'replace the value'.

Change the wording to "change the value" and I think you see the need, and
I think reset() conveys this.

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

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

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

> (2) There is no difference between both forms, but the 'value' form is
> deprecated
> (is funny to have a deprecated feature in a library just released, isn't
> it :-)
> Let me explain:
> If only operator*() were there, the following would work
> (or at least it wouldn't with some popular compilers):
>
> template<class T> foo ( T ) ;
> void bar()
> {
>   optional<int> opt (2);
>   foo(*opt);
>   // ERROR here: the compiler doesn't call "operator T&" from the proxy.
> }

But what's the need for the proxy?

> So, I needed to add a method to allow you to workaround the case above:
>
> foo(bar.value());
>
> Anyway, there is another way:
>
> foo( *bar.peek() );
>
> but this has problem: peek() is intended to return NULL in case the
> optional is uninitialized(), so you loose the possibility of having an
> assert in debug mode as you have with 'value()'.

But shoudln't operator*() be able to fullfill all needs here?  Like I
said, I seem to be missing something.

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

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

> 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.  It eludes me how it wouldn't work, since this
is truly precisely how pointers (which you're emulating) work.

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

> I'm convinced now that comparing against an uninitialized optional isn't
> either true nor false, is undefined.

I'm not.  Convince me.

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

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

>> >> * The fact that optional<T> does not require T to have a default
>> constructor is significant, and should have some more emphasis in
>> the documentation.  This is what opens up use cases other than the
>> "return a value or uninitialized" case discussed ad nauseum in the
>> document. For instance, I expect to be able to put this class to
>> use in an implementation of an "async_result" type for
>> Boost.Threads.
>> > I see.
>> > Could you give me a summary of the usage in async_result?
>> > I can add it as the list of usages.
>> > I know we have talked about this but I don't remember the details.
>>
>> Well, what I'm envisioning for async_result is different from anything
>> discussed thus far, but a simple usage example would be:
>>
>> int foo()
>> {
>>    // return some complex and lengthy (execution-time-wise)
>> calculation
>> }
>>
>> async_result<int> res;
>> boost::thread thrd(res.call(&foo));
>> thrd.join();
>> cout << *res << endl;
>>
>> (Note that this is a design in progress and I'd expect several things
>> in the interface to change slightly, but hopefully this gives you the
>> idea.)
>>
>> Here, the value "holder" has to be created outside of the call, and
>> passed in to the function object (though this part is handled
>> transparently), rather than being created in the function and
>> returned.  But it's still important that we not have to default
>> construct this object, lest you needlessly restrict the types in which
>> this mechanism can work.
>>
> OK, I see.
> Here it is just the fact that optional<> has a mechanism to
> bypass default contruction which is exploited; not the fact that
> it can be uninitialized per see.

Exactly!

William E. Kempf