<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 11, 2014 at 4:00 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"><div><div class="h5">On Tue, Nov 11, 2014 at 3:58 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: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"><div><div>On Tue, Nov 11, 2014 at 3:38 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"><div><div>On Tue, Nov 11, 2014 at 3:26 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:0 0 0 .8ex;border-left:1px #ccc 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" class="cremed">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" class="cremed">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 class="h5"><div> </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><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><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc 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" class="cremed">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>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><br></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> </div><div><div class="h5"><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><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><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc 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" 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>
</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>