Rename point_traits.h into type_traits.h

[kunaltyagi]

Can we change the name of point_traits.h to type_traits.h? Will this be a breaking change? if so, should we have a redirect to the new name?

At first sight yes, yes and yes.

Originally posted by @SergioRAgostinho in #3651 (comment)

[taketwo]

LGTM. You prefer to not merge it before 1.10.1?

[kunaltyagi]

Yes. Deprecation between minor releases doesn't sit right by me.

[aPonza]

Sorry to bother in a seemingly unrelated issue, but I think this header could be kept/subdivided. Let me explain. From #3701 (comment) I have, among the other questions, this:

• 16x "warning: Detected potential recursive class relation between class and base class " can be fixed with a /** \cond NO_WARN_RECURSIVE */-/** \endcond */ pair for all four traits (as per this SO answer) but it'd need a new header: just adding the pair leads to "warning: Conditional section with label 'NO_WARN_RECURSIVE' does not have a corresponding \endcond command within this file.". The solutions seems to be moving
  

  pcl/common/include/pcl/point_traits.h

  Lines 130 to 187 in dac8961

  	// name
  	/* This really only depends on Tag, but we go through some gymnastics to avoid ODR violations.
  	We template it on the point type PointT to avoid ODR violations when registering multiple
  	point types with shared tags.
  	The dummy parameter is so we can partially specialize name on PointT and Tag but leave it
  	templated on dummy. Each specialization declares a static char array containing the tag
  	name. The definition of the static member would conflict when linking multiple translation
  	units that include the point type registration. But when the static member definition is
  	templated (on dummy), we sidestep the ODR issue.
  	*/
  	template<class PointT, typename Tag, int dummy = 0>
  	struct name : name<typename POD<PointT>::type, Tag, dummy>
  	{
  	// Contents of specialization:
  	// static const char value[];
  	// Avoid infinite compile-time recursion
  	BOOST_MPL_ASSERT_MSG((!std::is_same<PointT, typename POD<PointT>::type>::value),
  	POINT_TYPE_NOT_PROPERLY_REGISTERED, (PointT&));
  	};
  	// offset
  	template<class PointT, typename Tag>
  	struct offset : offset<typename POD<PointT>::type, Tag>
  	{
  	// Contents of specialization:
  	// static const std::size_t value;
  	// Avoid infinite compile-time recursion
  	BOOST_MPL_ASSERT_MSG((!std::is_same<PointT, typename POD<PointT>::type>::value),
  	POINT_TYPE_NOT_PROPERLY_REGISTERED, (PointT&));
  	};
  	// datatype
  	template<class PointT, typename Tag>
  	struct datatype : datatype<typename POD<PointT>::type, Tag>
  	{
  	// Contents of specialization:
  	// using type = ...;
  	// static const std::uint8_t value;
  	// static const std::uint32_t size;
  	// Avoid infinite compile-time recursion
  	BOOST_MPL_ASSERT_MSG((!std::is_same<PointT, typename POD<PointT>::type>::value),
  	POINT_TYPE_NOT_PROPERLY_REGISTERED, (PointT&));
  	};
  	// fields
  	template<typename PointT>
  	struct fieldList : fieldList<typename POD<PointT>::type>
  	{
  	// Contents of specialization:
  	// using type = boost::mpl::vector<...>;
  	// Avoid infinite compile-time recursion
  	BOOST_MPL_ASSERT_MSG((!std::is_same<PointT, typename POD<PointT>::type>::value),
  	POINT_TYPE_NOT_PROPERLY_REGISTERED, (PointT&));
  	};

  and

  pcl/common/include/pcl/register_point_struct.h

  Lines 316 to 345 in dac8961

  	#define POINT_CLOUD_REGISTER_FIELD_NAME(r, point, elem) \
  	template<int dummy> \
  	struct name<point, pcl::fields::BOOST_PP_TUPLE_ELEM(3, 2, elem), dummy> \
  	{ \
  	static const char value[]; \
  	}; \
  	\
  	template<int dummy> \
  	const char name<point, \
  	pcl::fields::BOOST_PP_TUPLE_ELEM(3, 2, elem), \
  	dummy>::value[] = \
  	BOOST_PP_STRINGIZE(BOOST_PP_TUPLE_ELEM(3, 2, elem)); \
  	/***/
  	#define POINT_CLOUD_REGISTER_FIELD_OFFSET(r, name, elem) \
  	template<> struct offset<name, pcl::fields::BOOST_PP_TUPLE_ELEM(3, 2, elem)> \
  	{ \
  	static const std::size_t value = offsetof(name, BOOST_PP_TUPLE_ELEM(3, 1, elem)); \
  	}; \
  	/***/
  	#define POINT_CLOUD_REGISTER_FIELD_DATATYPE(r, name, elem) \
  	template<> struct datatype<name, pcl::fields::BOOST_PP_TUPLE_ELEM(3, 2, elem)> \
  	{ \
  	using type = boost::mpl::identity<BOOST_PP_TUPLE_ELEM(3, 0, elem)>::type; \
  	using decomposed = decomposeArray<type>; \
  	static const std::uint8_t value = asEnum<decomposed::type>::value; \
  	static const std::uint32_t size = decomposed::value; \
  	}; \
  	/***/

  and

  pcl/common/include/pcl/register_point_struct.h

  Lines 351 to 356 in dac8961

  	#define POINT_CLOUD_REGISTER_POINT_FIELD_LIST(name, seq) \
  	template<> struct fieldList<name> \
  	{ \
  	using type = boost::mpl::vector<BOOST_PP_SEQ_ENUM(seq)>; \
  	}; \
  	/***/

  into a new header with an appropriate name. Do I have the ok and which name would you like?

I'm thinking these bits I need from point_traits.h could be kept where they are and integrated with the register_point_struct.h bits. The rest of the point_traits.h code should then be moved to type_traits.h together with an #include <pcl/point_traits.h>.

Am I incorrectly understanding the code? Would you rather have a different header (point_struct_traits.h) or fix?

[SergioRAgostinho]

After you resolve the conflict feel free to merge.

[kunaltyagi]

I'll rebase it a bit first.