This follows up on bug 85. In presence of pointer types T*, our constness helpers work on the value type T, whereas STL helpers don't special-case pointer types. hence: us: T* -> const T* stl: T* -> T* const This applies to remove_const and add_const. I suggest to rename our helpers to slightly different names to prevent confusion with STL. How about: add_const -> add_const_on_value_type remove_const -> remove_const_on_value_type We should then have add_const and remove_const behave exactly like in the stl.
Another bug: template<typename T> struct remove_const { typedef T type; }; template<typename T> struct remove_const<const T> { typedef T type; }; template<typename T> struct remove_const<T const &> { typedef T & type; }; template<typename T> struct remove_const<T const *> { typedef T * type; }; Here, if T = const U* const, since that's a constant type, the first specialization may be taken by the compiler, resulting in const U*, as the STL would do. But if T = const U*, we return U*, unlike the STL.
According to: http://publib.boulder.ibm.com/infocenter/comphelp/v8v101/index.jsp?topic=%2Fcom.ibm.xlcpp8a.doc%2Flanguage%2Fref%2Fpartial_specialization.htm "If the compiler finds more than one specialization, then the compiler tries to determine which of the specializations is the most specialized. A template X is more specialized than a template Y if every argument list that matches the one specified by X also matches the one specified by Y, but not the other way around. If the compiler cannot find the most specialized specialization, then the use of the class template is ambiguous; the compiler will not allow the program." In our case (remove_const), that doesn't make a difference as only the specialization for const T matches const U* const so we're really buggy. But it means that (provided it's unit tested) it should still be OK to implement these helpers by specializing for N cases as we currently do.
Created attachment 40 [details] Implements the proposed changes. The attached patch should fix this issue. I tested via #include <cassert> #include <iostream> #include <type_traits> #include <Eigen/Core> using namespace Eigen; int main() { typedef float U; static_assert(std::is_same< std::remove_const<U const*>::type, internal::remove_const<U const*>::type >::value, "remove_const"); static_assert(std::is_same< std::remove_const<const U*>::type, internal::remove_const<const U*>::type >::value, "remove_const"); static_assert(std::is_same< std::remove_const<const U*>::type, internal::remove_const<const U*>::type >::value, "remove_const"); static_assert(std::is_same< std::add_const<U>::type, internal::add_const<U>::type >::value, "add_const"); static_assert(std::is_same< std::add_const<U*>::type, internal::add_const<U*>::type >::value, "add_const"); static_assert(std::is_same< std::add_const<U&>::type, internal::add_const<U&>::type >::value, "add_const"); static_assert(std::is_same< internal::add_const_on_value_type<U&>::type, U const& >::value, "add_const_on_value_type"); static_assert(std::is_same< internal::add_const_on_value_type<U*>::type, U const* >::value, "add_const_on_value_type"); static_assert(std::is_same< internal::add_const_on_value_type<U>::type, const U >::value, "add_const_on_value_type"); static_assert(std::is_same< internal::add_const_on_value_type<const U>::type, const U >::value, "add_const_on_value_type"); static_assert(std::is_same< internal::add_const_on_value_type<U const>::type, const U >::value, "add_const_on_value_type"); static_assert(std::is_same< internal::add_const_on_value_type<const U* const>::type, const U* const>::value, "add_const_on_value_type"); static_assert(std::is_same< internal::add_const_on_value_type<U* const>::type, const U* const>::value, "add_const_on_value_type"); }
In fact, my local history also contains the renames.
Created attachment 41 [details] The complete patch.
Comment on attachment 41 [details] The complete patch. -template<typename T> struct remove_const { typedef T type; }; -template<typename T> struct remove_const<const T> { typedef T type; }; -template<typename T> struct remove_const<T const &> { typedef T & type; }; -template<typename T> struct remove_const<T const *> { typedef T * type; }; +template<typename T> struct remove_const_on_value_type { typedef T type; }; +template<typename T> struct remove_const_on_value_type<const T> { typedef T type; }; +template<typename T> struct remove_const_on_value_type<T const &> { typedef T & type; }; +template<typename T> struct remove_const_on_value_type<T const *> { typedef T * type; }; This is still buggy, right? remove_const_on_value_type<U const * const> will match the first specialization on <const T> hence will return T const *. It won't match at all the specialization on T const * because there is no type T making T const * be the same as U const * const. So: - please expand the meta.cpp unit test - may I suggest always putting const after the type, it will be far less confusing in this context.
Just an additional note, so nobody gets confused. The errors from the patch above are fixed. Actually, we have decided on IRC to remove "remove_const_on_value_type" since a) its semantics is confusing b) more importantly - we don't use it. The patch will be pushed one the tests have been run.
-- GitLab Migration Automatic Message -- This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/124.