<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Oct 3, 2014 at 12:13 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"><div dir="ltr">MemberTypoDiags looks like it'd be very well-written as a lambda (it's just forwarding state to the operator() call). This would imply changing the underlying API from unique_ptr<TypoDiagnosticGenerator>, removing TypoDiagnosticGenerator, and using std::function<void (<span style="color:rgb(0,0,0)">const TypoCorrection &TC)> instead. The callers would then just pass in lambdas and std::function would provide the type erasure, dynamic destruction, etc.<br><br></span><pre style="color:rgb(0,0,0)">MemberExprTypoRecovery is a bit longer, but still does seem like a lambda might suite (& even if it doesn't - writing it as a functor (which it already is - but you would/could strip off the base class and the virtuality of the op()) and having the API use std::function is a bit more flexible)</pre></div></blockquote><div>Making MemberExprTypoRecovery a simple functor wouldn't work because it needs to capture state that isn't passed in when called (the four arguments to its constructor). As for TypoDiagnosticGenerator, MemberTypoDiags has the same problem of needing to capture state, and while it is short enough that writing it as a lambda that captures the needed state wouldn't be too painful, some of the subsequent TypoDiagnosticGenerator descendants both capture state and are longer / more complex than MemberTypoDiags (e.g. EmptyLookupTypoDiags in <a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140818/113266.html">http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140818/113266.html</a> is longer than MemberExprTypoRecovery and has state that goes beyond simply capturing variables).<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"><div dir="ltr"><pre style="color:rgb(0,0,0)">Otherwise this all looks /fairly/ straightforward (again, I don't have all the domain knowledge to judge this - the getDeclFromExpr and its use might use comments (and/or the added code that uses it might be pulled out into another function with a self-documenting name)). But again, some of this is probably just my unfamiliarity with the code.</pre></div></blockquote><div>getDeclFromExpr does exactly what it's name says. :) I've added the following comment to the one call to getDeclFromExpr to explain what is being done and why (and added a bit of whitespace around the code block, which had been moved to the EmitAllDiagnostics helper added in response to your comments on patch #5):</div><div><br></div><div><div> // Extract the NamedDecl from the transformed TypoExpr and add it to the</div><div> // TypoCorrection, replacing the existing decls. This ensures the right</div><div> // NamedDecl is used in diagnostics e.g. in the case where overload</div><div> // resolution was used to select one from several possible decls that</div><div> // had been stored in the TypoCorrection.</div></div><div><div> if (auto *ND = getDeclFromExpr(</div><div> Replacement.isInvalid() ? nullptr : Replacement.get()))</div><div> TC.setCorrectionDecl(ND);</div></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"><div dir="ltr"><pre style="color:rgb(0,0,0)"></pre></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 25, 2014 at 2:00 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank" 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>></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"><div dir="ltr">ping</div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 26, 2014 at 11:06 AM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank" 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>></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"><div dir="ltr">+dblaikie</div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 31, 2014 at 1:20 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank" 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>></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"><br>
This includes adding the new TypoExpr-based lazy typo correction to<br>
LookupMemberExprInRecord as an alternative to the existing eager typo<br>
correction.<br>
---<br>
lib/Sema/SemaExprCXX.cpp | 40 ++++++++++-<br>
lib/Sema/SemaExprMember.cpp | 116 ++++++++++++++++++++++++++++---<br>
test/SemaCXX/arrow-operator.cpp | 5 +-<br>
test/SemaCXX/typo-correction-delayed.cpp | 32 +++++++++<br>
test/SemaCXX/typo-correction-pt2.cpp | 2 +-<br>
test/SemaCXX/typo-correction.cpp | 10 +--<br>
6 files changed, 186 insertions(+), 19 deletions(-)<br>
create mode 100644 test/SemaCXX/typo-correction-delayed.cpp<br>
<br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>