[boost] Formal Review: Typeof

From: David Abrahams (dave_at_[hidden])
Date: 2005-05-20 08:04:31

Here are some preliminary remarks:

> 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?
For experts, you could include 

  BOOST_TYPEOF_REGISTER_ANOTHER_TEMPLATE   // ;-)

which doesn't increment the registration group.

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

> What is your evaluation of the documentation?

Still sorely lacking.  For example, there are no examples that show
how BOOST_TYPEOF_TPL, BOOST_AUTO_TPL, and BOOST_LVALUE_TYPEOF should
be used, nor even any documentation of how many parameters they take.

When trying to use the library I discovered the hard way that I had to
close all namespaces before using any of the registration macros --
and, "obviously," fully qualify the names being registered.  This
should be noted in the documentation.

The documentation should make it clear how BOOST_TYPEOF et. al are
meant to be used in dependent contexts.

There is absolutely no formal reference documentation.

The documentation was generated by saving a MS-Word file to HTML.  In
general this produces bloated HTML files that often don't work as
expected in browsers.

Also, I found the documentation hard to grasp quickly.  It's a little
too informal and unstructured.  These docs should be re-organized and
divided into sections and subsections.  Preferably there should be a
rewrite in QuickBook.

> What is your evaluation of the potential usefulness of the library?

Mega-useful.

> Did you try to use the library?  

Yes.

> With what compiler?  

vc7.1 vc8.0 gcc-3.3.3

> Did you have any problems?

Noted above.

> How much effort did you put into your evaluation? A glance? A quick
> reading?

Only a quick skimming of the documentation.

> In-depth study?
> Are you knowledgeable about the problem domain?

Fairly.

> And finally, every review should answer this question:
>
> 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.  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.
Yes, that would be raising the bar a bit over what we've expected of
people in the past, but we _need_ to do that, and this particular
library is too important to allow it to slide in with substandard
docs.

-- 
Dave Abrahams
Boost Consulting
www.boost-consulting.com