<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jun 17, 2014 at 2:44 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=dblaikie@gmail.com&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">(things might be easier to review in Phab, but I'm not sure how easy<br>

it is to manage a series of dependent patches is there)<br></blockquote><div><br></div><div>Yup, that it's a series of dependent patches instead of one enormous patch is why I decided this wasn't the time for me to try out Phabricator.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
TypoCorrectionConsumer's ctor:<br>
<br>
"std::unique_ptr<CorrectionCandidateCallback> &CCC," - looks like<br>
you're actually trying to pass ownership here. Pass the unique_ptr by<br>
value to represent this contract.<br>
<br>
CorrectTypo function:<br>
<br>
"std::unique_ptr<CorrectionCandidateCallback> &&CCC," - also passing<br>
ownership. Passing rvalue ref is certainly more helpful than passing<br>
by const ref, but passing by value is potentially even better. When<br>
passing by value it's unambiguous that the caller will take ownership<br>
and that the callee's unique_ptr will be null. With an rvalue<br>
reference it's still possible for a caller to not take ownership, or<br>
to put some other value in the unique_ptr that the callee is meant to<br>
use... a bad idea, but possible.<br>
<br>
Same for ClassifyName's "CCC" parameter.<br></blockquote><div><br></div><div>The unique_ptr reference (both & and &&) parameters were because I was getting multiple compile errors. I just went through and made all of the cases pass-by-value and with the current version of the patches and only got one compile error, which was solved with a std::move(). I actually meant to try to attempt switching the unique_ptrs back to pass-by-value before mailing the patches since the patches have been changed considerably since I originally saw the errors (oh the dangers of a long weekend! lol).</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
<br>
<br>
2014-06-17 14:14 GMT-07:00 Kaelyn Takata <<a href="mailto:rikka@google.com" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=rikka@google.com&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">rikka@google.com</a>>:<br>

<div><div class="h5">> ---<br>
>  include/clang/Parse/Parser.h      |  5 ++--<br>
>  include/clang/Sema/Sema.h         | 27 +++++++++----------<br>
>  include/clang/Sema/SemaInternal.h | 18 ++++++-------<br>
>  lib/Parse/ParseExpr.cpp           | 10 +++----<br>
>  lib/Parse/ParseStmt.cpp           |  6 ++---<br>
>  lib/Parse/ParseTentative.cpp      |  8 +++---<br>
>  lib/Parse/Parser.cpp              |  8 +++---<br>
>  lib/Sema/SemaCXXScopeSpec.cpp     |  9 +++----<br>
>  lib/Sema/SemaDecl.cpp             | 57 +++++++++++++++++++--------------------<br>
>  lib/Sema/SemaDeclCXX.cpp          | 27 ++++++++++---------<br>
>  lib/Sema/SemaDeclObjC.cpp         | 21 +++++++--------<br>
>  lib/Sema/SemaExpr.cpp             | 35 ++++++++++++------------<br>
>  lib/Sema/SemaExprMember.cpp       | 15 +++++------<br>
>  lib/Sema/SemaExprObjC.cpp         | 18 ++++++-------<br>
>  lib/Sema/SemaInit.cpp             |  4 +--<br>
>  lib/Sema/SemaLambda.cpp           |  4 +--<br>
>  lib/Sema/SemaLookup.cpp           | 21 ++++++++-------<br>
>  lib/Sema/SemaOpenMP.cpp           |  7 +++--<br>
>  lib/Sema/SemaOverload.cpp         | 23 +++++++++-------<br>
>  lib/Sema/SemaTemplate.cpp         | 17 ++++++------<br>
>  lib/Sema/SemaTemplateVariadic.cpp |  8 +++---<br>
>  21 files changed, 175 insertions(+), 173 deletions(-)<br>
><br>
><br>
</div></div>> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=cfe-commits@cs.uiuc.edu&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">cfe-commits@cs.uiuc.edu</a><br>

> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
</blockquote></div><br></div></div>