Problems like the following could be avoided by making the copy constructors of all base-classes protected: https://stackoverflow.com/questions/51958972/eigen-densebase-segmentation-fault Actually, the user should never be able to directly construct a Base class, so all constructors should be protected. I don't think this will break any valid/working code, but would expose some easy to make errors. Am I missing any valid use case of having public constructors here?
I thought that has always been the case, but indeed it looks like that in addition to the default ctor we are missing the copy-ctor.
Disabling the copy-ctor requires to implement copy-ctors for all derived classe, like: CwiseNullaryOp(const CwiseNullaryOp& other) : m_rows(other.m_rows), m_cols(other.m_cols), m_functor(other.m_functor) {} I'm afraid that non-trivial copy-ctor will reduce the opportunities for compiler optimizations. This also means more boilerplate code and more opportunities for stupid bugs.
I would probably do it only for every *Base class which usually should be empty and don't cause overhead when copying -- and which is the more likely cause of errors. Also, I guess copying, e.g., a CwiseNullaryOp would not actually cause problems (except for the usual pitfalls like referring to objects which got destructed before usage). Alternatively, we could define something like this only in c++11 mode (and have it empty otherwise): #define EIGEN_HIDE_COPY_CTOR(name) protected: name(const name&) = default;
Created attachment 908 [details] patch v1 my bad, I mistakenly put one in a private section. Here are my local changes for brief review. All unit tests are green.
Looks good to me (did not test it yet). I think DenseCoeffsBase, MapBase and PlainObjectBase could also get protected.
-- 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/1587.