On 12 Nov 2013 at 12:54, Steven Watanabe wrote:

I'm aware of this bug. link-fail doesn't work with the msvc toolset, because there are two separate updating actions (msvc.link and msvc.manifest). FAIL_EXPECTED currently requires that /every/ updating action fails. No one was using it before, so put it at low priority.

That is very useful to know indeed, and explains the problem. Could a workaround for TypeIndex be to disable the generation of the manifest file? For unit test executables - especially ones expected to fail to link - we don't care, so is disabling that second step possible? Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

2013/11/13 Vicente J. Botet Escriba <...>

* In http://apolukhin.github.io/type_index/boost_typeindex/getting_started.html boost::type_info| is a drop-in replacement for |std::type_index

Fixed

* In http://apolukhin.github.io/type_index/boost_typeindex_header_reference.html#...

Shouldn't |||std::hash needs to be specialized for boost:index? |template <> struct std::hashboost::type_index;

The same applies for boost::type_info.

That's a good question for many of the Boost libraries. One of the policies in Boost - is not to mess with std:: namespace. On the other hand specializing std::hash is a common solution in C++11. Many Boost libraries just do not specialize std::hash, so I followed that design.

* In http://apolukhin.github.io/type_index/boost/type_info.html These functions |||

//public static functions <http://apolukhin.github.io/ type_index/boost/type_info.html#idp5825712-bb> template<typename T> static const boost::type_info < http://apolukhin.github.io/type_index/boost/type_info.html> & construct http://apolukhin.github.io/type_index/boost/type_info.html#idp5826208-bb() noexcept; template<typename T> static const boost::type_info < http://apolukhin.github.io/type_index/boost/type_info.html> & construct_with_cvr <http://apolukhin.github.io/ type_index/boost/type_info.html#idp5829664-bb>() noexcept; template<typename T> static const type_info < http://apolukhin.github.io/type_index/boost/type_info.html> & construct_rtti_only <http://apolukhin.github.io/ type_index/boost/type_info.html#idp5833264-bb>(T &) noexcept; template<typename T> static const type_info < http://apolukhin.github.io/type_index/boost/type_info.html> & construct_rtti_only <http://apolukhin.github.io/ type_index/boost/type_info.html#idp5837360-bb>(T *);

| ||are in some way duplicates of | ||

template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id <http://apolukhin.github.io/ type_index/boost/type_id.html>(); template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id_with_cvr < http://apolukhin.github.io/type_index/boost/type_id_with_cvr.html>(); template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id_rtti_only < http://apolukhin.github.io/type_index/boost/type_id_rtti_ on_idp5863488.html>(T &); template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id_rtti_only < http://apolukhin.github.io/type_index/boost/type_id_rtti_ on_idp5867296.html>(T *);

| Defining the preceding functions factories friend of boost::type_info, could be enough.

Added a note to member functions that they work exactly like functions in boost:: namespace. Did not removed text (having it in-place looks more user friendly, than forcing user to go and search the description oin some other place/method) * Why don't replace

|

template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id <http://apolukhin.github.io/ type_index/boost/type_id.html>();

by

template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id <http://apolukhin.github.io/ type_index/boost/type_id.html>(T & v);

template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id <http://apolukhin.github.io/ type_index/boost/type_id.html>(T * v=0);

so that the user can use

class D { /* ... */ }; D d1; const D d2; boost::type_id(d1) == boost::type_id(d2); // yields true boost::type_id<D>() == boost::type_id<const D>(); // yields true boost::type_id<D>() == boost::type_id(d2); // yields true boost::type_id<D>() == boost::type_id(); // yields true

User can do exactly the same using current interface: class D { /* ... */ }; D d1; const D d2; boost::type_id_rtti_only(d1) == boost::type_id_rtti_only(d2); // yields true boost::type_id<D>() == boost::type_id<const D>(); // yields true boost::type_id<D>() == boost::type_id_rtti_only(d2); // yields true boost::type_id<D>() == boost::type_id(); // yields true Names have "_rtti_only" to warn user that this call requires RTTI. This must ring a bell and user may switch to type_id<D> version, which is more portable. type_id_rtti_only is more explicit, so I like it more. But if there'll be many recomendations during review period to switch names to boost::type_id, I'll do that.

|||* Is there any reason to name |type_id_rtti_only|| instead of type_id to emulate the std typeid behavior. I would reverse the names

template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id <http://apolukhin.github.io/ type_index/boost/type_id.html>(T & v); // behaves like typeid(v)

template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id <http://apolukhin.github.io/ type_index/boost/type_id.html>(T * v); // behaves like typeid(v)

template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id_no_rtti < http://apolukhin.github.io/type_index/boost/type_id_rtti_ on_idp5863488.html>(T &); //heavier but doesn't need rtti

template<typename T> const type_info <http://apolukhin.github.io/ type_index/boost/type_info.html> & type_id_no_rtti < http://apolukhin.github.io/type_index/boost/type_id_rtti_ on_idp5863488.html>(T *);//heavier but doesn't need rtti

Names were given according to the following logic: * TypeIndex library promises that it works without RTTI * calls that do not work without RTTI must be explicitly marked * calls that do work without RTTI - is just what was promised by the library, no need to explicitly mark that the call does not requires RTTI In both situation we can not 100% emulate the syntax typeid: typeid(Type) // this can be done by macro, but macros are not good typeid(variable) * why do you need template_index it it is the same as template_info?

This is done for uniformity: if there is a type_info and type_index, than it must be template_info and template_index. I've tried to implement all the functionality of type_index in type_info, but that attempt failed (so we have two classes). Either type_info was not able to copy, or following code failed to compile: // Note the std:: namespace! std::type_index t = boost::type_info_with_cvr<const int>();

* What about separating NORTTI inBOOST_TYPE_INDEX_FORCE_ NORTTI_COMPATIBILITY?

Fixed

* what is behind the name template in template_info?what about renaming it to type_info_ptr?

`template` is an attempt to reflect in name its behavior. `template_info` == information about template parameter type

* stating that something works as the standard C++, would either need a reference to a version, or describe explicitly the bahavior on the library documentation.

I've missed the point, where in docs must be the note about version added?

Best, Vicente

Great thanks for all the comments! I've updated the docs and sources, -- Best regards, Antony Polukhin

2013/11/13 Gavin Lambert

On 13/11/2013 20:22, Quoth Antony Polukhin:

Names have "_rtti_only" to warn user that this call requires RTTI. This

...

must ring a bell and user may switch to type_id<D> version, which is more portable. type_id_rtti_only is more explicit, so I like it more. But if there'll be many recomendations during review period to switch names to boost::type_id, I'll do that.

[...]

Names were given according to the following logic:

...

* TypeIndex library promises that it works without RTTI * calls that do not work without RTTI must be explicitly marked * calls that do work without RTTI - is just what was promised by the library, no need to explicitly mark that the call does not requires RTTI

Couldn't this be resolved by changing the first to "TypeIndex library promises to be an enhanced implementation of standard typeid".

Then you could just use: * boost::type_id<Type>() // typeid(Type) * boost::type_id(variable) // typeid(variable)

Both would work with or without RTTI but the second would only report the declared type of the variable in the absence of RTTI (which is what typeid does for sliced value types anyway).

This looks very error prone. In current implementation disabling RTTI leads to compile errors in case of an attempt to determinate type at runtime. Using your approach code will compile, behavior of functions will silently change and user will be debugging the application. Or even won't find the issue and release a bogus program.

Granted that you're no longer being explicit about RTTI safety or not in the actual method name, but I would hope that library authors who wish their libraries to work either way would test accordingly, so it should not matter.

Why force them to test code for errors, that can be detected and avoided at compile-time?

It seems to me that you're already aiming "higher" than bare-bones no-RTTI support with this library, with the name demangling and cv-preservation features for example. (Though please see my other reply regarding demangling.)

Well may be a little bit :) But those features are widely used and looks like type_info is the correct place for them. -- Best regards, Antony Polukhin

2013/11/13 Gavin Lambert

On 13/11/2013 08:34, Quoth Niall Douglas:

Boost community feedback is requested for the formal peer review of

...

the TypeIndex library by Antony Polukhin.

[...]

NOTE: Please read the TypeIndex documentation (linked to below)

...

before asking any questions e.g. about potential code bloat etc. You'll probably find the answer already there.

The docs for .name_demangled() indicate "Readable names may also differ between compilers: struct user_defined_type, user_defined_type."

