Boost logo

Boost Users :

Subject: Re: [Boost-users] [typeindex v3.0] Peer review begins Mon 21st ends Wed 30th
From: Mathieu Champlon (m.champlon_at_[hidden])
Date: 2014-04-26 05:07:02

On 20/04/2014 20:14, Niall Douglas wrote:
> 1. What is your evaluation of the design?

Is there any reason why bti::type_id_runtime(variable) could not be
bti::type_id(variable) ? Is it to make it stand out or is there a
technical limitation ?

> 2. What is your evaluation of the implementation?

Here is what I noticed:

I find the name type_index_facade a bit misleading as a facade to me
refers to the design pattern which obviously isn't what it is about.

(in type_index_facade.hpp)

     inline bool equal(const Derived& rhs) const BOOST_NOEXCEPT {
         return raw_name() == rhs.raw_name() || !std::strcmp(raw_name(),
(and similarly in before(...))

Why first test with == ? Won't strcmp take care of it?
Or is it to filter out the invalid cases where raw_name() returns a null
pointer? Are we sure that either both or none of them can be null?
Just below, hash_code doesn't seem to check when calling std::strlen.
Maybe raw_name() is supposed to always return a non-null pointer? In
that case it should be added to the function documentation.

Doesn't type_index_facade::type_info(), type_index_facade::raw_name()
etc.. shadow the same functions in the derived classes?
I don't know if that could be a problem like some compilers issuing a

> 3. What is your evaluation of the documentation?

It's quite nice overall, I noted a few glitches:

. I find the phrasing of "And that is the point, where problems start"
in the motivation section a bit awkward. To be honest I had to re-read
it a few times before I could understand what it meant.
I think something much simpler would be easier to read like ", which is
where problems start"

. with a quick glance at the motivation section it wasn't obvious to
know if Boost.TypeIndex fixes the const/ref/volatile stripping issue
with the compilers which don't implement it correctly or if it levels
the differences by always stripping them
Again I think a simpler phrasing would help, maybe "some implementations
of `typeid(T)` erroneously strip const, volatile and references from
type" which would imply that Boost.TypeIndex fixes this

. the section "Making own type_index" could probably be renamed
"Customizing type_index" or "Making a custom type_index"

Well, English not being my first language I may just be delirious here. :)

I found the "How to use" section very useful as this allowed me to
quickly replace hand-written code with Boost.TypeIndex
However it took me a couple of minutes to understand what
please_save_modifiers and with_crv meant, and I'm not sure all C++
developers out there now that crv stands for const reference volatile...
Maybe adding a C++ style comment explaining each line would help. Also
links to each function documentation would be nice.

The doxygen comment for type_id_runtime has a typo in "/// Retunrs
runtime information about specified type." (should be Returns)
A little further the documentation for runtime_val has a similar one
(Varaible instead of Variable)
Isn't there some way to run the doc through a spell checker to catch these?

The documentation of type_id_runtime doesn't explain how it behaves in
respect to cvr qualifiers?

     // noexcept comparison operators for type_index_facade classes.
     bool operator?(const type_index_facade & lhs,
                    const type_index_facade & rhs);

Is this a doxygen glitch? Should there be a note stating '?' stands for
'==', '!=', etc..?

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

I believe the motivation section speaks for itself: this library is
indeed useful.
My personal interest lies only in retrieving debugging/diagnosis
information which oddly seems to also be the case for most of the other
reviewers so far (I didn't expect that!).

> 5. Did you try to use the library? With what compiler? Did you have
> any problems?

I replaced my hand-crafted type pretty name extraction [1] with
boost::typeindex in about 10 mn [2].
All my tests [3] compile and run without a glitch on MSVC2010 and gcc
4.6.3. I just removed one of them which was testing the pretty name of a
type local to a function (obviously this doesn't work with pre-C++11
The code was based on BOOST_SP_TYPEID and an ad hoc formatting filter.
As others have stated I believe this latter feature could be a
worthwhile addition to Boost.TypeIndex.

> 6. How much effort did you put into your evaluation? A glance? A
> quick reading? In-depth study?

A bit of time to integrate it into my library and to write a few tests
in order to better understand how the library behaves, about 2h in total.
I somewhat carefully read the documentation and browsed the code a bit.

> 7. Are you knowledgeable about the problem domain?

Not much past extracting pretty names for logging, and only for MSVC and

> And finally, every review should answer this question:
> 8. 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.


Thanks for writing the library Antony.



Boost-users list run by williamkempf at, kalb at, bjorn.karlsson at, gregod at, wekempf at