fix(treewide): clean up move semantics#11843
fix(treewide): clean up move semantics#11843Mic92 merged 6 commits intoNixOS:masterfrom xokdvium:dev/move-fixes
Conversation
`auto &&` and `T &&` are forwarding references and can be either lvalue or rvalue references. Moving from universal references is incorrect and should not be done. Moving from integral or floating-point values is pointless and just worsens debug performance.
Moving from arguments where it should be done.
| BasicDerivation(BasicDerivation &&) = default; | ||
| BasicDerivation(const BasicDerivation &) = default; | ||
| BasicDerivation& operator=(BasicDerivation &&) = default; | ||
| BasicDerivation& operator=(const BasicDerivation &) = default; |
There was a problem hiding this comment.
If all the constructors are default, can't we just omit them?
There was a problem hiding this comment.
Not really, thats the problem. Move ctors are deleted if the virtual dtor is defined
There was a problem hiding this comment.
There was a problem hiding this comment.
Same with user-defined copy ctors for ref. A defined copy ctor deletes the move ctor/assignment operator. That's why it's better to not define anything by-hand if possible. In that case the compiler will do everything that the member types support.
|
I might have gotten a bit overzealous with the |
|
Also pushed a fix for |
Motivation
Running
clang-tidywith the following config has revealed some incorrect/inefficient usage of move semantics.These fixes are for the more egregious problems I was able to find. These pessimizations can pretty negatively affect the compiler codegen and the final performance.
Context
One more reason to enable regular
clang-tidyruns: #11839Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.