<table><tr><td style="">dfaure added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D10341" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>I like the idea of enabling moves for KFileItem very much.</p>

<p>But here's a fun fact: your unittest passes even without the rest of the patch.</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">PASS   : KFileItemTest::testMove()</pre></div>

<p>That's because std::move() doesn't move, it only makes the argument eligible for e.g. a move ctor, but will call a copy ctor if there's no move ctor. So that test is not really testing that move works ;)<br />
That's always a bit tricky to test, I guess, because one can't really rely on the state of the moved-from object to be anything in particular. And we want =default, not to implement some counters in there.<br />
Oh well, then maybe there's no real way to unittest that moving works.</p>

<p>Anyhow, the thing I'm wondering is the following: does the rule-of-zero lead to more opportunities for optimizations by the compiler, who can see "from the outside" that the 5 special members are the default generated ones, while your patch "hides" the implementation, lowering the visibility for the compiler in the rest of the code? Well, one could just move the 5 "=default" to the header to fix that.<br />
Of course in both cases (rule-of-zero or 5*=default in the header), it means we are tied to the 5 defaults for all of KF5, for BIC reasons. But that seems rather sensible in this case (the only member will always be the d pointer, and it's unlikely we'll move away from refcounting...).<br />
And in fact the other reason against rule-of-zero is that we can't just remove the copy-ctor and operator=, that would be BIC (existing code links to it).</p>

<p>In summary: this looks good to me, I'm just wondering if inlining the 5 "=default" wouldn't be better, for optimization purposes.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D10341" rel="noreferrer">https://phabricator.kde.org/D10341</a></div></div><br /><div><strong>To: </strong>markg, dfaure, mwolff<br /><strong>Cc: </strong>ngraham, Frameworks, michaelh<br /></div>