Re: [boost] Formal review: Optional library

From: Fernando Cacciola (fernando_cacciola_at_[hidden])
Date: 2002-12-10 12:12:05

----- Original Message -----
From: "William E. Kempf" <wekempf_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Monday, December 09, 2002 7:48 PM
Subject: Re: [boost] Formal review: Optional library

>
> 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'.
That is, a smart pointer carries two things: a pointer value + an object
value.
You handle the pointer value via get(),reset(), etc...
You handle the object value via operators *() and ->().
But optional<>, OTOH, wraps *only* an object value.
It does not 'contain a pointer' as a smart pointer does.

Now, optional<> do has a member function that returns a pointer
to its wrapped object, but this is a pointer which is actually
intended to allow you to access the value from within optional<>,
while at the same time let you handle the uninitialized state
conviently.

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

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

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

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 "peek()" should be named "get()".  Despite it's unusual (and
> >> needed) use, this is a form of a smart pointer, and should follow the
> >> same naming conventions that people have grown accustomed to through
> >> std::auto_ptr and the Boost smart pointers.
> >>
> > Fair enough.
> > I'm actually trying to vouch for my peek/acquire idiom here.
> > Essentially, the name 'peek()' is intended to convey the fact that the
> > pointer
> > points to a managed object and that the ownership is not being
> > transfered along with the pointer (as oposed to acquire()).
>
> Where did acquire() come from?

See my response to Dirk Gerrits.

>
> > 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.
Now I want peek() even more than before, precisely to stress out that is not
containing
a poiner.

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

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

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

>
> >> * I think "uninitalize()" should be named "reset()".  Not only for
> >> consistency with other smart pointers, but also because the term isn't
> >> really even meaningful english.
> > Hmmm.
> > Does 'reset' conveys what it is doing?
> > Is it clear that 'reseting' an optional<> means to leave it
> > uninitialized? I'm not sure; but if others see it like you do, I'll
> > change it.
>
> It makes sense to me.  But if reset() isn't understandable, then clear()
> or destroy() would be better names.
>
I don't know.
clear() would be appropriate if optional<> would be like a container, but it
isn't.
destroy() sounds as if the 'optional<>' object itself is being destroyed.
I would expect that an object upon which you have called destroy()
is no longer usable, but this is not the case case with optional<>:
uninitialized is a 'valid' state; the optional<> object is not destroyed.

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

> >> * 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.
But given that I actually agree with you in principle, I will see if there
are
member functions which can be removed.
Perhaps, replacing value() by a 'safe_peek()' (which is just like peek() but
which
asserts instead of returning NULL). And removing 'initialized' entirely.

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

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

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

>
> > optional<> follows the same idiom.
> > You need to use the operator*() to perform the comparisons because you
> > need to dereference an optional<> in order to access its value. For this
> > reason, the proxy, returned by operator*(),
> > does have some conditional operators (== and !=) defined,
> > so you can compare optional "values".
> >
> >
> >> * 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 ?
}

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.

> >> * 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.
Nice example. I'll see to add this to the docs. Thanks.

Fernando Cacciola