Is it feasible to enforce some sort of compiler-independent uniformity on this (eg. guaranteeing that even if the native typeid output would insert "struct" or "class", name_demangled() would strip it off before returning it)? It would be good for code portability if users didn't have to worry about possibly externally stripping such things to produce "nice" output, or consistent output between compilers.

This is almost impossible: different compilers decorate names differently. This means that for each compiler we need to write parser/lexer that: * puts const, volatile, rvalue, reference at correct position (always after the type or always before) * strips away class, struct, __cdecl* * unifies template parameters representation * makes other unifications (array representations, wchar_t and __wchar_t unifications and so on...) This may be done some day and `name_portable()` method can be added. But I'm afraid this won't happen soon. I'm also not sure "name_demangled" is a good method name, as "name" being

mangled is a GCC implementation detail that is not true in MSVC, for example. I would prefer something more generic, such as "long_name" or "friendly_name", or like MSVC using "raw_name" in place of the current "name", and "name" in place of the current "name_demangled".

According to C++03 and C++11 Standard type_info::name() must return const char*. Getting readable name requires some work and an internal buffer inside boost::type_info. This looks like a bad solution (so "raw_name()" and "name()" won't fit). MSVC also mangles names, but hides that from user. So as for me, name_demangled() looks not bad. -- Best regards, Antony Polukhin

2013/11/13 Gavin Lambert

On 13/11/2013 20:46, Quoth Antony Polukhin:

This is almost impossible: different compilers decorate names differently.

...

This means that for each compiler we need to write parser/lexer that: * puts const, volatile, rvalue, reference at correct position (always after the type or always before) * strips away class, struct, __cdecl* * unifies template parameters representation * makes other unifications (array representations, wchar_t and __wchar_t unifications and so on...)

This may be done some day and `name_portable()` method can be added. But I'm afraid this won't happen soon.

Granted it's hard to sort out all the cases. But solving the common cases for the common compilers (eg. leading struct) should be easy, and people could chip away at the other cases over time if they cared to.

According to C++03 and C++11 Standard type_info::name() must return const

...

char*. Getting readable name requires some work and an internal buffer inside boost::type_info. This looks like a bad solution (so "raw_name()" and "name()" won't fit).

But you're not implementing std::type_info, you're implementing boost::type_info, so you should be free to return a std::string if you feel like it. You're already changing the behaviour on MSVC, because std::type_info::name returns the long name but boost::type_info::name returns the short name.

This will break the ability of TypeIndex to be a drop-in replacement for typeid: boost::any a; ... const char* m = a.type().name(); // with boost::type_info this would be impossible

Or how about: * const char *name() : returns whatever std::type_info::name does [if RTTI] * std::string short_name() : returns the raw/mangled name * std::string long_name() : returns the long/demangled name

Or flip the components (name_short and name_long) if you prefer; it sorts better but reads worse that way, I think.

(With RTTI disabled, all three would probably return the same value.)

This is a good idea, thanks! Using it less user code will be broken. Do you like the following names: * const char* name() // minor behavior change on MSVC * const char* name_mangled() // this method must be added * std::string name_demangled() // remains exactly the same -- Best regards, Antony Polukhin

2013/11/13 Andrey Semashev

On Wed, Nov 13, 2013 at 1:28 PM, Antony Polukhin wrote:

...

2013/11/13 Gavin Lambert

...

Or how about: * const char *name() : returns whatever std::type_info::name does [if RTTI] * std::string short_name() : returns the raw/mangled name * std::string long_name() : returns the long/demangled name

Or flip the components (name_short and name_long) if you prefer; it sorts better but reads worse that way, I think.

(With RTTI disabled, all three would probably return the same value.)

This is a good idea, thanks! Using it less user code will be broken.

Do you like the following names: * const char* name() // minor behavior change on MSVC * const char* name_mangled() // this method must be added * std::string name_demangled() // remains exactly the same

I haven't had the chance to review the library (yet), but I second that boost::type_info::name() should be equivalent to std::type_info::name(), including MSVC.

All special logic of demangling should be implemented in a new function, like name_demangled (and let me suggest a few other names for it - readable_name(), pretty_name()).

name_mangled() is controversial because formally you have no guaranteed way to acquire a mangled name of the type. Some compilers provide it, others don't. I assume, you don't want to implement mangling algorithms for these compilers, so I'm in favor of removing this function or at least changing its semantics and name to something less obligatory, like raw_name() or underlying_name().

Agreed. People may use auto-fill in IDE a lot (I do), so maybe starting methods from name_* is more auto-fill friendly: * const char* name() // same as std::type_info::name() * const char* name_raw() // * std::string name_pretty() // was name_demangled() In this way you just type name_ and see all the available name methods. -- Best regards, Antony Polukhin

2013/11/13 Andrey Semashev

On Wed, Nov 13, 2013 at 2:02 PM, Antony Polukhin wrote:

...

2013/11/13 Andrey Semashev

...

On Wed, Nov 13, 2013 at 1:28 PM, Antony Polukhin wrote:

...

2013/11/13 Gavin Lambert

...

Or how about: * const char *name() : returns whatever std::type_info::name does [if RTTI] * std::string short_name() : returns the raw/mangled name * std::string long_name() : returns the long/demangled name

Or flip the components (name_short and name_long) if you prefer; it sorts better but reads worse that way, I think.

(With RTTI disabled, all three would probably return the same value.)

This is a good idea, thanks! Using it less user code will be broken.

Do you like the following names: * const char* name() // minor behavior change on MSVC * const char* name_mangled() // this method must be added * std::string name_demangled() // remains exactly the same

I haven't had the chance to review the library (yet), but I second that boost::type_info::name() should be equivalent to std::type_info::name(), including MSVC.

All special logic of demangling should be implemented in a new function, like name_demangled (and let me suggest a few other names for it - readable_name(), pretty_name()).

name_mangled() is controversial because formally you have no guaranteed way to acquire a mangled name of the type. Some compilers provide it, others don't. I assume, you don't want to implement mangling algorithms for these compilers, so I'm in favor of removing this function or at least changing its semantics and name to something less obligatory, like raw_name() or underlying_name().

Agreed.

People may use auto-fill in IDE a lot (I do), so maybe starting methods from name_* is more auto-fill friendly:

* const char* name() // same as std::type_info::name() * const char* name_raw() // * std::string name_pretty() // was name_demangled()

In this way you just type name_ and see all the available name methods.

IMHO, it's better when the names differ from the beginning, so you don't have to select the variant in the combo-box. It's enough to just press 'n', 'r' or 'p' to get the required function first in the list. Also, some IDE's allow to select auto-completion choices based on substrings in any part of the function name, not just the beginning.

In any case, I think a clear name is more important than trying to make it work nicely in a particular IDE. There are many IDEs, of varying qualities and levels of sophistication, and in the end the only thing that matters is the code.

Then it will be: * const char* name() // same as std::type_info::name() * const char* raw_name() // mangled/short/not very readable name * std::string pretty_name() // was name_demangled() -- Best regards, Antony Polukhin

2013/11/13 Andrey Semashev

On Wed, Nov 13, 2013 at 2:16 PM, Antony Polukhin wrote:

...

Then it will be:

* const char* name() // same as std::type_info::name() * const char* raw_name() // mangled/short/not very readable name * std::string pretty_name() // was name_demangled()

Looks great to me.

I like it too. Now it is a drop in replacement that supports even the MSVC extensions. -- Best regards, Antony Polukhin

On Wednesday 13 November 2013 12:44:54 Niall Douglas wrote:

On 13 Nov 2013 at 14:28, Antony Polukhin wrote:

...

...

...

* const char* name() // same as std::type_info::name() * const char* raw_name() // mangled/short/not very readable name * std::string pretty_name() // was name_demangled()

Looks great to me.

I like it too. Now it is a drop in replacement that supports even the MSVC extensions.

I'm afraid I disagree - you're breaking the interface contract with std::type_info when RTTI is off.

Previous commenters may not realise that template_info::name() does NOT return the same value as type_info::name() when RTTI is off. It returns an internal unique const char * value, that's all.

AFAIR, std::type_info::name() gives no guarantees of the returned string contents whatsoever. I don't see why boost::type_info::name() should offer anything more. Also, I don't think it has to return the same string with and without RTTI enabled.

My concern is that MSVC's raw_name() does something very explicit: if you read http://msdn.microsoft.com/en-us/library/vstudio/70ky2y6k.aspx, it explicitly says that raw_name() specifically returns the mangled ("decorated") form of the type expression. TypeIndex, with RTTI off, would not return such a string for its raw_name().

I do not see why boost::type_info::raw_name() should have semantics equivalent to MSVC std::type_info::raw_name(), especially since the latter is non- standard. We can implement boost::type_info::raw_name() in terms of MSVC std::type_info::raw_name(), but we surely cannot guarantee that it will always return what MSVC std::type_info::raw_name() does, on other compilers. This is the reason I objected against name_demangled() naming. In my view, boost::type_info::raw_name() should be described as a function returning an implementation-defined string that may be equivalent to boost::type_info::name() but, if possible, may be in a platform-specific mangled format. FWIW, boost::type_info interface description should allow to implement name(), raw_name() and pretty_name() all the same way - based on std::type_info::name() or another adequate source of strings in the lack of native RTTI.

Regarding the problem of name() compatibility: it is deeply unfortunate that MSVC chose that name() should be demangled, while everyone else chose that name() should be mangled. It means you see this everywhere which uses type_info:

#ifdef _MSC_VER ti.raw_name() #else ti.name() #endif

A little irrelevant, but I personally don't think MSVC choice was wrong. I'd prefer std::type_info::name() to return a human readable string on most compilers instead of having to write hacks like __cxa_demangle().

One of the things I really like about boost::type_info is that we do away with this silliness and name() always means "the shortest unique representation string" on every platform.

Again, std::type_info::name() does not have that meaning and there is no reason for boost::type_info::name() to have it. boost::type_info::name() should be exactly what it advertises itself - an equivalent for std::type_info::name(). For any other preference you should be using boost::type_info::raw_name() or boost::type_info::pretty_name().

Can I suggest this instead: instead of making boost::type_info more like the flawed std::type_info, can you leave boost::type_info to be pure and instead add a new boost::type_info_std or something which does replicate std::type_info's implementation specific quirks?

I don't see the point in such duplication. boost::type_index will only work with boost::type_info anyway.

On Wednesday 13 November 2013 14:29:07 Niall Douglas wrote:

On 13 Nov 2013 at 22:46, Andrey Semashev wrote:

...

...

Can I suggest this instead: instead of making boost::type_info more like the flawed std::type_info, can you leave boost::type_info to be pure and instead add a new boost::type_info_std or something which does replicate std::type_info's implementation specific quirks?

I don't see the point in such duplication. boost::type_index will only work with boost::type_info anyway.

There are two primary use cases for boost::type_info: (i) as a better (static) std::type_info (ii) as a direct std::type_info substitute working with RTTI off.

What boost::type_info does not currently provide which std::type_info does is a method of retrieving the type mangling string with RTTI off. It could be easily added if name() can return a std::string, but that is outside the scope of this peer review.

Those use cases are not identical.

Yes, but they also are not exclusive. At least, not with respect to name functions.

I also don't see why my proposal to provide both type_info replacements has anything to do with your arguments which appear to mostly be based on API purity rather than practical concerns. I proposed that boost::type_info be pure and fitting your stated wishes, while a new boost::type_info_std (which is implicitly convertible to a boost::type_info) is quirky. You get what you want, while people wanting a direct drop in perfect substitute for std::type_info also get what they want. Trying to get boost::type_info to on its own be everything to everybody I don't think is wise because the use cases are semantically distinct.

I don't like the idea of having two practically equivalent classes which basically do the same thing. The difference between the classes would be in the behavior, which is not specified by the standard in the first place and can be adjusted in a non-breaking manner in a unified boost::type_info class.

2013/11/13 Andrey Semashev

On Wednesday 13 November 2013 12:44:54 Niall Douglas wrote:

...

Can I suggest this instead: instead of making boost::type_info more like the flawed std::type_info, can you leave boost::type_info to be pure and instead add a new boost::type_info_std or something which does replicate std::type_info's implementation specific quirks?

I don't see the point in such duplication. boost::type_index will only work with boost::type_info anyway.

boost::type_index and std::type_info can be compared with each other. boost::type_info is derived from std::type_info and it can be used in any method that accepts std::type_info. Let me put the question in other way: do we want to support MS extensions? If yes, then advantages are: * less user code will be broken during conversion for std:;type_info to boost::type_info (see for example boost::any::type()) Disadvantages: * supporting vendor specific extensions is a bad practice * we loose the meaning of name() as a "fast short nonreadable name" and get something indeterminated instead * more functions => more fuss for the user * calls to name() will become slower on MS Windows Let's change the balance in a following way (NEW WAY): * const char* name() // same as std::type_info::raw_name() * const char* raw_name() // // same as std::type_info::raw_name() * std::string pretty_name() // was name_demangled() Now we have following advantages: * less user code will be broken during conversion for std::type_info to boost::type_info (see for example boost::any::type()) * we do not loose the meaning of name() as a "fast short nonreadable name" and do not get indeterminated faunctions instead * calls to name() are fast Disadvantages: * supporting vendor specific extensions is a bad practice * more functions => more fuss for the user * calls to name() on MSVC will return different result (however name() may return whatever it whants, according to Standard) But do we *really* want to encourage users that write unportable code? -- Best regards, Antony Polukhin

On 14 Nov 2013 at 10:49, Gavin Lambert wrote:

...

* we loose the meaning of name() as a "fast short nonreadable name" and get something indeterminated instead

Where is that meaning assigned? I don't believe the standard specifies what form the return value will take, just that it's some unique string literal.

That description of its meaning came from me, so I'll answer this: it isn't defined by the standard to be that, but it IS defined by common idiomatic usage to be so. Any type registry in Boost I've seen appears to use name() (or raw_name() if _MSC_VER) as being the least cost way of obtaining a unique string identifier for some type specification. Certainly Antony has striven to duplicate that idiom by avoiding the use of malloc in name(), and hence on pre-C++11 compilers TypeIndex cannot return a string which is identical to what is returned by name() (or raw_name() if _MSC_VER) with std::type_info. I personally think there is a separate use case for both situations i.e. a standards compliant name() which isn't ABI compliant but is equally low cost, and a perfect RTTI-disabled substitute exactly replicating name() but with the cost of fire and forget mallocs - the only thing needing this is code which needs the mangling specifically, or code which needs RTTI disabled code to work perfectly with RTTI enabled code e.g. code which stores name() to persistent storage. For me personally that would imply extra adapters adding such extra functionality if and only if there is sufficient demand for it, but we'll see how things look at the end of peer review. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

On 15 Nov 2013 at 10:51, Gavin Lambert wrote:

Well, thus far when using type_info as an index into a collection I use only its type_info* value (as this is guaranteed by the standard to be unique per type but constant across multiple invocations of typeid on the same type of object), and name() is only used to print a human readable name (via MSVC) for diagnostic purposes (since the standard does NOT guarantee that the string value be unique, unlike the pointer, it's not really suitable for use as a key -- although granted most of the time you can get away with it). It's also faster. :)

Unfortunately typeid() does NOT necessarily return the same type_info instance for some type if done across DLL or shared object boundaries. This is because the compiler generates static type_info instances only for those types where typeid() is used (with weak or selectany linkage), and the linker links them up as if they were any ordinary structure instance. That means a copy per DLL/SO. The workaround is to use the mangled type specification, and to do a string comparison to match them. That works very well, so long as you are using the same compiler. In my own code I have even dumped the mangled form to disc as an easy type identifying string id for an on-disc type database (expect more of this coming for my C++ Now 2014 presentation if it is accepted!). Due to these interop problems Antony's TypeIndex does string comparisons as well. It's the only reliable way I believe.

I think that at a minimum if RTTI is enabled then boost::type_info::name() must return exactly what std::type_info::name() would, on that platform. (This is still "least cost" since it'd just be passing the return value through without modification.)

Me personally I'd prefer name() to always return the mangled name on every platform so it's portable and consistent. Unless one is using a quirks type shim not called boost::type_info. Remember that boost::type_info is not intended to be an exact replacement for std::type_info. It is expected that Boost code using std::type_info will need "porting" to boost::type_info. Antony has provided all the necessary patches required for many Boost libraries.

It should be free to add additional methods (such as raw_name or short_name or long_name or whatever) that can do whatever, including memory allocation and returning temporary values, but as long as drop-in-replacement is a goal it can't be allowed to change the behaviour of name() itself.

Again, it's not intended to be a perfect drop in. I do see Steven's point about it accidentally becoming treated by the compiler as a perfect drop in due to type slicing. How serious that is is something I'll need to consider.

When RTTI is disabled, of course, it has more freedom in the actual returned value because the standard name() doesn't exist. But it still must be able to return a const char* that has static lifetime, because people are expecting that from the RTTI-enabled case. (In particular, it must be legal to call name() on a temporary and use the returned value as a bare const char* at any later point in the code, even once the temporary is out of scope.)

On non-MSVC compilers, merely the attempt to access or create a type_info would generally cause a compiler error if RTTI is disabled. I therefore think that in this use case boost::type_info can merrily do whatever it feels like, with any API definition it feels like including returning std::string from name(). After all, the code didn't compile at all before, so no backwards compatibility is needed here - if it compiles at all and no semantics are broken, who cares? On MSVC, typeid() works just fine as does type_info if RTTI is disabled. It's just that typeid() can no longer cope with polymorphism and the compiler warns if you try to use typeid with a polymorphic type. Here things are tougher if and only if there is Boost code written to work on MSVC with RTTI off. I don't believe there is any such code in Boost, and therefore this is also a non-issue.

If the library is intended to be an alternative implementation that is *not* a drop-in replacement, then some of these strictures could be relaxed a bit (and it shouldn't inherit from std::type_info any more).

My overwhelming concern with boost::type_info is whether the present implementation will cause surprises. Antony has already done a lot of work to remove a lot of surprises by generating warnings or static assertion failures. I need to ponder more deeply the concern about undefined behaviour during a type slicing cast vs having it act as a proxy to a const std::type_info & supplied via a constructor. It'll take me a few days to ponder this I think. The latter is not undefined behaviour, but do you actually gain anything in practice? Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

On 15 Nov 2013 at 13:44, Gavin Lambert wrote:

True, but how often does generation and comparison of a typeid actually need to cross DLL/SO boundaries? (Storage might, but generally that's just as an opaque value.) Typically usage would either be fairly localised or would be in templates -- and while it's possible to export and import templates across DLL/SO boundaries, it's usually not actually done.

I beg to differ! A classic example is Boost.Python where more than one Python extension DLL module is loaded, both with a dependency to the same Boost.Python DLL instance. Boost.Python's type registries correctly store and compare mangled strings to cope with this quite common situation.

...

Me personally I'd prefer name() to always return the mangled name on every platform so it's portable and consistent. Unless one is using a quirks type shim not called boost::type_info.

I kind of see that the other way around -- if you want to call something "type_info", then it ought to behave as close as possible like the standard "type_info" (albeit with some extras). If you want different behaviour you should give it a different name. :)

Yes and no. There is semantic equivalence, API equivalence and ABI equivalence. They're all not necessarily the same thing. I personally don't mind a type_info which fixes the mistakes in the standard type_info if it causes the compiler to puke instead of hidden misoperation. There is something to be said for eliminating the name() function altogether actually as its meaning is too vague and non-portable. Removing it in boost::type_info guarantees that the porting effort makes the author choose exactly what they mean with all the runtime costs implied (e.g. a demangled type string is usually run time calculated and longer than a mangled type string).

...

Remember that boost::type_info is not intended to be an exact replacement for std::type_info. It is expected that Boost code using std::type_info will need "porting" to boost::type_info. Antony has provided all the necessary patches required for many Boost libraries.

I have received the opposite impression from discussion thus far. Perhaps I have misinterpreted something.

People jumped in with their particular preferred angles, as is normal during peer review. Also as is normal during peer review the relevance of the discussion to the library being reviewed can become quite tenuous. In this situation we've had a very good, and very useful (to me as the review manager) discussion. It helps hugely the library is so small and single purpose.

Again, if it's not being positioned as a drop-in then it's less of an issue (but means more user code might need to be changed). But then if it's not being positioned as a drop-in then I don't know why it's derived from std::type_info and doing the casts the way it is. Maybe there's some other reason for that?

Antony was trying to save on code bloat, amongst other things. The UB based implementation had the huge benefit of zero bloat with RTTI on. I think unfortunately considering the rationale I gave in a previous message we probably can't go with that, it's too UB due to the virtual function table mismatch. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

On 22 Nov 2013 at 18:28, Gottlob Frege wrote:

...

There is something to be said for eliminating the name() function altogether ...

Yes, first determine what types of name functions you want (mangled, demangled, compatible, unique, etc). Then supply ALL those functions.

Indeed, this is what I suggested to Antony off-list: /*! Returns a static const char string of unknown format uniquely identifying this type. The only guarantee is that this string will be unique to the type within this process lifetime.*/ const char *unique_name() const noexcept; /*! Returns a representation of this type suitable for printing. This call may take some time as its storage may not be cached.*/ std::string pretty_name() const; class enum mangling { Native, //!< Whatever the native mangling used by this toolset is MSVC, //!< The Microsoft C++ mangling format Itanium //!< The Itanium C++ mangling format }; /*! Returns the mangled form of the string representation of the type. After the calculation the value is cached statically such that the c_str() function can be used to convert the returned string to a const char * format identical to what may be returned by std::type_info::name() (or raw_name() on MSVC). This function may throw an exception if it does not support mangled type string calculation, including when mangling=Native on some platforms.*/ const std::string &mangled_name(mangling=Native) const; Note I have not convinced Antony of the merit of this approach (i.e. of not providing a name() function at all, and leaving it up to user code to subclass boost::type_index if so desired with something which does implement some name() function locally to that specific library). Thanks for the feedback Tony, and I hope things at our former employer aren't too bad. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

On 25 Nov 2013 at 12:23, Gavin Lambert wrote:

Given that I'm still mostly using not-C++11 compilers I get a little nervous when I see "class enum"s. Not that this is a reason to not do it. :)

Oh sure, the code I wrote was purely for illustration. It would get wrapped in the appropriate Boost class enum macro magic in practice.

I'm not sure what the motivation for having the returned mangled type configurable is though. Presumably most existing use cases would just want Native, and the burden of providing alternates might be significant (and therefore left unimplemented and throwing an exception, as specified, limiting the potential usefulness of this anyway). Though as I've said before I admit to ignorance about many of the use cases, so maybe there is a need for this that I am unaware of.

You're right I should have explained this much better. There is a need for a truly portable type specification string such that code from GCC can work with code from MSVC and so on where the type string is being kept in storage. Antony wants to (much later on) add a custom type mangling scheme to TypeIndex which would be universal and absolutely as compact as possible (symbol mangling schemes are designed to be fast rather than compact). I simply had the class enum in there to facilitate such an extension later. (Also, I think it very possible that clang might one day let you generate any mangling scheme you like, so I didn't want to rule that out).

Why not just specify a mangled_name that will return the native mangled name if it can, but is allowed to return the same as unique_name? Or is that too close to what unique_name itself will do?

mangled_name() here is *guaranteed* to return exactly what std::type_info does, even if RTTI is turned off. I would expect such a facility to be highly useful for where boost::type_index binaries might be mixed with std::type_info binaries and you need to compare a type_index indexed type with a std::type_info indexed type. Also, of course, sometimes you really do want the exact symbol mangling to be returned because you're going to parse that mangling into a AST, and use that AST to auto-generate yourself some LLVM :). Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

AMDG On 11/13/2013 12:17 PM, Antony Polukhin wrote:

2013/11/13 Andrey Semashev

...

On Wednesday 13 November 2013 12:44:54 Niall Douglas wrote:

...

Can I suggest this instead: instead of making boost::type_info more like the flawed std::type_info, can you leave boost::type_info to be pure and instead add a new boost::type_info_std or something which does replicate std::type_info's implementation specific quirks?

I don't see the point in such duplication. boost::type_index will only work with boost::type_info anyway.

boost::type_index and std::type_info can be compared with each other. boost::type_info is derived from std::type_info and it can be used in any method that accepts std::type_info.

<snip>

Let's change the balance in a following way (NEW WAY): * const char* name() // same as std::type_info::raw_name() * const char* raw_name() // // same as std::type_info::raw_name() * std::string pretty_name() // was name_demangled()

As far as I am concerned, boost::type_info::name *must* return the same value as std::type_info::name. You have to expect that use of boost::type_info will be mixed with std::type_info. It's unreasonable to expect everything to use boost::type_info, and if the behavior is different, it *will* cause surprises. You could perhaps make a case for different behavior if boost::type_info were completely separate, but since boost::type_info inherits from std::type_info, there's absolutely no excuse. In Christ, Steven Watanabe

On 13/11/2013 23:16, Quoth Antony Polukhin:

* const char* name() // same as std::type_info::name() * const char* raw_name() // mangled/short/not very readable name * std::string pretty_name() // was name_demangled()

That'd be ok, but the reason why I suggested "short" and "long" rather than "raw" and "pretty" is that even "raw" and "pretty" make implications that we might not follow (eg. with RTTI off, all three will return the same value, which might be neither raw nor pretty, but is at least relatively both short and long if it's the only possible name). (And a future extension might be an "even prettier name" that tries to remove leading "struct" etc.) Also, earlier in the thread you dismissed the idea of storing a string buffer within the type_info, but I'm wondering if that might not be a good idea after all. If type_info is permitted to contain std::string members that are initially empty but populated on first call to their corresponding name function, then repeated calls to the same name function would only pay any demangling/parsing cost on the first call. It won't make any difference in the direct typeid rvalue->name case, but if code is keeping a type_info instance around (eg. as a key in a container) it seems reasonable to assume that one of the name methods might get called more than once. Slight downside is the extra size of the type_info (particularly in the case where it is used without ever asking for the name), but I don't think this is too onerous. It's tempting to also make all the methods return "const char *" (to possibly save on copies) if we have member strings to return them from, but this is still potentially dangerous if the user assumes the string has static lifetime when called on a temporary type_info (which is probably a common pattern for the drop-in-replacement case). On the other hand, the drop-in-replacement probably wouldn't be calling pretty_name/long_name so it might not matter; we'd just need to document that the return value should immediately be saved to a std::string if called on a temporary type_info, or that the type_info should be kept around longer. (This would be adding a copy in the temporary case to save copies in the repeated-call case though. I'm not sure which direction would be better to optimise towards.)

2013/11/14 Steven Watanabe

AMDG

On 11/12/2013 11:34 AM, Niall Douglas wrote:

...

Boost community feedback is requested for the formal peer review of the TypeIndex library by Antony Polukhin. Feedback requested includes:

1. Should this library be accepted into Boost?

No. This library should not be accepted into Boost in its current form.

type_info.hpp:139: return static_cast(typeid(type));

This is undefined behavior, and there is no way to make it correct.

This is a necessary evil and it is harmless: * all the methods of std::type_info (except destructor) are non virtual, so for example calls to name() will call boost::type_info::name() * it is *undefined* which destructor (std::type_info or boost::type_info) will be called *but*: * boost::type_info contains no members, it's size must be equal to the size of std::type_info, so no harm if boost::type_info destructor won't be called * destructor of std::type_info will be called anyway I can add assert to tests to ensure that sizeof(std::type_info) == sizeof(boost::type_info). -- Best regards, Antony Polukhin

2013/11/14 Andrey Semashev

On Thu, Nov 14, 2013 at 9:26 AM, Antony Polukhin wrote:

...

2013/11/14 Steven Watanabe

...

AMDG

On 11/12/2013 11:34 AM, Niall Douglas wrote:

...

Boost community feedback is requested for the formal peer review of the TypeIndex library by Antony Polukhin. Feedback requested includes:

1. Should this library be accepted into Boost?

No. This library should not be accepted into Boost in its current form.

type_info.hpp:139: return static_cast(typeid(type));

This is undefined behavior, and there is no way to make it correct.

This is a necessary evil and it is harmless: * all the methods of std::type_info (except destructor) are non virtual, so for example calls to name() will call boost::type_info::name() * it is *undefined* which destructor (std::type_info or boost::type_info) will be called *but*: * boost::type_info contains no members, it's size must be equal to the size of std::type_info, so no harm if boost::type_info destructor won't be called * destructor of std::type_info will be called anyway

I can add assert to tests to ensure that sizeof(std::type_info) == sizeof(boost::type_info).

I think the undefined behavior can be removed with the following trick:

1. boost::type_info should not derive from std::type_info (it would have no base classes or data members). Its destructor should be deleted, as well as its constructors and assignment. 2. The above cast should be replaced with reinterpret_cast:

return reinterpret_cast(typeid(type));

3. All methods of boost::type_info should perform the reverse reinterpret_cast of *this before accessing std::type_info.

The standard warrants that such two-way casting would yield the original reference to std::type_info.

However, this turns out to be quite a lot of hackery and makes me wonder if boost::type_info is needed at all. Why not just have boost::type_index that will work directly with std::type_info?

Unfortunately, boost::type_info must derive from std::type_info for compatibility reasons. Without it the following code won't compile: std::type_index ti = boost:type_id<int>(); This may look like a rare case, however will be quite common if other Boost libraries adopt the TypeIndex. Is my favorite example with Boost.Any: boost::any a = 10; // Imagine that Boost.Any uses boost:;type_info instead of std::type_info class_that_constructs_from_std_type_info = a.type(); // operator const std::type_info&() won't help std::type_index ti = a.type(); In v1.0 of TypeIndex library there was no boost::type_info, there was only boost::type_index. version 2 was started because v1.0 was hard to use in existing Boost libraries that that return std::type_info to the user. Version 1.0 is not a drop-in replacement. It can be used only if we have full control over the code that uses it. Why there is a such need in drop-in replacement? Here are the reasons: * some compilers fail to compare std::type_info across modules (surprise!) * some libraries fail to write a sane hashing function for std::type_info * users want to use Boost without RTTI and fail: http://stackoverflow.com/questions/14557313/is-it-possible-to-use-boost-prog... Here are some more issues in Boost: * there is a mess with stripping/not stripping const, volatile, reference * there is a mess with name() calls, plenty of places where name() used for debug/user output without demangling * some of the Boost libraries demangle names by themselfs and forget to dealocate memory (at least valgrind says so) In three words: std::type_info is broken. There is a need to give users an ability to write portable code, but do not break existing users code (that do fail on some compilers). There is a need to put all the std::type_info workarounds all around the Boost in s single place, which will be easy to maintain. It is not good to see in different libraries code like that: // Borrowed from Boost.Python library: determines the cases where we // need to use std::type_info::name to compare instead of operator==. #if defined( BOOST_NO_TYPEID ) # define BOOST_FUNCTION_COMPARE_TYPE_ID(X,Y) ((X)==(Y)) #elif (defined(__GNUC__) && __GNUC__ >= 3) \ || defined(_AIX) \ || ( defined(__sgi) && defined(__host_mips)) # include <cstring> # define BOOST_FUNCTION_COMPARE_TYPE_ID(X,Y) \ (std::strcmp((X).name(),(Y).name()) == 0) # else # define BOOST_FUNCTION_COMPARE_TYPE_ID(X,Y) ((X)==(Y)) #endif Or like this: // See boost/python/type_id.hpp // TODO: add BOOST_TYPEID_COMPARE_BY_NAME to config.hpp # if (defined(__GNUC__) && __GNUC__ >= 3) \ || defined(_AIX) \ || ( defined(__sgi) && defined(__host_mips)) \ || (defined(__hpux) && defined(__HP_aCC)) \ || (defined(linux) && defined(__INTEL_COMPILER) && defined(__ICC)) # define BOOST_AUX_ANY_TYPE_ID_NAME #include <cstring> # endif Did you note that macro *differ*? And by the way, in both cases it is not optimal! -- Best regards, Antony Polukhin

2013/11/14 Andrey Semashev

On Thu, Nov 14, 2013 at 11:48 AM, Antony Polukhin wrote:

...

Unfortunately, boost::type_info must derive from std::type_info for compatibility reasons. Without it the following code won't compile:

std::type_index ti = boost:type_id<int>();

This may look like a rare case, however will be quite common if other

Boost

...

libraries adopt the TypeIndex.

I think the above need not to work. After all, you don't expect this to work:

std::shared_ptr< int > p = boost::make_shared< int >(10);

It should be no problem with sticking to boost::type_index and boost::type_id in Boost libraries, and for user's code we could provide means to convert between boost::type_index and std::type_index.

...

Is my favorite example with Boost.Any:

boost::any a = 10;

// Imagine that Boost.Any uses boost:;type_info instead of std::type_info class_that_constructs_from_std_type_info = a.type();

// operator const std::type_info&() won't help std::type_index ti = a.type();

Boost.Any can use boost::type_index internally and a.type() can still return std::type_info const& for backward compatibility. We just need a method such as this:

std::type_info const& boost::type_index::get() const;

...

Why there is a such need in drop-in replacement? Here are the reasons:

* some compilers fail to compare std::type_info across modules (surprise!) * some libraries fail to write a sane hashing function for std::type_info * users want to use Boost without RTTI and fail:

http://stackoverflow.com/questions/14557313/is-it-possible-to-use-boost-prog...

...

Here are some more issues in Boost: * there is a mess with stripping/not stripping const, volatile, reference * there is a mess with name() calls, plenty of places where name() used

for

...

debug/user output without demangling * some of the Boost libraries demangle names by themselfs and forget to dealocate memory (at least valgrind says so)

In three words: std::type_info is broken.

I think that these problems can be tackled by just having boost::type_index and boost::type_id. boost::type_index can implement the name functions we discussed and offer interfacing with std::type_info and implement workarounds for its bugs. boost::type_id (or preferably its variations) can solve problems with cv-qualification, references and visibility by wrapping the type before constructing boost::type_index.

The only reason for boost::type_info I see is to emulate RTTI when it's disabled. But as I see in the code, it is just an alias for template_info in this case. So perhaps we should just make it that - an alias to std::type_info or boost::template_info, depending on the configuration. This way it will just allow to use the type_info type portably, without preprocessor checks everywhere.

We are arguing about class type_info: public std::type_info { ...}; vs class type_info{ const std::type_info* ptr_; public: const std::type_info& get(); ... }; typedef ... std_type_info_if_exist_t; Can you explain me once more, what are the advantages of the second solution? -- Best regards, Antony Polukhin

2013/11/14 Andrey Semashev <...>

class type_index { boost::type_info const* m_type_info;

public: explicit type_index(boost::type_info const&); boost::type_info const& get() const; // as previously discussed: const char* name() const; const char* raw_name() const; std::string pretty_name() const; };

Don't really like the "get()" name. Following name is more clear: public: boost::type_info const& type_info() const noexcept; Ok? -- Best regards, Antony Polukhin

Unfortunately, boost::type_info must derive from std::type_info for compatibility reasons.

It sounds like you just want to extend the functionality of `std::type_info`. Why not make `pretty_name()` a free function that accepts `std::type_info`? Also, an additional `compare_type_info()` function could be added as well to compare types portably. Plus, it would avoid the issues with using strange hacks that border on undefined behavior.

* users want to use Boost without RTTI and fail

Additionally, `boost::type_info` could be a typedef to `std::type_info` when rtti is enabled and then to some internal type to manage type info when rtti is disabled. -- View this message in context: http://boost.2283326.n4.nabble.com/TypeIndex-Peer-review-period-for-library-... Sent from the Boost - Dev mailing list archive at Nabble.com.

AMDG On 11/13/2013 09:26 PM, Antony Polukhin wrote:

2013/11/14 Steven Watanabe

...

On 11/12/2013 11:34 AM, Niall Douglas wrote:

...

Boost community feedback is requested for the formal peer review of the TypeIndex library by Antony Polukhin. Feedback requested includes:

1. Should this library be accepted into Boost?

No. This library should not be accepted into Boost in its current form.

type_info.hpp:139: return static_cast(typeid(type));

This is undefined behavior, and there is no way to make it correct.

This is a necessary evil and it is harmless: * all the methods of std::type_info (except destructor) are non virtual, so for example calls to name() will call boost::type_info::name() * it is *undefined* which destructor (std::type_info or boost::type_info) will be called *but*: * boost::type_info contains no members, it's size must be equal to the size of std::type_info, so no harm if boost::type_info destructor won't be called * destructor of std::type_info will be called anyway

No, it is not harmless. You cannot safely assume that undefined behavior is ever harmless, just because you can't think of anything that can go wrong. In particular, the compiler is free to make assumptions that can cause your code to miscompile with optimizations on.

I can add assert to tests to ensure that sizeof(std::type_info) == sizeof(boost::type_info).

In Christ, Steven Watanabe

AMDG On 11/14/2013 12:29 PM, Niall Douglas wrote:

On 14 Nov 2013 at 7:01, Steven Watanabe wrote:

...

No, it is not harmless. You cannot safely assume that undefined behavior is ever harmless, just because you can't think of anything that can go wrong. In particular, the compiler is free to make assumptions that can cause your code to miscompile with optimizations on.

For the purposes of my peer review report, can you say if you know of any misoperation which any major compiler might do in this circumstance?

I believe that this was a problem for Boost.Move with gcc, which does something similar. It's actually quite common for old C code that casts between different structs to fail with newer compilers. For instance, Python does not work without -fno-strict-aliasing. In Christ, Steven Watanabe

AMDG On 11/15/2013 05:50 AM, Antony Polukhin wrote:

But this error appears only when we mutate data ( https://svn.boost.org/trac/boost/wiki/Guidelines/WarningsGuidelines search for "strict-aliasing rules")

...and can be arbitrarily broken by (unknown) future optimizations, with no recourse. We wouldn't have all the problems with strict aliasing that we do today if programmers 10 years ago hadn't made exactly the same argument that you are, now. (Sure, it's undefined behavior, but we know what the compiler actually does, so it's really okay...). In Christ, Steven Watanabe

On 15 Nov 2013 at 19:45, Antony Polukhin wrote:

...

...and can be arbitrarily broken by (unknown) future optimizations, with no recourse. We wouldn't have all the problems with strict aliasing that we do today if programmers 10 years ago hadn't made exactly the same argument that you are, now. (Sure, it's undefined behavior, but we know what the compiler actually does, so it's really okay...).

You're absolutely right. I've liked the idea of "make it in a way that user won't notice a difference but typeid issues will be solved" so much, that turned a blind eye to a hack with UB.

This will be fixed (possibly code from v1.0 will be taken, which had no boost::type_info and contained type_index class that had a pointer to std::type_info).

Do you have other ideas of what can be fixed/improved?

Before you make any substantial changes here Antony, I have been busy pondering on this UB issue and on what to recommend to the review wizards. Here are the facts: 1. The problem code is like this: static_cast(typeid(no_cvr_t)); ... where typeid() returns a std::type_info and where boost::type_info singly inherits publicly from std::type_info and no other type. 2. Note that std::type_info contains a virtual function table on libstdc++, Dinkumware and probably any reasonable STL implementation. Therefore boost::type_info will also contain a virtual function table. 3. The problematic static cast is an upcast to a derived type, not a cast between unrelated types. No multiple or virtual inheritance is in play. 4. Which would be legal if and only if typeid originally returned a boost::type_info, because you can legally cast to a base type and back to a derived type. Here are my thoughts: 1. I would be happy with the static upcast if no virtual function table were in play because it's single inheritance, and static_cast provides enough safeguards (e.g. erroring if the type's inheritance tree is unknown at the point of static_cast use). It's a very common pattern in C++ code and is used throughout (especially older parts of) Boost. It is sufficiently common that no future compiler could ever disable it or misoperate with it. I agree that could hinder significantly future C++ features, but that is not TypeIndex's problem because of so much preexisting usage. It's an established idiom, and that's that: in my opinion TypeIndex has the right to use established idioms if they are currently safe with all possible present compiler technologies as implied by the present C++ standard. 2. Unfortunately, there IS a virtual function table, and therefore this line would fail: dynamic_cast(typeid(no_cvr_t)); where this line does not fail: static_cast(typeid(no_cvr_t)); That seems to me to be unacceptable. If a virtual function table is present, it MUST be correct with respect to the type the compiler is using it as. So, Antony, if you can get the virtual function table to correctly say it's a boost::type_info and not a std::type_info, I'll recommend in my report this is fine. I don't mind a certain amount of evil :) to achieve that given it's such an isolated case which can be easily wrapped in a single location. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

On 15 Nov 2013 at 13:34, Steven Watanabe wrote:

...

problem because of so much preexisting usage. It's an established idiom, and that's that: in my opinion TypeIndex has the right to use established idioms if they are currently safe with all possible present compiler technologies as implied by the present C++ standard.

I don't agree that this is an idiom. I know that Boost.Move uses it, but I'm not aware of any other Boost library that does. Boost.Move is not nearly as problematic, because it only uses this in the fallback for C++03. It will eventually be phased out in C++11.

Let me clarify myself, as I've suddenly realised I was being unclear: Let there be a DLL exporting function: struct Foo { ... }; struct MoreFoo : Foo { ... }; MoreFoo &convert(Foo &in) { return static_cast(in); // upcast to more derived type } Let there be another DLL in which we do: MoreFoo a; auto b=convert(a); // legal Foo c; auto d=convert(c); // not legal Where I was coming from was the case of the first DLL: there the compiler cannot know if the inbound Foo & originally came from a MoreFoo or not. It simply must proceed as if it did. That's what I meant by a very common idiom - we write code like the first DLL all the time where we cannot possibly know if we are doing undefined behaviour, because that's on the caller, not us. The shared library boundary makes avoiding this impossible. If we implement C++ Modules, then it all gets much more interesting, and that's what I was referring to about this idiom being a problem for future C++ features. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

On Friday 15 November 2013 16:09:38 Niall Douglas wrote:

Here are my thoughts:

1. I would be happy with the static upcast if no virtual function table were in play because it's single inheritance, and static_cast provides enough safeguards (e.g. erroring if the type's inheritance tree is unknown at the point of static_cast use). It's a very common pattern in C++ code and is used throughout (especially older parts of) Boost. It is sufficiently common that no future compiler could ever disable it or misoperate with it. I agree that could hinder significantly future C++ features, but that is not TypeIndex's problem because of so much preexisting usage. It's an established idiom, and that's that: in my opinion TypeIndex has the right to use established idioms if they are currently safe with all possible present compiler technologies as implied by the present C++ standard.

I disagree. Virtual functions or type inheritance are not relevant. The standard is quite clear on this case (see 5.2.9/2), and compilers are allowed to mess up that code. The actually stored object is different from what the reference is being cast to. If a compiler, say, has to adjust the pointer as the part of static_cast, the resulting pointer would be invalid and any access to it would be another UB. I know you rely on the fact of single inheritance and assume that the pointer doesn't need any adjustment. But that is not guaranteed by the standard, and such language implementation would be valid. The preexisting practice is not an argument to me because that's not the kind of practice we'd like to preserve or advertise. That practice does cause a lot of headache from time to time. Gladly, such hacks are not that common nowdays.

2. Unfortunately, there IS a virtual function table, and therefore this line would fail:

dynamic_cast(typeid(no_cvr_t));

where this line does not fail:

static_cast(typeid(no_cvr_t));

That seems to me to be unacceptable. If a virtual function table is present, it MUST be correct with respect to the type the compiler is using it as.

So, Antony, if you can get the virtual function table to correctly say it's a boost::type_info and not a std::type_info, I'll recommend in my report this is fine. I don't mind a certain amount of evil :) to achieve that given it's such an isolated case which can be easily wrapped in a single location.

Ignoring the problem is not acceptable for a new library, IMO. The fact that some select compilers currently bear with such code doesn't mean they won't in the future, let alone there may be compilers that break it already. Putting that kind of jeopardy in the base of the library design is not a good solution by any measure. More so, when a cleaner solution is available.

On 15 Nov 2013 at 17:50, Antony Polukhin wrote:

...

I believe that this was a problem for Boost.Move with gcc, which does something similar. It's actually quite common for old C code that casts between different structs to fail with newer compilers. For instance, Python does not work without -fno-strict-aliasing.

Looks like I've got what are you talking about. It's an optimization in GCC, that assumes that if there is a reference to two different types and neither of those types is char, than those types are different in memory (are not aliases):

void foo(int64_t& v1, int32_t& v2) { /* GCC assumes that address of v1 is not a part of v2 adresses*/ v2 = 2; cout << v1; }

int64_t i64 = 1; foo(i64, reinterpret_cast(i64)); // outputs 1

But this error appears only when we mutate data ( https://svn.boost.org/trac/boost/wiki/Guidelines/WarningsGuidelines search for "strict-aliasing rules")

This isn't really the same thing as Stephen mentioned. The above example is explicitly unrelated types i.e. a int64_t is not a int32_t, and the compiler is permitted to absolutely assume that to be true in the strongest possible way. The strict-aliasing problems in Python and the Linux kernel are due to them casting between unrelated structure types without the proper use of volatile qualification and/or going through void * or char * as the carrying opaque pointer type, such that the compiler's alias optimisation pass will assume that two unrelated type pointers will never refer to the same patch of memory which is permitted by the standard. Using volatile or void * or char * can inhibit that optimisation pass by disabling the ability to assume two pointers cannot refer to the same memory, and therefore inhibit unsafe code being generated. It is of course still undefined behaviour, but it's extremely common in C out of necessity. What TypeIndex does is much safer - it upcasts to a derived and therefore related type, and the compiler knows the types are related (or static_cast would fail). I cannot see how alias optimisation could ever therefore generate unsafe code in this case. Upcasting to a derived type is a very common - if often illegal - idiom in C++, but it is usually safe. It also saves programmers from writing a lot of extra boilerplate, hence its popularity. As I mentioned in the other post, for me it's the incorrectness of the virtual function table which kills this use of UB for me. Not the static upcast. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

AMDG On 11/15/2013 01:48 PM, Niall Douglas wrote:

On 15 Nov 2013 at 17:50, Antony Polukhin wrote:

...

<snip> int64_t i64 = 1; foo(i64, reinterpret_cast(i64)); // outputs 1

But this error appears only when we mutate data ( https://svn.boost.org/trac/boost/wiki/Guidelines/WarningsGuidelines search for "strict-aliasing rules")

This isn't really the same thing as Stephen mentioned.

In all essentials, it is the same. The precise rule that it violates is different, but it's still forbidden. "If a non-static member function of a class X is called for an object that is not of type X, or of a type derived from X, the behavior is undefined" (C++11 9.3.1 [class.mfct.non-static]) In Christ, Steven Watanabe

2013/11/15 Mathieu Champlon

Hi,

Quickly skimming the documentation and code I'm just wondering what exactly would be the problem if I were to do :

template <class T> const type_info& my_type_id( const T& ) { return type_info::construct<T>(); }

my_type_id( variable ) // instead of boost::type_id_rtti_only(variable)

?

type_info::construct<T>() works with the type T, while boost::type_id_rtti_only(variable) attempts to determinate the real type of variable. Here's an example: class Base { virtual *~*Base(){} }; class Derived: public Base {}; ... Derived d; Base& variable = d; my_type_id( variable ).name_demangled(); // returns `Base` boost::type_id_rtti_only(variable).name_demangled(); // returns `Derived ` -- Best regards, Antony Polukhin

2013/11/16 Mathieu Champlon <...>

Do you think getting the type info of the "current" type of a variable (as opposed to it's real type) could sometimes prove useful ? Could it be worth adding a function in TypeIndex ?

This is a very very common use case in Boost and widely used technique in libraries that use type erasure. Examples are Boost.Function, Boost.Any, Boost,Graph, Boost.Math and others... Such function is already in TypeIndex library, see boost::type_info<T>(). Also using a local type as a template argument is illegal in C++03 (not in

C++11 any more). For instance :

void f() { struct s {}; boost::type_id< s >(); // illegal in C++03 typeid( s ); // legal }

Maybe it should be stated in the documentation as this is an area where boost::type_id is not a drop-in replacement for typeid (in C++03) ?

Neat example, but I don't think that this must be mentioned in TypeIndex docs. This is a C++03 related restriction and a rare use case, so it will just complicate the docs. -- Best regards, Antony Polukhin

2013/11/16 Mathieu Champlon

On 16/11/2013 16:58, Antony Polukhin wrote:

...

2013/11/16 Mathieu Champlon <...>

...

(as opposed to it's real type) could sometimes prove useful ? Could it be worth adding a function in TypeIndex ?

This is a very very common use case in Boost and widely used technique in

Do you think getting the type info of the "current" type of a variable libraries that use type erasure. Examples are Boost.Function, Boost.Any, Boost,Graph, Boost.Math and others... Such function is already in TypeIndex library, see boost::type_info<T>().

Yes, but I meant given :

#define SOME_MACRO_IN_MY_LIBRARY( variable ) do_something_complicated_ requiring_type_info_of_type_of_variable

I cannot use boost::type_info<T>() easily can I ? Granted I could do boost::type_info< BOOST_TYPEOF( variable ) >() but that's a bit cumbersome compared to for instance a simple boost::type_info( variable ).

Sorry, I meant boost::type_id ( http://apolukhin.github.io/type_index/boost/type_id.html) Well, usually you have the type of the variable nearby, and can use it in type_id<T>: template <class T> void operator()(const T& var) { type_id<T>(); // no need in type_id(var); } ... int i; type_id<int>() // no need in type_id(i); Cases when it is hard to detect variable type are rare in Boost and I do not really wish to produce many trivial functions and make the library heavier. Anyway, user can always write such function in half a minute: tempalte <class T> inline const boost::type_info& my_type_id(const T&) { return boost::type_id<T>(); } -- Best regards, Antony Polukhin

2013/11/16 Andrey Semashev <...>

...

2. Any conditions which should be attached to acceptance into Boost e.g. fixes, additional testing, changes to documentation. Please be as specific as possible here (bullet points are good!)

While reviewing the library I inspected the code and the documentation (mostly the Getting started and Examples sections). I did not compile the code or tests.

There are a few critical issues that need to be resolved before the inclusion:

* The undefined behavior of casting std::type_info const& to boost::type_info const& has to be removed. I described my preferred solution earlier in the discussion [1], but that particular solution is not a requirement. Anthony can of course choose another solution, as long as it solves the problem and does not undermine the library utility.

Question that disturbs me is, what type must be returned by the type_id<T>() method. Returning just a const std::type_info& seems almost useless, returning const boost::type_index& is not very portable/thread-safe because requires static const type_index variable construction. Must it return a type_index?

* BOOST_CLASSINFO_COMPARE_BY_NAMES macro should be named with the library name as its part (e.g. BOOST_TYPE_INDEX_COMPARE_BY_NAMES). It should probably be made user-definable, if this workaround is needed on some platform Boost doesn't cover. Also, I'm not sure it should be defined by default on gcc <4.5 and Intel on Linux. I'm pretty sure gcc on Linux worked fine since 4.1, if symbol visibility is set correctly (some later version of gcc added an attribute to simplify this) and it's hard to believe that no version of Intel works. Are these checks confirmed by tests?

It is a known bug with GCC on ARM architecture. Additional testing required to check, is it safe to use GCC's comparisons on x86. This can be done later, not during the library review.

* I don't think type_id_rtti_only() is a good name. I'd suggest making these functions overloads of type_id() with no arguments and simply make them optional with #ifndef BOOST_NO_RTTI. I.e.:

template< typename T > type_info const& type_id();

#ifndef BOOST_NO_RTTI template< typename T > type_info const& type_id(T*); template< typename T > type_info const& type_id(T&); #endif

Returning just a std::type_info will require additional care from the user to compare type_infos correctly. Returning a type_index will solve that problem. How about just having a copyable, hashable boost::type_index class that is comparable with std::type_info (something close to v1.0 of TypeIndex)? In that case the library will have anly a boost::type_info typedef, boost::type_index class for RTTI and boost::template_index for situation when RTTI is off. -- Best regards, Antony Polukhin

2013/11/16 Andrey Semashev

On Saturday 16 November 2013 19:46:25 Antony Polukhin wrote:

...

2013/11/16 Andrey Semashev

<...> I don't think there's any use for template_index, as I outlined in my

review. type_index already has all the functionality and should work whether or not RTTI is enabled.

template_index and type_index are two different types intentionally. It's the last attempt to notify user that he has done something wrong: // This is an interface of some library. Library is compiled without RTTI, // user uses this header with RTTI on. void some_function(boost::type_index& ti); If template_index and type_index are two different types and BOOST_TYPE_INDEX_FORCE_NO_RTTI_COMPATIBILITY is not defined, then user will get a linker error on GCC. If those two classes were exactly the same type, there'll be no link error. But user will get undefined behavior with hard detectable errors. Thanks for very good advices and a big review! -- Best regards, Antony Polukhin

On 16 Nov 2013 at 16:53, Andrey Semashev wrote:

...

Feedback requested includes:

1. Should this library be accepted into Boost?

Yes, if the critical conditions below are met (otherwise no). After the required modifications to the library are made, another review is needed, maybe a fast track one.

A very comprehensive feedback, and very useful for my report. Thank you Andrey! Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

On 17 Nov 2013 at 0:41, Klaim - Joël Lamotte wrote:

...

1. Should this library be accepted into Boost?

YES.

Thanks for the feedback. Very useful for my report.

One question though, I'm not totally clear if there is a remarkable difference in performance and binary size between with RTTI using std and without RTTI using this library.

You make a valid point. It's been many years since I last wrote code capable of working with RTTI off, and therefore my knowledge of how well modern compilers generate RTTI is seriously out of date. I have probably been overly conservative as a result by working with a decade old mental model. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

On 18 Nov 2013 at 7:16, Rob Stewart wrote:

...

1. Should this library be accepted into Boost?

No, not in its present state.

Firstly, thank you for the feedback. Very useful for my report. A reminder to the list: peer review period ends tomorrow Thurs 21st Nov. I will write up a report on all the feedback sometime this weekend (I have a complex systems conference at UW all day Thurs and Fri, indeed I'm convening one of the sessions), and I would assume the peer review wizards will probably want another round of peer review, hopefully this time a short (five day) round. My thanks to everyone who has contributed so far. Your peer review is what makes Boost what it is! Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

On 21.11.2013 16:57, Antony Polukhin wrote:

2013/11/21 Jan Herrmann

...

Only from reading documentation I have a few questions:

1. In http://apolukhin.github.io/type_index/boost_typeindex/ examples.html#boost_typeindex.examples.getting_through_the_inheritance_is written that polymorphic types can't be identified without RTTI. It should be possible for a library user to make his types compatible with Boost.TypeIndex to identify them. For example he can return boost::type_info from a virtual function inside each class:

struct A { virtual ~A(){} virtual boost::type_info const& get_boost_type_info() const { return boost::type_id<A>()); } }; struct B: public A { virtual boost::type_info const& get_boost_type_info() const { return boost::type_id<B>()); } }; struct C: public B { virtual boost::type_info const& get_boost_type_info() const { return boost::type_id<C>()); } };

Boost.TypeIndex should provide a way to use such methods inside boost::type_id_rtti_only. Is something like this planned?

This looks like a *very* good addition, thanks for the idea! This will be implemented as soon as possible (before the next review).

I think some macros can make it very easy to implement this for clients.

...

2. As far as I understand almost everything inside the library is build on top of string comparison. All these strings are available at compile time. Are there plans to make boost::template_info a literal type? It would help with metaprogramming to have a system representing types with the same set of operations and behavior at compile time and run time (I think of sorting classes at compile time, store boost::template_info of them in an array and use some binary search at run time).

This can be interesting. That feature will be implemented, however I do not see how this could be possible to do in C++03/C++98.

Yes it's only possible with constexpr, but I see your work as a part for building better TypeIndex like systems. Users should be able to build their own RTTI system with information about types they need. For example a RTTI system could provide information about size and alignment or provide a type erased clone implementation (from a void* to void* with same type).

...

3. What are he exact reasons for storing types with const, volatile and reference in boost::type_id_with_cvr? As long as there is no way to query this information and remove it at run time I think it would better users store them together with type_index if the need this feature. It could result in hard to find errors when users use type_if_with_cvr in one module and not expecting them in another.

This is done for libraries that use type erasure technique a lot. Sometimes there may be a need to store those modifiers (there was an example in examples section for that case: http://apolukhin.github.io/type_index/boost_typeindex/examples.html#boost_ty...)

There must be no problems with mixing boost::type_id_with_cvr and boost::type_id. If there are no modifiers, result will be the same:

boost::type_id_with_cvr<int>() == boost::type_id<int>() boost::type_id_with_cvr() == boost::type_id()

Or am I missing some use case?

I see type erasure in conjunction with type recovery. And I think these bits of information are not worth. If it is possible that somebody calls boost::type_id_with_cvr in one module other modules which needs to probe for types have to check every type 12 times (2 (const) x 2 (volatile) x 3(no ref, ref, rvalue rev). Think of implementing a ad-hoc visitor (http://www.drdobbs.com/generic-programmingtypelists-and-applica/184403813) with TypeIndex and void*. When implementing a function which only needs a const value client has to probe for const and non-const types only to get a const value out of it. I'm not sure whether volatile should be considered. For example no container in std has overloads for volatile access to elements, but they are all const correct. References are another problem. In case of type erased storage they are very distinct form values but sometimes they should be used like values. If I remember correctly there where long debates about std::optional and references in isocpp.proposals or general this year. Arguments could be same here. I wouldn't feel comfortable to work with type properties without being able to query them. Type recovery is hard enough. But if somebody needs these information there are a few ways. One would be to align type_index at a 8 Byte boundary so users could store pointers and would have 4 bits. The other option is to let users build their own facilities via composition of some state variable and type_index. Jan Herrmann