<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">On Tue, Nov 12, 2019 at 2:59 PM Logan Smith <<a href="mailto:logan.r.smith0@gmail.com">logan.r.smith0@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">Appreciate the feedback, Arthur. Bit of a tangent, but I'm curious why you'd prefer `string get() && { return move(s); }` over `string&& get() && { return move(s); }`. The only real difference I can see is that after 'your' version, the member `s` is guaranteed to be moved-from, whereas it might not be with the reference version?</div></blockquote><div><br></div><div>[cc: Nicolai Josuttis, which I probably should have done in my previous email :)]</div><div>I think I remember what Nico and I agreed on, but I'm even less sure that I remember the <i>reasons</i> we agreed on it. ;)</div><div><br></div><div>IIRC, the main rationale was a version of the "trading off safety for performance" argument.</div><div>- In the case of the const getter, when you make it return a reference, you trade off some amount of safety to avoid a copy. Avoiding a copy is a big deal, enough to tip Nico's calculus (but not mine) in favor of returning a reference by default.</div><div>- In the case of the rvalue-ref-qualified getter, when you make it return a reference, you trade off that same amount of safety, but this time you're merely avoiding a move-construction. Move-constructions are supposed to be cheap, so this benefit is not big enough to tip even Nico's calculus in favor of return-by-reference in this case.</div><div><br></div><div>Also, you're probably not even avoiding the move-construction, just delaying it. With a const getter, it's totally plausible that the caller doesn't intend to make a copy of the value but just wants to "observe" it. With an rvalue-ref-qualified getter, the caller is definitely saying "Please give me ownership of (a copy of) this value." They are definitely not going to just "observe" the value; they're going to transfer it somewhere else.</div><div><br></div><div>    std::string v;</div><div>    v = obj.get();  // copy-construct and then move-assign, or take a ref and then copy-assign? No big diff.</div><div>    v = std::move(obj).get();  // move-construct and then move-assign, or take a ref and then move-assign? No big diff.</div><div>    std::cout << obj.get();  // copy-construct, or take a ref? Big difference! This tips the calculus for the const getter case.</div><div>    std::cout << std::move(obj).get();  // Nobody writes this. But even if they did: move-construct, or take a ref? No big diff.</div><div><br></div><div>Your point about guaranteeing a moved-from state for the member `s` is also quite valid. Consider the difference between</div><div><br></div><div>    // <a href="https://godbolt.org/z/kw_52f">https://godbolt.org/z/kw_52f</a></div><div>    struct S {</div><div>        std::shared_ptr<int> p_;</div><div>        std::shared_ptr<int>&& pref() && { return std::move(p_); }<br>        std::shared_ptr<int> pval() && { return std::move(p_); }</div><div>    };<br></div><div><br></div><div>    S s;</div><div>    std::weak_ptr<int> w1 = std::move(s).pref();  // leaves s.p_ unchanged<br></div><div>    std::weak_ptr<int> w2 = std::move(s).pval();  // nulls out s.p_</div><div><br></div><div>In other words, returning-by-value usually makes the program easier to reason about. References can be sneaky (or "unsafe") in more ways than just aliasing.</div><div><br></div><div>–Arthur</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 12, 2019 at 4:33 PM Arthur O'Dwyer <<a href="mailto:arthur.j.odwyer@gmail.com" target="_blank">arthur.j.odwyer@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Tue, Nov 12, 2019 at 11:10 AM Logan Smith via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>@David there is a lot of existing machinery I've found useful, such as tidy::matchers::isExpensiveToCopy. As for the "for (X y : z)" case, that seems simpler, since the checker could analyze all the uses of `y` within the loop body and do what's most appropriate, whereas there'd be no practical way for this check to analyze all the call sites of A::getS().<br></div><div><br></div>@Noel thanks for the input. </div></blockquote><div><br></div><div>(FWIW, Noel's example with `auto x = get();` was actually fine. What wouldn't be fine would be `const auto& x = get();` or `decltype(auto) x = get();`. <a href="https://quuxplusone.github.io/blog/2018/04/05/lifetime-extension-grudgingly-accepted/" target="_blank">This blog post</a> is relevant to your interests.)</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">Here's another example of a case whose behavior is subtly broken by this idea:<br><div><br></div><div>    A globalA;</div><div>    void doSomething(const std::string&); // possibly modifies globalA</div><div>    ...</div><div>    doSomething(globalA.getS());</div><div><br></div><div>With the by-value return, the string parameter is copied and 'frozen', whereas with the reference version it could change mid-function with any modifications to globalA. </div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>This kind of stuff is feeling more and more like a nail in the coffin of this idea. I don't feel crazy about introducing a check with such subtle implications</div></div></blockquote><div><br></div><div>FWIW, I agree. I always say that C++ prefers value semantics everywhere by default: C++ likes to make implicit copies, pass by value, etc. etc. The programmer can do work to remove some of those copies — pass-by-reference, even return-by-reference — but every time you replace a copy by a reference, you're trading off safety for performance. More incidental aliasing relationships equals less safety.</div><div><br></div><div>Refactoring `void foo(string a, string b)` into `void foo(const string& a, const string& b)` is usually safe in practice, and anyway the programmer can check the O(1) amount of code inside `foo` to ensure that the transformation seems safe. The newly introduced aliasing relationship has a limited scope.</div><div>Refactoring `string bar()` into `const string& bar()` is usually "safe but confusing" in practice, and [as you already observed] there's no way for the programmer to check all O(N) call-sites to see whether the transformation seems safe. The newly introduced aliasing relationship has unbounded scope.</div><div><br></div><div>(BTW, Nico Josuttis and I had a nice discussion at CppCon 2019 about this getter-return-by-reference idiom. We ended up agreeing that if you have an rvalue-ref-qualified getter, it should rather return by [pr]value than by rvalue reference. I believe Nico would default to return-by-const-reference for regular getters; whereas I would default to return-by-value as a matter of course, until benchmarks showed that the code was "too slow and safe," such that a tradeoff of safety for performance was warranted.)</div><div><br></div><div>–Arthur</div></div></div>
</blockquote></div>
</blockquote></div></div></div></div></div>