<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 11, 2014 at 4:04 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">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"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Tue, Nov 11, 2014 at 4:00 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">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"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Tue, Nov 11, 2014 at 3:58 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">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"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Tue, Nov 11, 2014 at 3:38 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">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"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Tue, Nov 11, 2014 at 3:26 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">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">Author: rikka<br>
Date: Tue Nov 11 17:26:56 2014<br>
New Revision: 221735<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=221735&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=221735&view=rev</a><br>
Log:<br>
Create two helpers for running the typo-correction tree transform.<br>
<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>
Modified:<br>
cfe/trunk/include/clang/Sema/Sema.h<br>
cfe/trunk/lib/Sema/SemaExprCXX.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Sema/Sema.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=221735&r1=221734&r2=221735&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=221735&r1=221734&r2=221735&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Sema/Sema.h (original)<br>
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Nov 11 17:26:56 2014<br>
@@ -2718,6 +2718,18 @@ public:<br>
bool EnteringContext = false,<br>
const ObjCObjectPointerType *OPT = nullptr);<br>
<br>
+ ExprResult<br>
+ CorrectDelayedTyposInExpr(Expr *E,<br>
+ llvm::function_ref<ExprResult(Expr *)> Filter =<br>
+ [](Expr *E) -> ExprResult { return E; });<br></blockquote><div><br></div></div></div><div>Not sure: is it worth pulling this trivial lambda out into a function & use it for the two sites here? (maybe "ExprResult::makeExprResult(Expr*)" ... though that seems almost silly ("make" functions feel like they should be doing template argument deduction, but that's just because that's their most common use))<br><br>(if you're keeping it as a lambda, you could write it as "[](Expr *E) { return ExprResult(E); }" if you think that's better</div></div></div></div></blockquote><div><br></div></div></div><div>Honestly, I wish I could make the default filter simply be the ExprResult constructor, since that's pretty much what it is... </div></div></div></div></blockquote><div><br></div></div></div><div>Yeah, if it were any static member/free function you could totally do this - just not the ctor... pity about that.</div><div><div><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div><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">
+<br>
+ ExprResult<br>
+ CorrectDelayedTyposInExpr(ExprResult ER,<br>
+ llvm::function_ref<ExprResult(Expr *)> Filter =<br>
+ [](Expr *E) -> ExprResult { return E; }) {<br>
+ return ER.isInvalid() ? ER : CorrectDelayedTyposInExpr(ER.get(), Filter);<br>
+ }<br>
+<br>
void diagnoseTypo(const TypoCorrection &Correction,<br>
const PartialDiagnostic &TypoDiag,<br>
bool ErrorRecovery = true);<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=221735&r1=221734&r2=221735&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=221735&r1=221734&r2=221735&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Nov 11 17:26:56 2014<br>
@@ -5919,6 +5919,7 @@ namespace {<br>
class TransformTypos : public TreeTransform<TransformTypos> {<br>
typedef TreeTransform<TransformTypos> BaseTransform;<br>
<br>
+ llvm::function_ref<ExprResult(Expr *)> ExprFilter;<br>
llvm::SmallSetVector<TypoExpr *, 2> TypoExprs;<br>
llvm::SmallDenseMap<TypoExpr *, ExprResult, 2> TransformCache;<br>
llvm::SmallDenseMap<OverloadExpr *, Expr *, 4> OverloadResolution;<br>
@@ -5987,7 +5988,8 @@ class TransformTypos : public TreeTransf<br>
}<br>
<br>
public:<br>
- TransformTypos(Sema &SemaRef) : BaseTransform(SemaRef) {}<br>
+ TransformTypos(Sema &SemaRef, llvm::function_ref<ExprResult(Expr *)> &&Filter)<br></blockquote><div><br></div></div></div><div>Pass Filter by value if you're going to copy it anyway?</div></div></div></div></blockquote><div><br></div></div></div><div>Bizarre thing, I tried this when I originally updated this patch in response to your earlier review feedback and for reasons unknown switching this from passing by rvalue-reference to passing by value caused things to break... yet just now I tried it (even with my latest in-progress changes) and it behaved fine.</div></div></div></div></blockquote></div></div><div><br>\o/... *frysquint*<br></div></div></div></div></blockquote><div><br></div></div></div><div>I need to amend my statement. It isn't the rvalue-ref vs value that breaks things... the magic is actually in the std::move--after sending that email I realized I tested the removal of "&&" but didn't change "ExprFilter(std::move(Filter))" to simply "ExprFilter(Filter)". Once I did that on a client without any other local changed, 7 unit tests had segfaults.</div></div></div></div></blockquote><div><br>Subtle bug in function_ref is subtle. Fixed in LLVM in <span style="color:rgb(0,0,0)">r221753 - you should now be able to just pass the function_refs around by value, drop the std::move - function_ref is so cheap it's silly to explicitly move it around.<br></span><br>Richard - let me know how that LLVM change looks (CC'ing you here just so you have the extra context to our earlier discussion).<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"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><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"><div class="gmail_extra"><div class="gmail_quote"><div> </div><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div><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">
+ : BaseTransform(SemaRef), ExprFilter(std::move(Filter)) {}<br>
<br>
ExprResult RebuildCallExpr(Expr *Callee, SourceLocation LParenLoc,<br>
MultiExprArg Args,<br>
@@ -6012,6 +6014,9 @@ public:<br>
res = TransformExpr(E);<br>
error = Trap.hasErrorOccurred();<br>
<br>
+ if (!(error || res.isInvalid()))<br>
+ res = ExprFilter(res.get());<br>
+<br>
// Exit if either the transform was valid or if there were no TypoExprs<br>
// to transform that still have any untried correction candidates..<br>
if (!(error || res.isInvalid()) ||<br>
@@ -6060,6 +6065,25 @@ public:<br>
};<br>
}<br>
<br>
+ExprResult Sema::CorrectDelayedTyposInExpr(<br>
+ Expr *E, llvm::function_ref<ExprResult(Expr *)> Filter) {<br>
+ // If the current evaluation context indicates there are uncorrected typos<br>
+ // and the current expression isn't guaranteed to not have typos, try to<br>
+ // resolve any TypoExpr nodes that might be in the expression.<br>
+ if (!ExprEvalContexts.empty() && ExprEvalContexts.back().NumTypos &&<br>
+ (E->isTypeDependent() || E->isValueDependent() ||<br>
+ E->isInstantiationDependent())) {<br>
+ auto TyposResolved = DelayedTypos.size();<br>
+ auto Result = TransformTypos(*this, std::move(Filter)).Transform(E);<br>
+ TyposResolved -= DelayedTypos.size();<br>
+ if (TyposResolved) {<br>
+ ExprEvalContexts.back().NumTypos -= TyposResolved;<br>
+ return Result;<br>
+ }<br>
+ }<br>
+ return E;<br>
+}<br>
+<br>
ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC,<br>
bool DiscardedValue,<br>
bool IsConstexpr,<br>
@@ -6106,21 +6130,9 @@ ExprResult Sema::ActOnFinishFullExpr(Exp<br>
return ExprError();<br>
}<br>
<br>
- // If the current evaluation context indicates there are uncorrected typos<br>
- // and the current expression isn't guaranteed to not have typos, try to<br>
- // resolve any TypoExpr nodes that might be in the expression.<br>
- if (ExprEvalContexts.back().NumTypos &&<br>
- (FullExpr.get()->isTypeDependent() ||<br>
- FullExpr.get()->isValueDependent() ||<br>
- FullExpr.get()->isInstantiationDependent())) {<br>
- auto TyposResolved = DelayedTypos.size();<br>
- FullExpr = TransformTypos(*this).Transform(FullExpr.get());<br>
- TyposResolved -= DelayedTypos.size();<br>
- if (TyposResolved)<br>
- ExprEvalContexts.back().NumTypos -= TyposResolved;<br>
- if (FullExpr.isInvalid())<br>
- return ExprError();<br>
- }<br>
+ FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());<br>
+ if (FullExpr.isInvalid())<br>
+ return ExprError();<br>
<br>
CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>