Move constructors are not auto-generated for matrix types anymore

[egorodet]

Hi @redorav,
I've noticed that in one of the latest commits you've added manual implementation of the copy constructors and copy assignment operators to matrix types. As the result, C++ compiler does not generate move constructors and assignment operators automatically for these types and I received a bunch of issues from my static analysis system regarding std::move(matrix) calls and other std::move(...) calls for types that have matrix fields in Methane Kit. This can be fixed either by removing manual implementation of copy constructors and assignment operators to let C++ do the magic of auto-generating them properly or implement both copy and move constructors and assignment operators (according to rule of five). Also be sure to make move constructors and assignment operators noexcept according to standard. I have suggested to do this before in issue #40 which was fixed with removal of manual implementations. Is there any reason to keep these manual implementations? Are they different from the auto-generated ones?

[redorav]

Hi @egorodet, apologies, I meant to send you a message and life got in the way and I forgot :(

I wanted to see what your suggestions are in terms of implementing the necessary move constructor because we had this conversation before. The reason for putting back the copy constructors is I had a bug where a templated copy constructor in the swizzle class was wrong, and fixing that meant I had to have a manual implementation of the copy constructor (because the swizzle needs a non trivial copy constructor inside a union). I guess this means I need to go all the way and have move constructors, which is 100% fine by me, but I wasn't sure what the implementation needs to look like. Is it just like the copy constructor?

[redorav]

Just as an example, is it fine to implement a move constructor basically like a copy constructor (as there's no memory really allocated by the vector)?

hlslpp_inline float1& operator = (float1&& f) { vec = f.vec; return *this; }

[egorodet]

Yes, it should be fine to implement move as copy in your case, but it is also important to make both move constructor/assignment noexcept, so that types containing hlslpp vectors in members could have auto-generated noexcept move constructor/assignments as well. noexcept requirement comes from STL vector implementation specics. Here's another explanation from the static analyzer I use in my project: https://rules.sonarsource.com/cpp/RSPEC-5018

[redorav]

Thanks for that link, I can see why noexcept can be important in some cases. I am implementing them right now and I'll be committing them soon. There are some issues with some older compilers that don't support noexcept so I'll have to make some special define as well for that.

[redorav]

I have added move constructors and specified noexcept on all constructors as well. Hopefully this fixes your issues and we're back to where we were before. Thanks again for reporting and helping out.

[egorodet]

Thank you, @redorav. I've integrated latest changes of HLSL++ into MethaneKit/develop branch and Sonar Cloud static analysis did not show any new issues. It's worth to note that noexcept is usually used only for trivial move-constructors and move-assignment operators, because they are supposed to not allocate any dynamic memory and thus do not throw any exceptions, while normal constructors usually do allocate memory and may throw std::bad_alloc at least. Probably it's fine to use noexcept in your case for all constructors, since vector and matrix classes do not make any dynamic memory allocations.