|Summary:||CommaInitializer bug in absence of RVO|
|Product:||Eigen||Reporter:||Jitse Niesen <jitseniesen>|
|Component:||Core - expression templates||Assignee:||Nobody <eigen.nobody>|
|Severity:||major||CC:||chtz, gael.guennebaud, jacob.benoit.1, patrick.mihelich|
|Bug Depends on:|
Description Jitse Niesen 2014-03-07 16:40:34 UTC
Comment 1 Christoph Hertzberg 2014-03-07 17:31:04 UTC
Created attachment 424 [details] Proof-of-concept patch to avoid assertions if RVO is deactivated Unfortunately it seems this patch generally prohibits RVO. In C++11 mode with rvalue references it takes the short-cut via the move constructor. I accidentally worked on the 3.2 branch, but for the devel-branch it should work similarly. I'm not sure if this has an impact on Bug 716, but I guess not.
Comment 2 Patrick Mihelich 2014-03-08 02:31:12 UTC
Awesome, I just ran into this and you guys are already on top of it :). We're using -fno-elide-constructors in our coverage build due to a gcc/gcov limitation: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12076. Christoph's patch does work for me locally, with or without C++11. We are using C++11 in our build, so I'm happy to see move constructor support.
Comment 3 Gael Guennebaud 2014-03-08 09:14:00 UTC
Created attachment 425 [details] Simpler and unified workaround The previous patch does not work with clang. For some weird reasons, in the ctor from a CommaInitRef we get corrupted values. I tried to add a manual copy ctor in CommaInitRef but it is not called, If I make this copy ctor private, then it refuse to compile though. Nevertheless, I think we can apply the same trick for c++11 or c++03, simply using a const_cast... This simpler workaround is still compatible with RVO in c++03 mode.
Comment 4 Christoph Hertzberg 2014-03-08 10:35:27 UTC
(In reply to comment #3) > Nevertheless, I think we can apply the same trick for c++11 or c++03, simply > using a const_cast... This simpler workaround is still compatible with RVO in > c++03 mode. Yes, simply doing a const_cast was what I also had in mind (comment 0, [12:03]). I guess it is safe enough here, so go ahead and push that simpler patch.
Comment 5 Christoph Hertzberg 2014-03-12 14:04:32 UTC
I applied Gael's workaround and also added some comments to the constructor: https://bitbucket.org/eigen/eigen/commits/a2b4a00 https://bitbucket.org/eigen/eigen/commits/2c5d4f0
Comment 6 Nobody 2019-12-04 13:03:33 UTC
-- 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/755.