Re: [boost] [TypeIndex] Peer review period for library acceptance begins, ending Thurs 21st Nov

Subject: Re: [boost] [TypeIndex] Peer review period for library acceptance begins, ending Thurs 21st Nov
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2013-11-12 17:46:17

Le 12/11/13 20:34, Niall Douglas a écrit :
> What is Boost.TypeIndex?
>
> TypeIndex performs three main functions:
>
>
> Any questions about topics not in the documentation? Please do ask.
>
|
Hi,

First of all, thanks for submitting this library for review.

I've some minor typos remarks, questions and suggestions.

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

s/||std::type_index/||std::type_info

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

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

The same applies for boost::type_info.

* 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.

* 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<const D&>(); // yields true

|||* 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

* why do you need template_index it it is the same as template_info?

* What about separating NORTTI inBOOST_TYPE_INDEX_FORCE_NORTTI_COMPATIBILITY?

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

* 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.

Best,
Vicente