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: Antony Polukhin (antoshkka_at_[hidden])
Date: 2013-11-13 02:22:50

2013/11/13 Vicente J. Botet Escriba <vicente.botet_at_[hidden]>
<...>

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

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