[boost] Re: Formal Review: Typeof

From: Arkadiy Vertleyb (vertleyb_at_[hidden])
Date: 2005-05-20 09:17:58

Hi David,

Thanks for for what I consider overall a positive review.

> > What is your evaluation of the design?
>
> Pretty nice.  Why isn't BOOST_TYPEOF_INCREMENT_REGISTRATION_GROUP
> built into all of the registration macros, simply for ease-of-use?

The problem is that it is actually:

#include BOOST_TYPEOF_INCREMENT_REGISTRATION_GROUP()
^^^^^^

which IIUC makes it impossible to use this inside a macro.  When I started I
used MS-specific __COUNTER__ macro, which allowed it.  However the only
portable alternative seems to be the methods with preprocessor slots,
suggested sometime ago by Paul Mensonides, and this involves #including the
file.  We can special-case it for Microsoft, but I don't know if it worth
it...

> > What is your evaluation of the implementation?
>
> Haven't looked yet.  One thing that seems to be missing is consistency
> between the usage of native and non-native typeof.  On most compilers
> with native support, apparently "typename" is illegal before the
> typeof operator because it's clearly a type name.  However, on other
> compilers BOOST_TYPEOF(x) expands to:
>
>           some_template< something >::type
>
> which of course requires a typename prefix in order to work.

That's what BOOST_TYPEOF_TPL and BOOST_AUTO_TPL are intended for.  They are
supposed to be expanded correctly for dependent contexts, whether emulation
or native typeof is used.  However...

> I suggest that on compilers like GCC, BOOST_TYPEOF(x) be defined
> something like:
>
>         mpl::identity< __typeof__( x ) >::type
>
> so that everything can be uniform.

At the first glance I like it much better than what we have now with
BOOST_TYPEOF/AUTO_TPL.    Maybe these two macros should be eliminated rather
than better documented :-)

> I also know there are some bugs in
> the native GCC typeof that can cause it to ICE.  These can be worked
> around by passing the result of the expression through another layer,
> something like
>
>   template <class T> mpl::identity<T> make_identity(T&);
>   template <class T> mpl::identity<T const> make_identity(T const&);
>   #define TYPEOF(x) __typeof__(make_identity(x))::type
>
> I don't know if the authors are already doing this.  If they _are_
> managing to avoid known bugs in existing native typeof
> implementations, they should note it in the documentation, so people
> know the library is useful.

No, we are currently not doing anything about native typeof bugs.

> > What is your evaluation of the documentation?
> [ A few paragraphs explaining why the documentation is a mess ]

OK, I agree with [almost] everything about the docs.  We'll see what we can
do during the review period.

> > Do you think the library should be accepted as a Boost library?  Be
> > sure to say this explicitly so that your other comments don't
> > obscure your overall opinion.
>
> On the condition that the documentation undergoes major improvement,
> an enthusiastic yes.

Thanks again.

> Ideally I would like to see the authors work on
> improving the documentation immediately and have something to show
> before the end of the review period.

I think it should be possible, of course closer to the end of the review
period.

Regards,
Arkadiy