Looking at return types like (from http://eigen.tuxfamily.org/dox/classEigen_1_1MatrixBase.html#a660200abaf1fc4b888330a37d6132b76) internal::cast_return_type<Derived,const CwiseUnaryOp<internal::scalar_cast_op<typename internal::traits<Derived>::Scalar, NewType>, const Derived> >::type is more likely to confuse the average user. We could hide these into \internal typedefs/helper structs like these: //! \internal Return type of cast() template<typename NewType> struct CastReturnType { typedef /* complicated stuff here */ type; }; //! cast function: template<typename NewType> CastReturnType<NewType>::type cast() const; Actually, we already do that for many functions. An additional advantage of this would be to simplify the storage of unevaluated expressions without needing the C++11 auto keyword. Fixing this is mostly copy-and-paste and does not require deep understanding of Eigen's internals --> JuniorJob
Created attachment 472 [details] Patch for fixing return types in CwiseUnaryOps
While creating this patch a few questions came up: Why is in some cases the internal type trait for SCalar gives and sometypes the Scalar type? In one case even header and body differ in that regard. Is there consistent naming pattern which is documented somewhere? I tried to be as close as possible to whatever was already there. Also this patch is still untested because I expect that some changes will be necessary as soon I got some answers to above questions.
Comment on attachment 472 [details] Patch for fixing return types in CwiseUnaryOps Review of attachment 472 [details]: ----------------------------------------------------------------- Very good starting point. Of course the same will be needed for the CwiseBinaryOps ::: Eigen/src/plugins/CommonCwiseUnaryOps.h @@ +43,5 @@ > typedef CwiseUnaryView<internal::scalar_imag_ref_op<Scalar>, Derived> NonConstImagReturnType; > +/** \internal the return type of cast() */ > +typedef typename internal::cast_return_type<Derived, > + const CwiseUnaryOp<internal::scalar_cast_op<typename internal::traits<Derived>::Scalar, NewType>, const Derived> > + >::type CastReturnType; This will not work, because CastReturnType depends on NewType. The only C++03 compatible way is an internal helper struct similar to FixedSegmentReturnType (and others) in "BlockMethods.h" I'm not sure if the uppercase typedef (...) Type; is intended (maybe because it is, as it can be considered part of the interface), but I suggest to stay consistent with that.
(In reply to comment #2) > Why is in some cases the internal type trait for SCalar gives and sometypes the > Scalar type? In some cases it is not possible to use Scalar directly, because it is only defined inside the class. But if the implementation is inside the class, I don't think using Scalar directly should be a problem. Maybe in some cases things were moved from out-of-class definition to in-class definition, but the return type was not changed. > In one case even header and body differ in that regard. You can actually replace all occurrences inside the body by your new typedefs. This will even reduce code duplication. > Is there consistent naming pattern which is documented somewhere? I doubt anything regarding that is documented. I guess ${operation_as_noun}ReturnType is commonly used.
Partially, this has been fixed by this pull request: https://bitbucket.org/eigen/eigen/pull-request/86/ There is still room for discussion, how to name the typedefs, and where and how they should be visible in the doxygen documentation.
I did some experiments with macros and Doxygen, it seems to be perfectly possible to avoid a lot of boiler-plate code by things like this (simplified example): #define EIGEN_GENERATE_UNARY(NAME, RETURN_TYPE, OPERATOR, DOCUMENTATION) \ /** \typename RETURN_TYPE The return type of NAME(). */ \ typedef CwiseUnaryOp<internal::OPERATOR> RETURN_TYPE; \ /** \returns an expression of the coefficient-wise DOCUMENTATION of \c *this */ \ inline RETURN_TYPE NAME() const { \ return RETURN_TYPE(derived()); \ } Which could then be used like: EIGEN_GENERATE_UNARY(Abs2, Abs2ReturnType, scalar_abs2_op, squared absolute value) And Doxygen happily inserts macro parameters inside the comment. The only unfortunate problem is that commas are not usable inside macro parameters, at least not without further tricks. This will make even common things like \sa method1, method2, method3 harder than at the moment. However, it could be solved using an alias or a macro which generates a comma.
We could pass (\sa method1, method2, method3) as a single argument, but then without variadic macro I don't see how to strip the parentheses without knowing the number of commas in advance.
Actually, doxygen appears to accept variadic macros. That means we could do something like this: #ifdef PARSED_BY_DOXYGEN #define STRIP_PAREN(...) __VA_ARGS__ #endif #define EIGEN_G_U(NAME, RETURN_TYPE, OPERATOR, DOCU) \ EIGEN_GENERATE_UNARY(NAME, RETURN_TYPE, OPERATOR, STRIP_PAREN DOCU) which can be used as: EIGEN_G_U(abs2, Abs2ReturnType, scalar_abs2_op, (here comes documentation, which can, include, commas,!) ) This should not break C++98/03 compatibility, since the actual compiler will never remove the parentheses around the DOCU parameter. The only drawback I see with this macro approach is that it might confuse some IDEs. But I guess IDEs which are fine with our current #include "*plugin.h" inside class bodies should be able to handle this as well.
Then we're good. I'm not afraid by IDE because handling code within macro is much common than #include within class bodies and our inheritance mechanism is quite tricky too (e.g., to find that MatrixBase is base class of Matrix).
-- 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/833.