<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 31, 2014 at 1:21 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Fri, Oct 31, 2014 at 12:52 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">I believe the function_ref could also be changed to std::function now that it is allowed in LLVM, unless I'm overlooking something?</div></blockquote><div><br></div></span><div>It could, but that would add memory allocation overhead, etc - function_ref is more efficient in the cases where you only need to use it within a single full expression (as is the case when the code looks like yours "auto Result = TransformTypos(*this, std::move(Filter)).Transform(E);" - the TransformTypos object is destroyed by the end of the full expression)<br><br>I'm not sure what the right answer is here - the code as-is is correct, just a little subtle (as Twine is). I wouldn't object to std::function, but I don't object to the code as-is either - just mentioning it and wondering aloud if there might be a safer (& possibly as efficient) option.</div></div></div></div></blockquote><div><br></div><div>That makes sense. I'd done rvalue references for the function_ref because, not knowing the details, I was basically using it like std::function (not passing by value to avoid extra memory overhead, and using rvalue references instead of regular references so that e.g. lambdas could be passed directly and a default lambda could be provided for the argument). I just tried the code with the function_ref passed by value, and everything seems to work fine; since that seems to be the idiom, I shall switch this patch to doing so as well.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><span class="HOEnZb"><font color="#888888"><br><br>- David</font></span></div><div><div class="h5"><div> </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><div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 31, 2014 at 11:15 AM, 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">Looks like the standard idiom is to pass function_ref by value, rather than rvalue reference.<br><br>factoring out CorrectDelayedTyposInExpr is the sort of thing you can commit without pre-commit review (it's not a step backwards in readability, etc, for the existing code even if it's never reused for more callers - so it doesn't need to wait for the extra use-cases to justify it) & possibly leave off the filtering until /that's/ needed.<br><br>(the lifetime semantics of that function_ref will be a bit tricky if a TransformTypos is ever created as a named variable)<br><br><br><br><br><br></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Wed, Oct 29, 2014 at 12:49 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></div></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><div><br>
One takes an Expr* and the other is a simple wrapper that takes an<br>
ExprResult instead, and handles checking whether the ExprResult is<br>
invalid.<br>
<br>
Additionally, allow an optional callback that is run on the full result<br>
of the tree transform, for filtering potential corrections based on the<br>
characteristics of the resulting expression once all of the typos have<br>
been replaced.<br>
---<br>
 include/clang/Sema/Sema.h | 13 +++++++++++++<br>
 lib/Sema/SemaExprCXX.cpp  | 44 ++++++++++++++++++++++++++++----------------<br>
 2 files changed, 41 insertions(+), 16 deletions(-)<br>
<br>
<br></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank" 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>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>