[boost] Re: [Typeof] Formal Review in progress

From: Arkadiy Vertleyb (vertleyb_at_[hidden])
Date: 2005-05-26 13:23:42

Hi Sasha :)

Thanks for this review (and also for pointing me to RSDN, and helping with
Intel).

> - Why decode_type_impl and encode_type_impl primary templates are declared
> in unnamed namespace? All specializations are also in (other) unnamed

Why "(other)"?  I believe they are the same in the same TU...

> namespaces. Is it intentional?

Yes.

Automatic ID generation leads to potential ODR violation, since, depending
on the order of inclusion, the same template specializations can be
generated with different IDs, and therefore with different bodies.  This was
workarounded by placing them into anonimous namespaces, so that thay are
different in different TU, and so not the subject to ODR.

Unfortunately, this did not completely solve the problem -- it just moved it
to a different place.  Since BOOST_TYPEOF is now in anonimous namespace, it
is different in different TU, and so itself technically causes ODR if used
in the header files, pseudocode:

TU1:

struct A
{
    type_of::anonimous-TU1::decode<>::type;
};

TU2:

struct A
{
    type_of::anonimous-TU2::decode<>::type; // the same type, really, but
computed differently
};

This question was discussed in the past, and it was decided that we can live
with this as long as we have a specific test that verifies that compilers
don't really care.  We have such test, and at some point Martin Wille helped
me by running it on como, which is considered to be the most strict
compiler.  Of course it's also a part of our regular test.

> - It would be nice to put some functionality like get_unsigned<T> in
>   correspondent libraries. This way, we could have better support for
>   them. In this particular case, non-standard integral types could be
>  supported as well. See also my note about documentation.
>
> - encode_integral casts integral type to size_t. Is there any guarantee
>   that size_t is always big enough?
>
> - I don't understand why some big contants are hardcoded? For example,
>   encode_size_t template checks if n >= 0x3fffffff. If it's magic,
>   can you please comment it explicitly?

Actually a stronger limitation is the size of the array that I use to create
appropriate sizeof().  The magic constant above has to do with the fact that
I use a couple of bits as flags, such as if there is an overflow, and decide
whether to use the second integer for encoding.  I agree that this is not
portable as is to the 64 bit platforms, for example.

> > What is your evaluation of the documentation?
>
> Not yet complete.
>
> - There's no references to online versions of Steve Dewhurst's articles.
>   They can be found at http://www.semantics.org/localarchive.html
> - No information at all about how to compile in compliant mode.
> - Why only "well-known integral type" are registered implicitly?
>   I think that all integral types supported by boost::is_integral
>   should be supported by the library as well.

OK.

BTW, the library also has most Standard C++ Library types/templates
registered, but for this some additional headers need to be included, such
as:

#include <boost/typeof/std/string.hpp>
#include <boost/typeof/std/memory.hpp>
etc.

I don't know how noticable this is in the current doc, though.

> > Do you think the library should be accepted as a Boost library?
>
> It should be accepted.

Thanks again.

Regards,
Arkadiy