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