Since mid of December, I thing, several Test fail because of errors like listed below: (See e.g. http://eigen.tuxfamily.org/CDash/viewBuildError.php?buildid=4555) MSVC 2008 + MSVC 2010 Errors: #1 c:\develop\cdash\src\eigen\src/Core/Transpose.h(78) : error C2719: 'matrix': formal parameter with __declspec(align('16')) won't be aligned #2 c:\develop\cdash\src\eigen\src/Core/Block.h(122) : error C2719: 'xpr': formal parameter with __declspec(align('16')) won't be aligned probably unrelated: #3 C:\Develop\cdash\src\test\sparse_product.cpp(59) : error C2666: 'Eigen::DynamicSparseMatrix<_Scalar>::operator =' : 5 overloads have similar conversions MSVC 2010 Error: #4 C:\Program Files\Microsoft Visual Studio 9.0\VC\INCLUDE\deque(715) : error C2719: '_Val': formal parameter with __declspec(align('16')) won't be aligned c:\develop\cdash\src\eigen\src/StlSupport/StdDeque.h(98) : see reference to class template instantiation 'std::deque<_Ty,_Ax>' being compiled with [ _Ty=Eigen::internal::workaround_msvc_stl_support<Eigen::Matrix<float,4,4>>, _Ax=Eigen::aligned_allocator_indirection<Eigen::internal::workaround_msvc_stl_support<Eigen::Matrix<float,4,4>>> ]
#3 is most likely unrelated an covered by Bug 164.
Created attachment 80 [details] Should fix the alignment issue.
I attached a potential fix for the alignment issue. It is not ready to be pushed since I did not run all tests. I think also that the specializations of as_argument for Matrix and Array are not needed anymore.
I am unable to reproduce any errors on MSVC 2010 (32bit, debug|release) for o stddeque_1 o stddeque_2 o stddeque_3 o stddeque_4 o stddeque_5
(In reply to comment #3) > I attached a potential fix for the alignment issue. It is not ready to be > pushed since I did not run all tests. Yeah, for this kind of change, it's really important to run all tests. I see that you are still using nested<>... here. I seem to remember that's bad. I don't remember the precise thought process, but it was along the lines of: this as_argument business is just about only allowing the constructors, in Block and other expressions, that are legitimate given the constness of the expression type. This has nothing to do with nested<>. Also it's a pity that we don't have tests for const-correctness. As was discussed, it's hard to write tests that succeed when they fail to compile. But the stuff that you're touching here was introduced in r3632 for const correctness, bug 54. So could you please at least check that the test case in bug 54 comment 28 still fails to compile? Please test all 3 parts of it individually. (Block, Transpose, Diagonal). > > I think also that the specializations of as_argument for Matrix and Array are > not needed anymore. Indeed. It's funny, I originally tried something along the lines of what you are doing here and don't remember why I didn't settle for this.
(In reply to comment #5) > (In reply to comment #3) > > I attached a potential fix for the alignment issue. It is not ready to be > > pushed since I did not run all tests. > > Yeah, for this kind of change, it's really important to run all tests. > > I see that you are still using nested<>... here. I seem to remember that's bad. Sorry... I just realized that that's exactly what my current code is doing. Ignore this part! You've got my green light to push this, and also to remove the specializations (I confirm that with your patch they are not needed anymore) if it passes tests and doesn't introduce a const correctness regression. (See my previous comment)
I will be happy to run tests on my lame crutch. Then it will take just a little ...
I have to do some more tests tomorrow. It is not as easy as I thought and right now I am a bit tired. Benoit, your link to bug #54 was pretty essential. ;)
Benoit, I have fixed as_argument and everything works as expected. I am still not pushing it since I am now pretty much convinced that as_argument is not needed at all! I tested that too and it seems to be working. My current approach is to simply replace typename internal::as_argument<XprType>::type xpr by XprType& xpr Do you recall why you did not do this in the beginning? Is there an issue I am overseeing!?
What I proposed does not compile right away but I am prepping the repo such that this change will work correctly.
Created attachment 83 [details] This should finally fix the alignment issue while preventing the issues raised in bug #54.
Everything is fixed besides the std::deque issue though this one is already covered by bug #83 and thus I am closing this issue.
-- 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/156.