Boost logo

Boost :

From: Gennadiy E. Rozental (rogeeff_at_[hidden])
Date: 2001-09-16 03:26:18

--- In boost_at_y..., "Fernando Cacciola" <fcacciola_at_g...> wrote:
> The following is derived from conversations with Gennadiy Rozental
about the
> optional<> class.
> My initial design used a mechanism that allows optional<T> to
bypass T::T().
> This capability is required in order to allow a type without a
> constructor to be used with optional<T> ; and it also offers room
> optimization since the default constructor may not be trivial.
> Gennadiy was concerned about the unnecessary overhead introduced by
> bypass mechanism in cases where T is a POD; and he made a good job
> drawing my attention to this issue.
> There are two possible implementations:
> a value-based implementation, where optional<T> contains:
> T v ; bool initialized ;
> and a pointer-based implementation, where optional<T> contains:
> T* v ; aligned_storage<T> buffer;
> I setup a test bed and verified that for POD types, the overhead is
about as
> twice as much executed code for the typical operations.
> Therefore, I decided to do the following:
> I believe that the implementation choice for optional<T> depends
*only* on
> T -not in the context in which optional<T> is used-; therefore, the
> class can automatically select the appropriate implementation based
only on
> the type T. No policy class is required, or at least, not as a
> template argument.

I agree with premise, but I do not completely agree with conclusion,
specifically with the provided mechanism for optional internal
behaviour customization. Though I agree that Polycy template argument
seems to be unnecessary overhead, specializing optional_behaviour to
enforce value based implementation also also seems to be too
complex, especially if I would need it to do for every type that I am
using with optional. The simple solution could be just to provide
boolean template argument as selector. More complex solution is to
provide type argument that could be true_type, false_type or default -
where third case would mean current selection logic.
  Lyrical digression:
  I still think that usage of pointer-based optional seems to be a
little bit odd. Given bulky or expensive to construct type T I would
rather use plain pointers or to simplify memory management -
shared_ptr. In your example:

optional<T> array[N]; vs shared_ptr<T> array[N];

1. Does not call constructor 1. Does not call constructor
 at initialization stage at initialization stage
2. Does not call destructor for 2. Does not call destructor for
 elements that did not constructed elements that did not constructed
3. Value acess overhead the same 3. Value acess overhead the same
 as with plain pointer as with plain pointer
4. "Allocate" sizeof(T)*N memory 4. Memory allocation strategy is
 on stack. May not use all of it. configurable by allocator
 In case of bulky type T it parameter. May behave the same
 could be a big memory overhead. way as optional<T> with 1 time
 This could be especially memory allocation in a heap which
 important in system with small add neglectable overhead.
 memory Alternatively it could use some
                                     kind of smart strategy to manage
                                     memory usage.

That is why I think that in most cases value based bahaviour would be
used and propose it as default case for boolean argument. In those
rare cases when you whould need pointer based behavior you can
specifically specify that by second template argument. But if
everybody are disagree with my points, I admit that current way to
customize the internal behaviour is acceptable

> I change the code to reflect this.
> The new optional<T> uses ::boost::is_POD<T> to select a value or
> pointer-based implementation.
> The interface allows the user to override the default impl
selection for any
> type T to accommodate his/her needs.
> I also made up my mind about how to let the user test if a given
optional is
> initialized or not.
> Previously, optional<> used operator void*() to allow expressions
of the
> form:
> if ( opt ) or if ( opt != NULL )
> After *a lot* of consideration, I decided to support only
the 'bang-bang'
> idiom:
> if ( !!opt ) means initialized.
> if ( !opt) means uninitialized.

I admit that this is good solution if you want to eliminate any kind
of implicit conversion from optional to T

> I just hope enough people is aware and fond of this very useful
> because it removes all the problems with the other approaches.
> The new optional.hpp header contains a small rational for this
> decision.
> I also refactored things and updated the documentation and the test
> The new stuff has been updated in the FilesSection under optional
class (as
> before).
> I also included a zip file for easier downloading.

Now about implementation (I am not familiar with aligned storage side
so you won't see any comment on that):


line 17:
#include "boost\type_traits.hpp"

I would recomend to include specific traits header. No need to make
compiler life even more complex ( be pitifull to them. They are
trying so hard to be up to standart and work properly with all this
template madness ;-)))

line 155-160;
It just my general preferance to see first public interface then
followed by private implementation. It seems to be more reasonable,
though it is up to you (does boost have an appropriat guidance?)

line 166:
    optional () // no throw

Some comments ( including big description in the beggining) reflect
pointer-based behaviour, which is not the case any more.

lne 190-191:
    T const* get() const { return imp.get() ; }
    T* get() { return imp.get() ; }

Frankly speaking, I don't get it. After all those discusstion about
preventing user from making occasional error at least by means of
exception, you now allow to write:

