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