New user self-registration is currently disabled. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 921 - Warning in first_aligned with custom operator
Warning in first_aligned with custom operator
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: Core - general
3.2
All Linux
: Normal Compilation Problem
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-12-17 16:49 UTC by nafur
Modified: 2014-12-19 14:44 UTC (History)
3 users (show)



Attachments
First test case. (564 bytes, text/x-c++src)
2014-12-17 16:49 UTC, nafur
no flags Details
Second test case (356 bytes, text/x-c++src)
2014-12-17 16:49 UTC, nafur
no flags Details

Description nafur 2014-12-17 16:49:08 UTC
Created attachment 510 [details]
First test case.

eigen3/Eigen/src/Core/util/Memory.h:485 reads:

return std::min<Index>( (PacketSize - (Index((size_t(array)/sizeof(Scalar))) & PacketAlignedMask)) & PacketAlignedMask, size);

We define the following operator

template<typename C>
inline C operator-(const C& lhs, const std::shared_ptr<const A>& rhs);

that may end up in the global namespace (when a user uses "using namespace")
When compiling with gcc (4.9.2), this results in a warning as the compiler is not sure about which operator to choose.

The minimal example attached to this ticket results in the following warning:

In file included from /usr/include/eigen3/Eigen/Core:256:0,
                 from /usr/include/eigen3/Eigen/Dense:1,
                 from ~/src/tests/debug/Test_EigenWarning.cpp:5:
/usr/include/eigen3/Eigen/src/Core/util/Memory.h: In instantiation of ‘Index Eigen::internal::first_aligned(const Scalar*, Index) [with Scalar = std::complex<double>; Index = long int]’:
/usr/include/eigen3/Eigen/src/Core/Assign.h:403:78:   required from ‘static void Eigen::internal::assign_impl<Derived1, Derived2, 3, 0, Version>::run(Derived1&, const Derived2&) [with Derived1 = Eigen::Matrix<std::complex<double>, -1, 1>; Derived2 = Eigen::Matrix<std::complex<double>, -1, 1>; int Version = 0]’
/usr/include/eigen3/Eigen/src/Core/Assign.h:500:111:   required from ‘Derived& Eigen::DenseBase<Derived>::lazyAssign(const Eigen::DenseBase<OtherDerived>&) [with OtherDerived = Eigen::Matrix<std::complex<double>, -1, 1>; Derived = Eigen::Matrix<std::complex<double>, -1, 1>]’
/usr/include/eigen3/Eigen/src/Core/PlainObjectBase.h:414:46:   required from ‘Derived& Eigen::PlainObjectBase<Derived>::lazyAssign(const Eigen::DenseBase<OtherDerived>&) [with OtherDerived = Eigen::Matrix<std::complex<double>, -1, 1>; Derived = Eigen::Matrix<std::complex<double>, -1, 1>]’
/usr/include/eigen3/Eigen/src/Core/Assign.h:520:123:   required from ‘static Derived& Eigen::internal::assign_selector<Derived, OtherDerived, false, false>::run(Derived&, const OtherDerived&) [with Derived = Eigen::Matrix<std::complex<double>, -1, 1>; OtherDerived = Eigen::Matrix<std::complex<double>, -1, 1>]’
/usr/include/eigen3/Eigen/src/Core/PlainObjectBase.h:621:105:   required from ‘Derived& Eigen::PlainObjectBase<Derived>::_set_noalias(const Eigen::DenseBase<OtherDerived>&) [with OtherDerived = Eigen::Matrix<std::complex<double>, -1, 1>; Derived = Eigen::Matrix<std::complex<double>, -1, 1>]’
/usr/include/eigen3/Eigen/src/Core/Matrix.h:288:31:   required from ‘Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>::Matrix(const Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>&) [with _Scalar = std::complex<double>; int _Rows = -1; int _Cols = 1; int _Options = 0; int _MaxRows = -1; int _MaxCols = 1]’
~/src/tests/debug/Test_EigenWarning.cpp:17:47:   required from here
/usr/include/eigen3/Eigen/src/Core/util/Memory.h:485:41: note: candidate 1: operator-(int, long int) <built-in>
     return std::min<Index>( (PacketSize - (Index((size_t(array)/sizeof(Scalar))) & PacketAlignedMask))
                                         ^
~/src/tests/debug/Test_EigenWarning.cpp:10:3: note: candidate 2: C operator-(const C&, const std::shared_ptr<const A>&) [with C = Eigen::internal::first_aligned(const Scalar*, Index) [with Scalar = std::complex<double>; Index = long int]::<anonymous enum>]
 C operator-(const C& c, const std::shared_ptr<const A>& m) {

In the second attachment, I copied the essential code from Memory.h to trigger a smaller warning:

~/src/tests/debug/Test_EigenWarning2.cpp: In function ‘int main()’:
~/src/tests/debug/Test_EigenWarning2.cpp:17:53: warning: ISO C++ says that these are ambiguous, even though the worst conversion for the first is better than the worst conversion for the second:
  std::cout << (PacketSize - (tmp & PacketAlignedMask)) << std::endl;
                                                     ^
~/src/tests/debug/Test_EigenWarning2.cpp:17:53: note: candidate 1: operator-(int, long int) <built-in>
~/src/tests/debug/Test_EigenWarning2.cpp:10:3: note: candidate 2: C operator-(const C&, const std::shared_ptr<const A>&) [with C = main()::<anonymous enum>]
 C operator-(const C& c, const std::shared_ptr<const A>& m) {

While I have no idea why gcc even thinks about converting the integer type to a std::shared_ptr, casting the first argument to an int resolves this problem for me.
Comment 1 nafur 2014-12-17 16:49:26 UTC
Created attachment 511 [details]
Second test case
Comment 2 nafur 2014-12-17 16:52:16 UTC
Basically the same problem occurs at eigen3/Eigen/src/Core/Assign.h:208 and eigen3/Eigen/src/Core/Redux.h:88 (with operator+)
Comment 3 Christoph Hertzberg 2014-12-17 18:53:14 UTC
(In reply to nafur from comment #0)
> While I have no idea why gcc even thinks about converting the integer type
> to a std::shared_ptr, casting the first argument to an int resolves this
> problem for me.

PacketAlignedMask is 0, so perhaps gcc 'optimizes' (tmp & PacketAlignedMask) to a nullptr, and there is an implicit constructor from nullptr_t to shared_ptr.
IMO, overloading operator-(const C&, shared_ptr<>) for a templated C does not appear overly sane in general, mostly because you will likely get problems whenever someone calculates (anything-0), unless anything is an int.

We could circumvent that problem by using static const Index instead of anonymous enums, at least for local variables. (See this discussion, why we can't do that in general: http://thread.gmane.org/gmane.comp.lib.eigen/4960)
Comment 4 nafur 2014-12-18 10:19:25 UTC
Thanks for the insight on why gcc implicitly converts this to a nullptr.

I agree that such an operator is somewhat unsafe, at least if any int constant that happens to be zero is potentially converted to a shared_ptr.
I have guarded our operator with enable_if to accept only types that we consider appropriate and thereby exclude your anonymous enums.

Still, I'd suggest to use an explicit cast to int, i.e.:

return std::min<Index>( ((int)PacketSize - (Index((size_t(array)/sizeof(Scalar))) & PacketAlignedMask)) & PacketAlignedMask, size);

Note You need to log in before you can comment on or make changes to this bug.