optional<T> t = foo();

T t1 = *t.get();

and wouldn't hit him on hands? While for pointer-based behaviour it
will just cause memory access violation, for value-based behavoiur it
will silently gives an access to uninitialized value. Why not put
verify_is_initialized() check in get functions either?

line 222: also see the implementation in the optional_detail.hpp
    proxy operator *() { return proxy(this) ; }

I think it would be better to use reference in implementing of proxy
class. The idea id to make assignent as transparent as possible. So
statememt like:

optional<T> t;

*t = some_value;

would be interpreted by the compiler the same way as ( in value=based
t.imp.v = some_value;

I think with reference we have more chances.

  Lyrical digression:
 I think the rational should be to use use C++ pointer when you
need "Pointer" that can be uninitialized (point nowhere) or should be
able to change object it is pointed to. In other cases better to use
C++ reference instead. What do you think?

line 226:
    operator !() const { return !initialized() ; }

Wouldn't it be better to put bool ahead?

line 228:
static std::string implementation_choice()

I am not sure why do you need that function (other than for internal
testing). This couse you to include <string> header(and <typeinfo>
which is not present now but will be required by other compilers),
which is not used anywhere else.

line 241:
      #ifndef _NDEBUG

I am not sure about this name. Should we use some specific name
instead (like DEBUG_OPTIONAL)?

line 243: also see the implementation in the optional_detail.hpp
     optional_detail::accessing_uninitialized_optional_error() ;

Well, it is another general concern about design. I think that in
most cases you just need assert based implementation for this
function. The idea was to provide developer an information: Hey you
forgot "is initialized" check. Developer type it in, compile it and
move on. This error is not supposed to be cought somewhere, reported
and than - keep working. If you so desperately need exception based
error check provide a way to cusomize it. But default still sould be
assert as more straight way.


line 19:
#include "boost\type_traits.hpp"

see above

line 17,20:
#include "boost\aligned_storage.hpp"

It would be great to separate to behavoiurs so that value-based one
won't include those both header. Again - no need to enlage work for
compiler. But I still do not now easy way to do this.

line 36:
         OPT* x ;

see above

line 48-76
Comparison operators.

I still does not convinced should we or not provide templated
versions to allow implicit conversion so that

*t = u will compile without explicit cast (*t = (T)u;). I would like
to hear some other opinions on this metter. You can find our
discussion in and

line 80
     void __export accessing_uninitialized_optional_error() ;

Does __export is standart keyword?

line 106, 112:
             v = av ;

This statement could cause en exception. WE should not leave the
object in undefined state The flag should be reset to false. It
could be done be means of try-catch block where in catch we set init
to false and then rethrow or ,simplier, just adding additional
statement ahead of this init = false. So the assign will look like
init = false;
v = av;
init = true;

line 124-125:
         T v ;
         bool init ;

Why do you have them in public section of class?


line 32:
    template<int N> struct typed_int_value
      { BOOST_STATIC_CONSTANT(int, value = N ) ; } ;

  Lyrical digression:
  This is an idea I was thinking to implement for some time now.
Solaris SunPro does not support any expression with non-type template
arguments and this is one of my target compilers. But you missing
very important thing: prev and next (you will need preprocessor to
define enough specializations ):
    struct begin{};
    struct end{};
    template<int N> struct typed_int_value
      { BOOST_STATIC_CONSTANT(int, value = N ) ; } ;

    template<> struct typed_int_value<0>
         BOOST_STATIC_CONSTANT(int, value = 0 );
         typedef begin prev;
         typedef typed_int_value<1> next;

      } ;


    template<> struct typed_int_value<MAX_NUM_TYPE>
         BOOST_STATIC_CONSTANT(int, value = MAX_NUM_TYPE );
         typedef typed_int_value<MAX_NUM_TYPE-1> prev;
         typedef end next


Now you will be able to use this facility to implement some of the
compile-time tricks you would like to use (like GCD library currently
under review with some additional efforts).

line 50-52:
template<class Cond,class T0,class T1> struct select ;
template<class T0,class T1> struct select<type_true,T0,T1>
{ typedef T0 type ; } ;
template<class T0,class T1> struct select<type_false,T0,T1>
{ typedef T1 type ; } ;

There are couple versions of Meta IF that does not use partial
specialization. Look for example on changes proposed by me for select
type header file in

line 56 -63:
    struct end {};

    template<typename Head, typename Tail = end>
    struct list
      typedef Head head_type;
      typedef Tail tail_type;
    } ;

This is basically loki's TypeList. So the name should be typelist and
implementation could be much more powerfull (plus we could use
typed_int_value to make it more portable).

> Opinions are welcome.
> Thanks,
> Fernando Cacciola
> Sierra s.r.l.
> fcacciola_at_g...

This is my opinions.



Boost list run by bdawes at, gregod at, cpdaniel at, john at