Peer Review Report for proposed Boost.TypeIndex v2.1 Nov 12th – 21st 2013
Please let me know as soon as possible any errata, missing credits etc. Niall Boost Peer Review Report for proposed Boost.TypeIndex v2.1 Nov 12th – 21st 2013 This report is the first edition, dated Sunday 24th November 2013. The review manager was Niall Douglas http://www.nedprod.com/. Source: https://github.com/apolukhin/type_index/zipball/master Documentation: http://apolukhin.github.com/type_index/index.html Peer review discussion detail: http://boost.2283326.n4.nabble.com/TypeIndex-Peer-review-period-for-library-... My thanks to the Boost members whose peer review makes up this report: Robert Ramey, Vicente J. Botet Escriba, Gavin Lambert, Andrey Semashev, Steven Watanabe, pfultz2, Mathieu Champlon, Joël Lamotte Klaim, Rob Stewart, Jan Herrmann, Tony Van Eerd. Contents: · Summary of consensus from peer review ......................................................................................................... 1 · My recommendation to the library author and the Review Wizards ................................................................ 3 Summary of consensus from peer review: The consensus feedback was to accept TypeIndex after substantial modifications. There were two recommendations of rejection due to how substantial such modifications would need to be (i.e. a request for a second round of peer review with a new implementation). Note that my personal opinion based on post-review reflection is written in [square bracketed italics]. · Many seemed to feel that a boost::type_info’s member function API, where identically named to that of std::type_info member function API, ought to have identical compile-time effects. This implied that missing member function APIs, or other compile time errors for missing or incomplete functionality, would be okay but if name() returns some const char *, then any const char * so long as it meets the C++ standard is okay. [I think there are three use cases for a type_info like substitute: (i) as a lightest possible weight, non-RTTI requiring minimal type_info which can have any API it likes and ought to not be directly substitutable for std::type_info or std::type_index (ii) as a compile-time substitute for std::type_info in that it will compile without third party code modification, but may not produce reliable code due to not returning exactly the same values as std::type_info (iii) as a runtime substitute for std::type_info in that it will require code to be “ported” to it due to having a breaking API, but thereafter produces reliable code. I’ll be very blunt in saying that (ii) is a bad idea in my opinion, and (iii) is valuable but out of scope enhancement which could be added as a separate class after peer review. In my opinion, (i) is where we ought to focus, and I would recommend that the lightest possible weight type_info replacement be deliberately compile-time incompatible with std::type_info (where output is not identical to std::type_info) to force porting code to it so authors are not lazy.] · A large minority seemed to feel that a boost::type_info’s member function API, where identically named to that of std::type_info member function API, ought to have identical run-time outcomes. Most specifically, this would mean that member functions e.g. name() would return exactly what std::type_info’s name() does, even if for example name() returns a std::string instead of a const char * as would be necessary in pre-C++11 compilers. [This is really use case (iii) in the previous item.] · It was mentioned that it should be possible to optionally specialise std::hash<> for type_info for optional seamless use in hash taking containers. · Some felt that implementation-specific std::type_info quirks ought to be replicated. Others felt they should not, and a portable interface which works identically on all platforms is preferred. [This is really use case (iii) in the previous item.] · boost::type_info is currently initialised via a static cast from a std::type_info when RTTI is available. This is undefined behaviour, and some felt this to be a showstopper. I (Niall Douglas) looked into this more deeply due to its seriousness, and found that because most STL implementations define std::type_info with a virtual function table, the undefined behaviour static cast would cause dynamic_cast on a boost::type_info instance to misoperate. [I believe the RTTI induced misoperation to indeed be a showstopper, and that this use of undefined behaviour to reduce code bloat is an unsafe optimisation. A much lighter weight non-direct-substitute for std::type_info ought to be as code bloat parsimonious as is possible, while its lack of non-explicit interoperation ability with std::type_info ought to allow avoidance of needing any UB tricks.] · Some felt that any use of implied boost::type_info ought to be explicitly written as boost::type_info instead of relying on namespace lookup, as the name similarity to std::type_info may introduce confusion. [I agree] · It was mentioned that some function-local static data members are initialised in a thread unsafe way. [For information I have to hand a very lightweight BOOST_BEGIN_MEMORY_TRANSACTION() implementation ideal for this purpose. We could submit it to Boost.Detail and then everyone could use it?] · Magic macros such as __PRETTY_FUNCTION__ ought to not be referenced outside of a function as they don’t technically exist there. Checks for their existence ought to be within a function. · It was requested that a boost::type_id<> which takes some unknown variable instance as a parameter and therefore allows the compiler to deduce the type of the variable as the boost::type_index<T> type would be valuable. · A mechanism for programming compile-time logic with class inheritance trees such that inheritance can be determined at compile-time with RTTI disabled was requested. My recommendation to the library author and the Review Wizards: · In my opinion Antony ought to make TypeIndex v3 quite literally a very lightweight container of some unknown, but known to be uniquely identifying for some type, static const char * string. I think its class type and its list of member functions ought to be deliberately compile-time incompatible with std::type_info to force authors to upgrade their code. A conversion member function ought to be able to synthesise a corresponding std::type_info using typeid() from some boost::type_index<T>, but that’s about it. I would even, personally speaking, go so far as to only provide a boost::type_index and no corresponding boost::type_info, especially if the boost::type_id<T>() function can return a const boost::type_index<T>& and therefore can be used as a static const lref, or copy constructed from it etc. A suggested name() member function replacement which correctly breaks out the multiple confounding uses of std::type_info::name() into each of their three use cases (and which intentionally causes any use of name() to fail to compile) might be: Text Box: /*! Returns a static const char string of unknown format uniquelyidentifying this type.The only guarantee is that this string will beunique 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 isMSVC, //!< The Microsoft C++ mangling formatItanium //!< 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() (orraw_name() on MSVC).This function may throw an exception if it does notsupport mangled type string calculation, including when mangling=Native onsome Bear in mind that user code can always subclass boost::type_index and add their own name() implementation based on one or more of the above new member functions. · In other words, here I think “less is more” in every way you look at it. I think a slimmer less heavy TypeIndex would be a superior solution. · I request the Review Wizards to allow Antony some time to make the changes to produce a v3 of the library, and then to allow TypeIndex v3 a Fast Track Review of five days to ensure the Boost community is happy and that its feedback has been incorporated. It was asked during peer review that such a five day period please include a weekend. I would be happy to serve as review manager again if requested. Niall Douglas Waterloo, Canada, November 2013
Please let me know as soon as possible any errata, missing credits etc.
Niall
Boost Peer Review Report for proposed Boost.TypeIndex v2.1 Nov 12th - 21st 2013
This report is the first edition, dated Sunday 24th November 2013. The review manager was Niall Douglas http://www.nedprod.com/.
Source: https://github.com/apolukhin/type_index/zipball/master
Documentation: http://apolukhin.github.com/type_index/index.html
Peer review discussion detail: http://boost.2283326.n4.nabble.com/TypeIndex-Peer-review-period-for-library-...
My thanks to the Boost members whose peer review makes up this report: Robert Ramey, Vicente J. Botet Escriba, Gavin Lambert, Andrey Semashev, Steven Watanabe, pfultz2, Mathieu Champlon, Joël Lamotte Klaim, Rob Stewart, Jan Herrmann, Tony Van Eerd.
Contents:
· Summary of consensus from peer review ......................................................................................................... 1
· My recommendation to the library author and the Review Wizards ................................................................ 3
Summary of consensus from peer review:
The consensus feedback was to accept TypeIndex after substantial modifications. There were two recommendations of rejection due to how substantial such modifications would need to be (i.e. a request for a second round of peer review with a new implementation).
Note that my personal opinion based on post-review reflection is written in [square bracketed italics].
· Many seemed to feel that a boost::type_info´s member function API, where identically named to that of std::type_info member function API, ought to have identical compile-time effects. This implied that missing member function APIs, or other compile time errors for missing or incomplete functionality, would be okay but if name() returns some const char *, then any const cha * so long as it meets the C++ standard is okay.
[I think there are three use cases for a type_info like substitute: (i) as a lightest possible weight, non-RTTI requiring minimal type_info which can have any API it likes and ought to not be
Looks like the HTML formatting got mangled by the list bot gods, so the below is not particularly readable. You can find a PDF version at https://drive.google.com/file/d/0B5QDPUNHLpKMakQ2TXVGcHNkZ1E Niall On 24 Nov 2013 at 17:29, Niall Douglas wrote: directly substitutable for std::type_info or std::type_index (ii) as a compile-time substitute for std::type_info in that it will compile without third party code modification, but may not produce reliable code due to not returning exactly the same values as std::type_info (iii) as a runtime substitute for std::type_info in that it will require code to be oeported to it due to having a breaking API, but thereafter produces reliable code. ITMll be very blunt in saying that (ii) is a bad idea in my opinion, and (iii) is valuable but out of scope enhancement which could be added as a separate class after peer review. In my opinion, (i) is where we ought to focus, and I would recommend that the lightest possible weight type_info replacement be deliberately compile-time incompatible with std::type_info (where output is not identical to std::type_info) to force porting code to it so authors are not lazy.]
· A large minority seemed to feel that a boost::type_info´s member function API, where identically named to that of std::type_info member function API, ought to have identical run-time outcomes. Most specifically, this would mean that member functions e.g. name() would return exactly what std::type_info´s name() does, even if for example name() returns a std::string instead of a const car * as would be necessary in pre-C++11 compilers.
[This is really use case (iii) in the previous item.]
· It was mentioned that it should be possible to optionally specialise std::hash<> for type_info for optional seamless use in hash taking containers.
· Some felt that implementation-specific std::type_info quirks ought to be replicated. Others felt they should not, and a portable interface which works identically on all platforms is preferred.
[This is really use case (iii) in the previous item.]
· boost::type_info is currently initialised via a static cast from a std::type_info when RTTI is available. This is undefined behaviour, and some felt this to be a showstopper. I (Niall Douglas) looked into this more deeply due to its seriousness, and found that because most STL implementations define std::type_info with a virtual function table, the undefined behaviour static cast would cause dynamic_cast on a boost::type_info instance to misoperate.
[I believe the RTTI induced misoperation to indeed be a showstopper, and that this use of undefined behaviour to reduce code bloat is an unsafe optimisation. A much lighter weight non-direct-substitute for std::type_info ought to be as code bloat parsimonious as is possible, while its lack of non-explicit interoperation ability with std::type_info ought to allow avoidance of needing any UB tricks.]
· Some felt that any use of implied boost::type_info ought to be explicitly written as boost::type_info instead of relying on namespace lookup, as the name similarity to std::type_info may introduce confusion.
[I agree]
· It was mentioned that some function-local static data members are initialised in a thread unsafe way.
[For information I have to hand a very lightweight BOOST_BEGIN_MEMORY_TRANSACTION() implementation ideal for this purpose. We could submit it to Boost.Detail and then everyone could use it?]
· Magic macros such as __PRETTY_FUNCTION__ ought to not be referenced outside of a function as they don´t technically exist there. Checks for their existence ought to be within a function.
· It was requested that a boost::type_id<> which takes some unknown variable instance as a parameter and therefore allows the compiler to deduce the type of the variable as the boost::type_index<T> type would be valuable.
· A mechanism for programming compile-time logic with class inheritance trees such that inheritance can be determined at compile-time with RTTI disabled was requested.
My recommendation to the library author and the Review Wizards:
· In my opinion Antony ought to make TypeIndex v3 quite literally a very lightweight container of some unknown, but known to be uniquely identifying for some type, static const char * string. I think its class type and its list of member functions ought to be deliberately compile-time incompatible with std::type_info to force authors to upgrade their code. A conversion member function ought to be able to synthesise a corresponding std::type_info using typeid() from some boost::type_index<T>, but thatTMs about it. I would even, personally speaking, go so far as to only provide a boost::type_index and no corresponding boost::type_info, especially if the boost::type_id<T>() function can return a const boost::type_index<T>& and therefore can be used as a static const lref, or copy constructed from it etc.
A suggested name() member function replacement which correctly breaks out the multiple confounding uses of std::type_info::name() into each of their three use cases (and which intentionally causes any use of name() to fail to compile) might be:
Text Box: /*! Returns a static const char string of unknown format
uniquelyidentifying this type.The only guarantee is that this string will beunique 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 isMSVC, //!< The Microsoft C++ mangling formatItanium //!< 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() (orraw_name() on MSVC).This function may throw an exception if it does notsupport mangled type string calculation, including when mangling=Native onsome
Bear in mind that user code can always subclass boost::type_index and add their own name() implementation based on one or more of the above new member functions.
· In other words, here I think "less is more" in every way you look at it. I think a slimmer less heavy TypeIndex would be a superior solution.
· I request the Review Wizards to allow Antony some time to make the changes to produce a v3 of the library, and then to allow TypeIndex v3 a Fast Track Review of five days to ensure the Boost community is happy and that its feedback has been incorporated. It was asked during peer review that such a five day period please include a weekend. I would be happy to serve as review manager again if requested.
Niall Douglas Waterloo, Canada, November 2013
-- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
I would also like to see a performance page in the documentation, comparing RTTI implementations performances with this library performances.
On 25 Nov 2013 at 0:20, Klaim - Joël Lamotte wrote:
I would also like to see a performance page in the documentation, comparing RTTI implementations performances with this library performances.
There is a page in the docs about performance, so do you mean you want to see *empirical* performance results? Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
On Mon, Nov 25, 2013 at 1:31 AM, Niall Douglas
There is a page in the docs about performance, so do you mean you want to see *empirical* performance results?
Yes sorry, I meant a comparison of performance.
On Sunday 24 November 2013 17:29:52 Niall Douglas wrote:
· It was mentioned that some function-local static data members are initialised in a thread unsafe way.
[For information I have to hand a very lightweight BOOST_BEGIN_MEMORY_TRANSACTION() implementation ideal for this purpose. We could submit it to Boost.Detail and then everyone could use it?]
There is call_once in Boost.Thread and there will be once blocks and call_once in Boost.Sync. Do we need another reimplementation?
My recommendation to the library author and the Review Wizards:
· In my opinion Antony ought to make TypeIndex v3 quite literally a very lightweight container of some unknown, but known to be uniquely identifying for some type, static const char * string. I think its class type and its list of member functions ought to be deliberately compile-time incompatible with std::type_info to force authors to upgrade their code. A conversion member function ought to be able to synthesise a corresponding std::type_info using typeid() from some boost::type_index<T>, but that’s about it. I would even, personally speaking, go so far as to only provide a boost::type_index and no corresponding boost::type_info, especially if the boost::type_id<T>() function can return a const boost::type_index<T>& and therefore can be used as a static const lref, or copy constructed from it etc.
That's an unexpected turn. If boost::type_id<T>() returns a reference to a non-copyable object then it is useless for me because the essential advantage of type_index is its value semantics. And in such a design boost::type_index has very different semantics from std::type_index, so it shouldn't be named as such to avoid confusion. I would vote against accepting such design. If you are willing to follow that design then I'd like to ask you to rename the library and the type_index type to avoid the confusion and move all types and functions into the specific library namespace.
A suggested name() member function replacement which correctly breaks out the multiple confounding uses of std::type_info::name() into each of their three use cases (and which intentionally causes any use of name() to fail to compile) might be:
Text Box: /*! Returns a static const char string of unknown format uniquelyidentifying this type.The only guarantee is that this string will beunique 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 isMSVC, //!< The Microsoft C++ mangling formatItanium //!< 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() (orraw_name() on MSVC).This function may throw an exception if it does notsupport mangled type string calculation, including when mangling=Native onsome
Bear in mind that user code can always subclass boost::type_index and add their own name() implementation based on one or more of the above new member functions.
I don't think that mangled_name() (the last function that returns the cached mangled name) is a good idea. Requiring reimplementation of some particular mangling schemes (and by the way, why these specific ones?) is an unnecessary burden on the author (and maintainer later). Caching the name internally as a static member complicates the design (the cache has to be thread-safe and be accessible in global constructors/destructors). Caching the name as a regular member is unacceptable bloat (people expect type_index to be as light as a pointer to type_info; no dynamic memory allocation and associated possible exceptions are allowed). I stand by the three functions we discussed during the review: // Returns std::type_info::name() const char* name() const noexcept; // Returns some low-level (possibly mangled) name const char* raw_name() const noexcept; // Returns some possibly human readable name std::string pretty_name() const; I'll add that I still think that the first one is required (at least, for compatibility with std::type_index) and has ho have exactly that semantics.
On 25/11/2013 16:01, Quoth Andrey Semashev:
· In my opinion Antony ought to make TypeIndex v3 quite literally a very lightweight container of some unknown, but known to be uniquely identifying for some type, static const char * string. I think its class type and its list of member functions ought to be deliberately compile-time incompatible with std::type_info to force authors to upgrade their code. A conversion member function ought to be able to synthesise a corresponding std::type_info using typeid() from some boost::type_index<T>, but that’s about it. I would even, personally speaking, go so far as to only provide a boost::type_index and no corresponding boost::type_info, especially if the boost::type_id<T>() function can return a const boost::type_index<T>& and therefore can be used as a static const lref, or copy constructed from it etc.
That's an unexpected turn. If boost::type_id<T>() returns a reference to a non-copyable object then it is useless for me because the essential advantage of type_index is its value semantics. And in such a design boost::type_index has very different semantics from std::type_index, so it shouldn't be named as such to avoid confusion. I would vote against accepting such design.
If you are willing to follow that design then I'd like to ask you to rename the library and the type_index type to avoid the confusion and move all types and functions into the specific library namespace.
I'm fairly sure what Niall was proposing was a copyable object -- ie. basically the same semantics as the current proposed boost::type_index but storing data directly within rather than via indirection to boost::type_info, which would no longer exist. That sounds like a reasonable design to me. (It does mean code using the address-of-type_info as a key would have to change to use value-of-type_index instead, but that's probably safer anyway.) Although:
Caching the name internally as a static member complicates the design (the cache has to be thread-safe and be accessible in global constructors/destructors). Caching the name as a regular member is unacceptable bloat (people expect type_index to be as light as a pointer to type_info; no dynamic memory allocation and associated possible exceptions are allowed).
That does raise a good point.
On Monday 25 November 2013 17:13:43 Gavin Lambert wrote:
On 25/11/2013 16:01, Quoth Andrey Semashev:
· In my opinion Antony ought to make TypeIndex v3 quite literally a very lightweight container of some unknown, but known to be uniquely identifying for some type, static const char * string. I think its class type and its list of member functions ought to be deliberately compile-time incompatible with std::type_info to force authors to upgrade their code. A conversion member function ought to be able to synthesise a corresponding std::type_info using typeid() from some boost::type_index<T>, but that’s about it. I would even, personally speaking, go so far as to only provide a boost::type_index and no corresponding boost::type_info, especially if the boost::type_id<T>() function can return a const boost::type_index<T>& and therefore can be used as a static const lref, or copy constructed from it etc.
That's an unexpected turn. If boost::type_id<T>() returns a reference to a non-copyable object then it is useless for me because the essential advantage of type_index is its value semantics. And in such a design boost::type_index has very different semantics from std::type_index, so it shouldn't be named as such to avoid confusion. I would vote against accepting such design.
If you are willing to follow that design then I'd like to ask you to rename the library and the type_index type to avoid the confusion and move all types and functions into the specific library namespace.
I'm fairly sure what Niall was proposing was a copyable object -- ie. basically the same semantics as the current proposed boost::type_index but storing data directly within rather than via indirection to boost::type_info, which would no longer exist.
If so, that implies that copying type_index would be inefficient. I don't think this is a reasonable tradeoff. After all, type names is just one feature. In the motivating use case (a key in an associative container) type names are not needed and the proposed design would result in considerable overhead.
On 25/11/2013 18:02, Quoth Andrey Semashev:
If so, that implies that copying type_index would be inefficient. I don't think this is a reasonable tradeoff. After all, type names is just one feature. In the motivating use case (a key in an associative container) type names are not needed and the proposed design would result in considerable overhead.
Either way it'd just be a single pointer. Either a pointer to the std::type_info if RTTI is enabled or a const char * if RTTI is disabled (see template_info [1], probably just renamed to template_index). This is the same as std::type_index. The no-RTTI case does use full somewhat-human-readable names under the covers though, so the more types it is used with the more code space will be consumed by static strings (especially since each one incurs a "boost::detail::" namespace prefix penalty, along with some less avoidable odds and ends). [2] But this doesn't impact runtime performance, just binary size. (ie. the main change from the currently proposed code would be just dropping boost::type_info, and letting boost::type_index point to a std::type_info by containment instead of through inheritance, and retaining the parallel template_index -- or possibly keeping ONLY template_index and dropping RTTI compatibility.) Or at least I think that's what we're discussing. Sometimes it's hard to keep track. :) Regarding type names, in the proposed design (both before and next) they were essential as it does comparisons on the name string as well as the pointer value. This is a potential pessimisation from std::type_index that I'm still a little dubious about (though it should have minimal impact when searching a hashed container), but as Niall pointed out it does enable scenarios where type information has to be used across DLL/SO boundaries, as in plugins. [1] https://github.com/apolukhin/type_index/blob/master/boost/type_index/templat... [2] http://apolukhin.github.io/type_index/boost_typeindex/code_bloat.html
On Monday 25 November 2013 20:12:59 Gavin Lambert wrote:
On 25/11/2013 18:02, Quoth Andrey Semashev:
If so, that implies that copying type_index would be inefficient. I don't think this is a reasonable tradeoff. After all, type names is just one feature. In the motivating use case (a key in an associative container) type names are not needed and the proposed design would result in considerable overhead.
Either way it'd just be a single pointer. Either a pointer to the std::type_info if RTTI is enabled or a const char * if RTTI is disabled (see template_info [1], probably just renamed to template_index). This is the same as std::type_index.
I suppose it all depends on the implementation details. Niall has already stated that his idea was to make type_index an empty template class, which makes the copying overhead non-existent. I have to say though that this would not be the library I'm interested in. If type_index is not a template though, one possible implementation is to store a single pointer to the global descriptor that has a reference to std::type_info and the string mangled_name() returns. This way copying costs are minimized but other operations involving std::type_info become slightly more expensive due to an additional level of indirection. That leaves the descriptor lifetime and thread safety the only significant problem. Another way to implement it is to make the string returned by mangled_name() a member of type_index (in addition to the pointer to std::type_info), in which case all the problems with overhead and exceptions are in place. What I'm trying to say is that neither of these solutions appeal to me. What I want is a simple wrapper around std::type_info with value semantics and probably some type name helpers as long as they don't bring additional problems to the design, like the ones I mentioned. mangled_name() is a double evil in my view, because in addition to the problems it causes it also requires us to implement mangling schemes (I see no point in this function if it simply throws on some platforms). Even the no-RTTI case I consider as secondary, although it is perfectly supported by the design where (non- template) type_index refers to a type name string or a global object that contains it.
Regarding type names, in the proposed design (both before and next) they were essential as it does comparisons on the name string as well as the pointer value. This is a potential pessimisation from std::type_index that I'm still a little dubious about (though it should have minimal impact when searching a hashed container), but as Niall pointed out it does enable scenarios where type information has to be used across DLL/SO boundaries, as in plugins.
Type names are only essential to work around bugs on some platforms (i.e. inability to compare type_infos across module boundaries) or when native RTTI is disabled. In the normal world type names are completely optional.
On 25 Nov 2013 at 11:59, Andrey Semashev wrote:
mangled_name() is a double evil in my view, because in addition to the problems it causes it also requires us to implement mangling schemes (I see no point in this function if it simply throws on some platforms).
No one is suggesting one would use mangled_name() for anything except where you need to compare a boost::type_index<T> with a std::type_info for equality. There you can compare the strings returned by each for equality - if they are equal, they refer to the same type. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
On Monday 25 November 2013 13:46:21 Niall Douglas wrote:
On 25 Nov 2013 at 11:59, Andrey Semashev wrote:
mangled_name() is a double evil in my view, because in addition to the problems it causes it also requires us to implement mangling schemes (I see no point in this function if it simply throws on some platforms).
No one is suggesting one would use mangled_name() for anything except where you need to compare a boost::type_index<T> with a std::type_info for equality. There you can compare the strings returned by each for equality - if they are equal, they refer to the same type.
No they don't. You can never test boost::type_index and std::type_info for equivalence by comparing their names.
On 25 Nov 2013 at 17:13, Gavin Lambert wrote:
I'm fairly sure what Niall was proposing was a copyable object -- ie. basically the same semantics as the current proposed boost::type_index but storing data directly within rather than via indirection to boost::type_info, which would no longer exist.
Exactly right. Only thing missing is that I would have boost::type_index have sizeof=0 i.e. pure static data only.
That sounds like a reasonable design to me. (It does mean code using the address-of-type_info as a key would have to change to use value-of-type_index instead, but that's probably safer anyway.)
Correct. My proposed boost::type_index is completely unrelated to anything in the STL, and if you use its comparison operators it will do the right thing. Taking its address, if of the const lref returned by boost::type_id<>, will be no more unreliable that taking the address of a std::type_info.
Caching the name internally as a static member complicates the design (the cache has to be thread-safe and be accessible in global constructors/destructors). Caching the name as a regular member is unacceptable bloat (people expect type_index to be as light as a pointer to type_info; no dynamic memory allocation and associated possible exceptions are allowed).
That does raise a good point.
I've never had a problem here in my own code. Static data defaults to zero, so you do an atomic CAS to serialise construction on first use. I agree one is firing and forgetting the allocation though, and I would in my own code annotate the allocation with valgrind instrumentation to tell valgrind it's not a leak. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
On 25/11/2013 18:51, Quoth Niall Douglas:
I'm fairly sure what Niall was proposing was a copyable object -- ie. basically the same semantics as the current proposed boost::type_index but storing data directly within rather than via indirection to boost::type_info, which would no longer exist.
Exactly right. Only thing missing is that I would have boost::type_index have sizeof=0 i.e. pure static data only.
I don't see how you can do that without making it a template, which would render it useless. But I'm willing to be enlightened. ;)
On 25 Nov 2013 at 7:01, Andrey Semashev wrote:
about it. I would even, personally speaking, go so far as to only provide a boost::type_index and no corresponding boost::type_info, especially if the boost::type_id<T>() function can return a const boost::type_index<T>& and therefore can be used as a static const lref, or copy constructed from it etc.
That's an unexpected turn.
My report tries to recommend what I personally think is the quickest path to consensus. I should mention that Antony does not agree with my proposals, and as he has the most experience through being the library author I am happy to accept his judgment over mine (or anyone else's for that matter).
If boost::type_id<T>() returns a reference to a non-copyable object then it is useless for me because the essential advantage of type_index is its value semantics. And in such a design boost::type_index has very different semantics from std::type_index, so it shouldn't be named as such to avoid confusion. I would vote against accepting such design.
I'm not sure how you got this from my words. No one ever claimed that my proposed boost::type_id<T>() would return an uncopyable object - it returns a const boost::type_index<T>& to a statically constructed instance. One can use the static instance lref directly, take its address, or make a copy like std::type_index can. Under my design, a boost::type_index<T> has sizeof=0 and merely contains a statically constructed const char * shared by all instances. As it is zero sized, it is therefore POD.
Bear in mind that user code can always subclass boost::type_index and add their own name() implementation based on one or more of the above new member functions.
I don't think that mangled_name() (the last function that returns the cached mangled name) is a good idea. Requiring reimplementation of some particular mangling schemes (and by the way, why these specific ones?) is an unnecessary burden on the author (and maintainer later).
There is no need to reimplement any mangling schemes. For the MSVC and Itanium mangling schemes you simply chop the front and end off what is returned by __FUNCSIG__ or __FUNCTION__ and voila, there is your correct mangling.
Caching the name internally as a static member complicates the design (the cache has to be thread-safe and be accessible in global constructors/destructors). Caching the name as a regular member is unacceptable bloat (people expect type_index to be as light as a pointer to type_info; no dynamic memory allocation and associated possible exceptions are allowed).
unique_name() uses no dynamic memory and won't throw. That's your only guarantee. If you want more functionality, you agree to pay its price. Under what I proposed type_info permanently goes away. You get what TypeIndex provides and it's up to you if you want to interoperate with std::type_info. I proposed this because several reviewers felt that type_info implies some relation to std::type_info. I personally think that's daft, but that's not my role here - I am here to help reach consensus, so I tried to disappear anything called type_info.
I stand by the three functions we discussed during the review:
// Returns std::type_info::name() const char* name() const noexcept; // Returns some low-level (possibly mangled) name const char* raw_name() const noexcept; // Returns some possibly human readable name std::string pretty_name() const;
I'll add that I still think that the first one is required (at least, for compatibility with std::type_index) and has ho have exactly that semantics.
At least four people objected to name() returning anything different to what std::type_info::name() does. At least two of those are to my knowledge on the C++ standards committee (I could be wrong). Once again I reiterate that if you really, really want name() to be there, you are entirely free to subclass boost::type_index<T> yourself and add your own name() returning whatever is safe for your code. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
On Monday 25 November 2013 00:44:30 Niall Douglas wrote:
On 25 Nov 2013 at 7:01, Andrey Semashev wrote:
If boost::type_id<T>() returns a reference to a non-copyable object then it is useless for me because the essential advantage of type_index is its value semantics. And in such a design boost::type_index has very different semantics from std::type_index, so it shouldn't be named as such to avoid confusion. I would vote against accepting such design. I'm not sure how you got this from my words. No one ever claimed that my proposed boost::type_id<T>() would return an uncopyable object - it returns a const boost::type_index<T>& to a statically constructed instance. One can use the static instance lref directly, take its address, or make a copy like std::type_index can. Under my design, a boost::type_index<T> has sizeof=0 and merely contains a statically constructed const char * shared by all instances. As it is zero sized, it is therefore POD.
Sorry, I completely missed the part where type_index become a template. My comments were made under impression that boost::type_index is not a template and thus cannot have no members and be copyable. So now it does not erase the type and cannot be used as a key in a container. By all measure it is now a completely different beast than std::type_index, so please, change the naming. <nitpicking>And its size is not zero anyway.</nitpicking>
Bear in mind that user code can always subclass boost::type_index and add their own name() implementation based on one or more of the above new member functions.
I don't think that mangled_name() (the last function that returns the cached mangled name) is a good idea. Requiring reimplementation of some particular mangling schemes (and by the way, why these specific ones?) is an unnecessary burden on the author (and maintainer later).
There is no need to reimplement any mangling schemes. For the MSVC and Itanium mangling schemes you simply chop the front and end off what is returned by __FUNCSIG__ or __FUNCTION__ and voila, there is your correct mangling.
And what about other platforms? Throwing an exception unconditionally doesn't look like a portable solution.
Caching the name internally as a static member complicates the design (the cache has to be thread-safe and be accessible in global constructors/destructors). Caching the name as a regular member is unacceptable bloat (people expect type_index to be as light as a pointer to type_info; no dynamic memory allocation and associated possible exceptions are allowed).
unique_name() uses no dynamic memory and won't throw. That's your only guarantee. If you want more functionality, you agree to pay its price.
My concern was related to mangled_name(), which you documented as returning a reference to a cached std::string.
Under what I proposed type_info permanently goes away. You get what TypeIndex provides and it's up to you if you want to interoperate with std::type_info. I proposed this because several reviewers felt that type_info implies some relation to std::type_info. I personally think that's daft, but that's not my role here - I am here to help reach consensus, so I tried to disappear anything called type_info.
boost::type_info is not a problem by itself, as far as I understood the reviews. The problem was the hack through which it was implemented in the proposed library and the discrepancies with std::type_info. Remove these two issues and boost::type_info becomes a type quite useful for portability.
I stand by the three functions we discussed during the review:
// Returns std::type_info::name() const char* name() const noexcept; // Returns some low-level (possibly mangled) name const char* raw_name() const noexcept; // Returns some possibly human readable name std::string pretty_name() const;
I'll add that I still think that the first one is required (at least, for compatibility with std::type_index) and has ho have exactly that semantics.
At least four people objected to name() returning anything different to what std::type_info::name() does.
And that's what I suggested, isn't it?
On 25/11/2013 19:23, Quoth Andrey Semashev:
boost::type_info is not a problem by itself, as far as I understood the reviews. The problem was the hack through which it was implemented in the proposed library and the discrepancies with std::type_info. Remove these two issues and boost::type_info becomes a type quite useful for portability.
Yes, that's where I thought consensus was heading.
On 25 Nov 2013 at 10:23, Andrey Semashev wrote:
I'm not sure how you got this from my words. No one ever claimed that my proposed boost::type_id<T>() would return an uncopyable object - it returns a const boost::type_index<T>& to a statically constructed instance. One can use the static instance lref directly, take its address, or make a copy like std::type_index can. Under my design, a boost::type_index<T> has sizeof=0 and merely contains a statically constructed const char * shared by all instances. As it is zero sized, it is therefore POD.
Sorry, I completely missed the part where type_index become a template. My comments were made under impression that boost::type_index is not a template and thus cannot have no members and be copyable.
So now it does not erase the type and cannot be used as a key in a container. By all measure it is now a completely different beast than std::type_index, so please, change the naming.
I think we've got to the point where only code explains meaning, so
here is some fully working code for what I have in mind:
class static_string
{
protected:
const char *symbol;
template<class T> static const char *int_storage()
{
static const char mystorage[]=__FUNCDNAME__;
return mystorage;
}
const std::string &int_mangledstorage() const
{
static std::string
mystorage(std::string(".?AV")+std::string(symbol+16,
strlen(symbol)-16-23));
return mystorage;
}
public:
const char *unique_name() const noexcept { return symbol; }
const std::string &mangled_name() const { return
int_mangledstorage(); }
std::string pretty_name() const
{
const std::string &m=mangled_name();
char *temp=__unDNameEx(NULL, m.c_str()+1, 0, malloc, free,
0x2800);
std::string ret(temp);
free(temp);
return ret;
}
};
template<class T> class type_index : public static_string
{
public:
type_index() { symbol=static_string::int_storage<T>(); }
};
MSVC only, but it's very easy to add GCC/clang support. I tested this
using type_indexstd::string and I get:
unique_name=??$int_storage@V?$basic_string@DU?$char_traits@D@std@@V?$a
llocator@D@2@@std@@@static_string@@KAPBDXZ
mangled_name=.?AV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@
2@@std@@
pretty_name
=std::basic_string
<nitpicking>And its size is not zero anyway.</nitpicking>
Sure. I wasn't explaining myself well, as usual. I did write late last night if it's any excuse.
And what about other platforms? Throwing an exception unconditionally doesn't look like a portable solution.
If a compiler doesn't provide a magic macro for getting function signatures and its typeid() produces a compile error with RTTI off, all bets are off.
At least four people objected to name() returning anything different to what std::type_info::name() does.
And that's what I suggested, isn't it?
I had understood that you want name() to return something compliant with the C++ standard, not what std::type_info::name() precisely returns. Without RTTI and C++11, you cannot return a static const char unique string type identifier which is identical to std::type_info::name() - see my code example above where I slice the front and end off the mangling, plus prepend the correct preamble. In short, for C++98 and 03, you need dynamic memory if you want identical output. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
On Monday 25 November 2013 13:42:24 Niall Douglas wrote:
On 25 Nov 2013 at 10:23, Andrey Semashev wrote:
Sorry, I completely missed the part where type_index become a template. My comments were made under impression that boost::type_index is not a template and thus cannot have no members and be copyable.
So now it does not erase the type and cannot be used as a key in a container. By all measure it is now a completely different beast than std::type_index, so please, change the naming.
I think we've got to the point where only code explains meaning, so here is some fully working code for what I have in mind:
template<class T> class type_index : public static_string { public: type_index() { symbol=static_string::int_storage<T>(); } };
Yes, I understood you meant something like that (although your type_index derives from static_string which does contain a data member). But that type_index cannot be used as a key in std::map for instance, and therefore it is not the equivalent for std::type_index which I'm interested in. You could argue that you can use static_string in containers but what's the point of it if you can already use flyweight<string> or a string_ref with the same success. The point of type_index is not just strings but RTTI, which is in most cases more efficient than strings. I'm sorry, but the design you suggest does not seem right to me. This is not what I would have wanted from a Boost.TypeIndex library.
And what about other platforms? Throwing an exception unconditionally doesn't look like a portable solution.
If a compiler doesn't provide a magic macro for getting function signatures and its typeid() produces a compile error with RTTI off, all bets are off.
I'll remind you again that std::type_info::name() is not required to return a mangled name, and __func__ is not required either. On a perfectly compliant implementation your mangled_name() would fail and that is not acceptable, IMO. You can provide such a function as an optional extension, with the necessary macros to detect its availability in the client code, but putting it into the core interface of the library is a mistake.
At least four people objected to name() returning anything different to what std::type_info::name() does.
And that's what I suggested, isn't it?
I had understood that you want name() to return something compliant with the C++ standard, not what std::type_info::name() precisely returns.
I'm probably missing something. std::type_info::name() by definition returns something compliant with the standard, bugs aside. And I explicitly stated that boost::type_index::name() returns std::type_info::name().
Without RTTI and C++11, you cannot return a static const char unique string type identifier which is identical to std::type_info::name() - see my code example above where I slice the front and end off the mangling, plus prepend the correct preamble. In short, for C++98 and 03, you need dynamic memory if you want identical output.
First, std::type_info::name() result is not required to be unique. Second, I never suggested boost::type_index::name() to return equivalent strings with and without RTTI. In fact, I welcomed the possibility of the opposite during the review.
On 25 Nov 2013 at 23:35, Andrey Semashev wrote:
Yes, I understood you meant something like that (although your type_index derives from static_string which does contain a data member). But that type_index cannot be used as a key in std::map for instance, and therefore it is not the equivalent for std::type_index which I'm interested in.
You could argue that you can use static_string in containers but what's the point of it if you can already use flyweight<string> or a string_ref with the same success. The point of type_index is not just strings but RTTI, which is in most cases more efficient than strings.
I am still struggling to understand the merit of your argument. All type_info really is is a const char string of the mangled type (if you leave out virtual inheritance, which BTW is simply a linked list of pointers to more mangled type const char strings, so all a dynamic_cast<> does is to iterate strcmp() up/down the chain until it finds the right one). That's RTTI, no more, no less. Both GCC and MSVC internally use strcmp() on those strings when comparing type_info's, including for ordering for use in std::map<>. hash_code() I would also be fairly sure is simply a hash of the mangled type string. Therefore what I proposed is almost the same thing as RTTI, and certainly IS the same thing for use as value types inside containers. The only difference when RTTI is off is that the strings become a bit longer to compare and store, and that's it.
I'll remind you again that std::type_info::name() is not required to return a mangled name, and __func__ is not required either. On a perfectly compliant implementation your mangled_name() would fail and that is not acceptable, IMO.
Unless I misunderstood something, the previous complaints about name() was that it MUST return EXACTLY what std::type_info::name()/raw_name() does. If it can't return EXACTLY what name()/raw_name() does, it must not be there at all. As I have previously discussed more than once now, the use of name()/raw_name() to retrieve the underlying mangled string is a very commonly used idiom. Most type registries use it e.g. the one in Boost.Python because it's the only way you can compare type_info's for equivalence across DLLs. I can certainly see why it is so important that it return the exact same string, and hence why I think any name()/raw_name() not returning EXACTLY what std::type_info does is unwise. My opinion here is to simply leave it out, and let user code subclass their own derived class which implements whatever name() they prefer.
You can provide such a function as an optional extension, with the necessary macros to detect its availability in the client code, but putting it into the core interface of the library is a mistake.
No one is suggesting one would use mangled_name() for anything except where you need to compare a boost::type_index<T> with a std::type_info for equality. There you can compare the strings returned by each for equality - if they are equal, they refer to
If I understand what your argument has been so far, I would disagree with you. I think you don't understand how type_info works under the hood. the
same type.
No they don't. You can never test boost::type_index and std::type_info for equivalence by comparing their names.
You appear to think there can only ever be one const std::type_info& instance per typeid(T) for some type T. You are very, very wrong, and no compiler in recent years thinks so [1]. [1]: Many years ago GCC did assume different type_info instances must always refer to different types. My patch adding -fvisibility support to GCC 4.0 caused multiple type_info instances to be emitted for a type (one per SO), and therefore type_info comparisons since GCC 4.5 now do string comparisons of their containing mangled type string [2]. MSVC has always done a full string comparison to my knowledge. [2]: You probably want proof of this, because plenty of people don't believe me. Look at http://gcc.gnu.org/onlinedocs/libstdc++/libstdc++-api-4.5/a01071_sourc e.html around line 47. It'll explain the __GXX_MERGED_TYPEINFO_NAMES macro and it has the following sentence "We used to do inline pointer comparison by default if weak symbols are available, but even with weak symbols sometimes names are not merged when objects are loaded with RTLD_LOCAL, so now we always use strcmp by default". Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
AMDG On 11/25/2013 05:41 PM, Niall Douglas wrote:
On 25 Nov 2013 at 23:35, Andrey Semashev wrote:
I'll remind you again that std::type_info::name() is not required to return a mangled name, and __func__ is not required either. On a perfectly compliant implementation your mangled_name() would fail and that is not acceptable, IMO.
Unless I misunderstood something, the previous complaints about name() was that it MUST return EXACTLY what std::type_info::name()/raw_name() does. If it can't return EXACTLY what name()/raw_name() does, it must not be there at all.
I don't care what name() returns when std::type_info doesn't exist. I only care that the behavior be consistent, when it's possible to use both together. In Christ, Steven Watanabe
On 26/11/2013 14:41, Quoth Niall Douglas:
Both GCC and MSVC internally use strcmp() on those strings when comparing type_info's, including for ordering for use in std::map<>. hash_code() I would also be fairly sure is simply a hash of the mangled type string. Therefore what I proposed is almost the same thing as RTTI, and certainly IS the same thing for use as value types inside containers. The only difference when RTTI is off is that the strings become a bit longer to compare and store, and that's it.
The distinction is that std::type_index is intended specifically to be "the thing you use as map keys", whereas your suggested one cannot be used that way, and instead you must extract the string out of it and use that as the key instead. Technically these may not be significantly different approaches but at least for code readability purposes it's nicer to have a non-templated type_index that can be used as the key directly. Hence the indirection of using a templated free function to return a non-templated instance. It's also nicer to have the type_index as the key type when both purposes are required: you want it to use a "fast" (presumably mangled) representation when acting in the role of a key, but you also want to be able to easily obtain a "readable" (non-mangled) name for display purposes at some later time. (These two representations don't *have* to be different, eg. when RTTI is disabled and only the longer name is known; but when both forms are known to the implementation it is useful to use the appropriate one in each role.) I think Antony's proposed version is very close to this ideal; we just need to strip off or tweak some of the dodgier bits.
Unless I misunderstood something, the previous complaints about name() was that it MUST return EXACTLY what std::type_info::name()/raw_name() does. If it can't return EXACTLY what name()/raw_name() does, it must not be there at all.
Where RTTI is enabled throughout, the two must return compatible values (and ideally be directly comparable). Where RTTI is disabled throughout, the std one doesn't exist (or at least only works on a subset of types, in the case of MSVC) so boost::type_id can return whatever it likes. Where some compilation units are built with RTTI enabled and some are built with RTTI disabled, I'm less sure what the best behaviour is, but I was under the impression that the library wasn't intending to support this case anyway. ([1] reads as "don't do it", as far as I can tell.) [1] http://apolukhin.github.io/type_index/boost_typeindex/mixing_sources_with_rt... It could be a reasonable alternative to take *only* boost::template_info (as "template_index") and have that be the entirety of the library, forgoing any attempt to be compatible with std::type_info. However I think this is not the best choice, as taking advantage of RTTI when available (and it is usually available) produces superior results.
On 26 Nov 2013 at 16:09, Gavin Lambert wrote:
The distinction is that std::type_index is intended specifically to be "the thing you use as map keys", whereas your suggested one cannot be used that way, and instead you must extract the string out of it and use that as the key instead.
Well, I did have in mind that the underlying string carrying type would provide the appropriate map and hash semantics based on unique_name(). Andrey made a good point though - it's basically a string_ref. There is a part of me which thinks that subclassing string_ref with unique_name()/mangled_name()/pretty_name() etc might actually not be a terrible idea (and then subclassing that with type_index<T>).
I think Antony's proposed version is very close to this ideal; we just need to strip off or tweak some of the dodgier bits.
Sure. We've ended up straying quite far now from anything Antony will or is considering to produce. We're really musing about our personal philosophical differences in C++ design now.
Where some compilation units are built with RTTI enabled and some are built with RTTI disabled, I'm less sure what the best behaviour is, but I was under the impression that the library wasn't intending to support this case anyway. ([1] reads as "don't do it", as far as I can tell.)
No it wasn't and isn't. That idea was all mine. I have bad memories of fighting precompiled binary blobs with stupid options you see, and I can see a very valid use case being mixing up RTTI on and RTTI off code. A method for interop here would be really useful, especially as it's so cheap to implement. I agree it lacks a "business case" right now though.
It could be a reasonable alternative to take *only* boost::template_info (as "template_index") and have that be the entirety of the library, forgoing any attempt to be compatible with std::type_info. However I think this is not the best choice, as taking advantage of RTTI when available (and it is usually available) produces superior results.
Sure. I had in mind my unique_name() static initialiser would simply yank name() from std::type_info where that was available mainly because it's less code bloaty (MSVC always, GCC with RTTI on). Ok, I leave for the US for Thanksgiving in a few hours, so I probably am off list for a good while. Happy Thanksgiving every one. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
On 27/11/2013 20:07, Quoth Niall Douglas:
No it wasn't and isn't. That idea was all mine. I have bad memories of fighting precompiled binary blobs with stupid options you see, and I can see a very valid use case being mixing up RTTI on and RTTI off code. A method for interop here would be really useful, especially as it's so cheap to implement. I agree it lacks a "business case" right now though.
I can see that this *could* happen but having never run into it myself I'm not sure it's worth the pain it seems like it'd be to try to work around it. Mind you, I've yet to find a need to pass type information across DLL/SO boundaries either. Maybe I just need to get out more. :)
Sure. I had in mind my unique_name() static initialiser would simply yank name() from std::type_info where that was available mainly because it's less code bloaty (MSVC always, GCC with RTTI on).
You do need to be careful with that one. While MSVC's typeid does still exist with RTTI off, it's only half reliable. Calling it on any purely static type works fine, but calling it on a polymorphic type (anything where the declared type of the pointer/reference includes a vtable) results in UB (typically a segfault). (Though it does generate a compiler warning in this case, so it's not totally unheralded.) Where used with user-provided types (eg. boost::any) that's likely to be a recipe for Fun™. (Well, technically the call to typeid succeeds but it returns an invalid object reference, so any attempt to call name() or any other member on it will trigger UB. You might still be able to get away with comparing pointers to the type_info, though I'm not sure if that would produce the desired results anyway.)
Ok, I leave for the US for Thanksgiving in a few hours, so I probably am off list for a good while. Happy Thanksgiving every one.
Happy Thanksgiving!
On Monday 25 November 2013 20:41:49 Niall Douglas wrote:
On 25 Nov 2013 at 23:35, Andrey Semashev wrote:
Yes, I understood you meant something like that (although your type_index derives from static_string which does contain a data member). But that type_index cannot be used as a key in std::map for instance, and therefore it is not the equivalent for std::type_index which I'm interested in.
You could argue that you can use static_string in containers but what's the point of it if you can already use flyweight<string> or a string_ref with the same success. The point of type_index is not just strings but RTTI, which is in most cases more efficient than strings.
I am still struggling to understand the merit of your argument. All type_info really is is a const char string of the mangled type (if you leave out virtual inheritance, which BTW is simply a linked list of pointers to more mangled type const char strings, so all a dynamic_cast<> does is to iterate strcmp() up/down the chain until it finds the right one). That's RTTI, no more, no less.
That's your assumption of its implementation. And it is one way to implement it, indeed, but not the only one. My point is that type_infos are equivalent only if type_info::operator== returns true and not necessarily when strcmp(type_info::name(), type_info::name()) returns 0. The standard doesn't give you that guarantee.
Both GCC and MSVC internally use strcmp() on those strings when comparing type_info's, including for ordering for use in std::map<>.
GCC uses simple pointer comparison on platforms with appropriate linker support. That doesn't include Windows though.
hash_code() I would also be fairly sure is simply a hash of the mangled type string.
Again, that's just one possible implementation. Hashing just the address is also possible, for example.
Therefore what I proposed is almost the same thing as RTTI, and certainly IS the same thing for use as value types inside containers. The only difference when RTTI is off is that the strings become a bit longer to compare and store, and that's it.
Your solution has worse performance on some platforms and it also has the potential to be incorrect, if names are not unique.
I'll remind you again that std::type_info::name() is not required to return a mangled name, and __func__ is not required either. On a perfectly compliant implementation your mangled_name() would fail and that is not acceptable, IMO. Unless I misunderstood something, the previous complaints about name() was that it MUST return EXACTLY what std::type_info::name()/raw_name() does. If it can't return EXACTLY what name()/raw_name() does, it must not be there at all.
I assume, you mean when the native RTTI is disabled? That case is not normal, because the standard describes the language with RTTI, so you're free to provide the best possible alternative. But first think again alternative to what exactly. The standard only says that type_info::name() returns some string describing the type name. Anything that fits this description would be a suitable replacement.
As I have previously discussed more than once now, the use of name()/raw_name() to retrieve the underlying mangled string is a very commonly used idiom. Most type registries use it e.g. the one in Boost.Python because it's the only way you can compare type_info's for equivalence across DLLs. I can certainly see why it is so important that it return the exact same string, and hence why I think any name()/raw_name() not returning EXACTLY what std::type_info does is unwise.
The code that uses this concept is not portable and should not be advertised in Boost, IMO.
You appear to think there can only ever be one const std::type_info& instance per typeid(T) for some type T. You are very, very wrong, and no compiler in recent years thinks so [1].
[1]: Many years ago GCC did assume different type_info instances must always refer to different types. My patch adding -fvisibility support to GCC 4.0 caused multiple type_info instances to be emitted for a type (one per SO), and therefore type_info comparisons since GCC 4.5 now do string comparisons of their containing mangled type string [2]. MSVC has always done a full string comparison to my knowledge.
[2]: You probably want proof of this, because plenty of people don't believe me. Look at http://gcc.gnu.org/onlinedocs/libstdc++/libstdc++-api-4.5/a01071_sourc e.html around line 47. It'll explain the __GXX_MERGED_TYPEINFO_NAMES macro and it has the following sentence "We used to do inline pointer comparison by default if weak symbols are available, but even with weak symbols sometimes names are not merged when objects are loaded with RTLD_LOCAL, so now we always use strcmp by default".
From what I can see by the link you provided, type names can be compared either by addresses or by strcmp, depending on the configuration (the __GXX_MERGED_TYPEINFO_NAMES macro value). That only proves my point: don't assume anything about the implementation, rely on the standard only.
On 26 Nov 2013 at 7:28, Andrey Semashev wrote:
I am still struggling to understand the merit of your argument. All [snip] That's your assumption of its implementation. And it is one way to implement it, indeed, but not the only one. My point is that type_infos are equivalent only if type_info::operator== returns true and not necessarily when strcmp(type_info::name(), type_info::name()) returns 0. The standard doesn't give you that guarantee. [snip] The code that uses this concept is not portable and should not be advertised in Boost, IMO.
I think I now understand the source of this confusion - I see Boost as mostly a set of hacks around unhelpful compiler and platform deficiencies - it's basically a giant portability toolkit for me so I can write one set of code and Boost hides the evil that must go on underneath. I think you see Boost as more a staging ground for ISO standards, and hence you're thinking code ought to adhere to the standard rather than contemporary necessities of present toolset and platform exigencies. Neither interpretation is wrong. Nor is either right. But I think I get you now Andrey. Useful discussion. Happy Thanksgiving! Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/
2013/11/27 Niall Douglas
On 26 Nov 2013 at 7:28, Andrey Semashev wrote:
I am still struggling to understand the merit of your argument. All [snip] That's your assumption of its implementation. And it is one way to implement it, indeed, but not the only one. My point is that type_infos are equivalent only if type_info::operator== returns true and not necessarily when strcmp(type_info::name(), type_info::name()) returns 0. The standard doesn't give you that guarantee. [snip] The code that uses this concept is not portable and should not be advertised in Boost, IMO.
I think I now understand the source of this confusion - I see Boost as mostly a set of hacks around unhelpful compiler and platform deficiencies - it's basically a giant portability toolkit for me so I can write one set of code and Boost hides the evil that must go on underneath. I think you see Boost as more a staging ground for ISO standards, and hence you're thinking code ought to adhere to the standard rather than contemporary necessities of present toolset and platform exigencies.
I'd like to thank all the reviewers for giving advices and spending their time on review. I'll try to came up with a better version of TypeIndex soon. -- Best regards, Antony Polukhin
participants (6)
-
Andrey Semashev -
Antony Polukhin -
Gavin Lambert -
Klaim - Joël Lamotte -
Niall Douglas -
Steven Watanabe