[eigen] about changeset 6eb14e380

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

Hi,

changeset 6eb14e380 does:

[SNIP]
-      ei_assert(EIGEN_IMPLIES(ei_traits<Derived>::Flags&AlignedBit, (size_t(m_data) % 16 == 0))
+      ei_assert(EIGEN_IMPLIES(ei_traits<Derived>::Flags&AlignedBit, (size_t(m_data) % (sizeof(Scalar)*ei_packet_traits<Scalar>::size)) == 0)
[SNIP]
+  #ifdef EIGEN_VECTORIZE                                                                                                                                                                      
   VERIFY_RAISES_ASSERT((MQuaternionA(array3unaligned)));                                                                                                                                      
+  #endif                                                                                                                                                                                      
[SNIP]
+  #ifdef EIGEN_VECTORIZE
   VERIFY_RAISES_ASSERT((Map<VectorType,Aligned>(array3unaligned, size)))
+  #endif

I don't get it. First, the ei_assert above really wants to check 16-byte alignment. This value 16 is being replaced by a product that will only evaluate to 16 if vectorization is enabled. I guess that's why below you add the #ifdef EIGEN_VECTORIZE checks. But the unaligned-array-assert is unaffected by EIGEN_VECTORIZE. We wanted to keep it enabled even with vectorization disabled, so that people who develop on a platform without vectorization don't suddenly get crashes when they move to a platform with vectorization. So it is not good to have this VERIFY_RAISES_ASSERT only enabled when EIGEN_VECTORIZE is defined. All in all, this looks like a hack to compensate another hack.

So, I really don't understand what this changeset fixes. For me, the tests are already passing before this changeset.

Cheers,
Benoit