<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Nov 21, 2014 at 10:47 AM, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rikka<br>
Date: Fri Nov 21 12:47:58 2014<br>
New Revision: 222549<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=222549&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=222549&view=rev</a><br>
Log:<br>
Use the full-Expr filter to disambiguate equidistant correction<br>
candidates.<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaExprCXX.cpp<br>
cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp<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=222549&r1=222548&r2=222549&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=222549&r1=222548&r2=222549&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Nov 21 12:47:58 2014<br>
@@ -5992,7 +5992,7 @@ class TransformTypos : public TreeTransf<br>
typedef TreeTransform<TransformTypos> BaseTransform;<br>
<br>
llvm::function_ref<ExprResult(Expr *)> ExprFilter;<br>
- llvm::SmallSetVector<TypoExpr *, 2> TypoExprs;<br>
+ llvm::SmallSetVector<TypoExpr *, 2> TypoExprs, AmbiguousTypoExprs;<br>
llvm::SmallDenseMap<TypoExpr *, ExprResult, 2> TransformCache;<br>
llvm::SmallDenseMap<OverloadExpr *, Expr *, 4> OverloadResolution;<br>
<br>
@@ -6059,6 +6059,15 @@ class TransformTypos : public TreeTransf<br>
return nullptr;<br>
}<br>
<br>
+ ExprResult TryTransform(Expr *E) {<br>
+ Sema::SFINAETrap Trap(SemaRef);<br>
+ ExprResult Res = TransformExpr(E);<br>
+ if (Trap.hasErrorOccurred() || Res.isInvalid())<br>
+ return ExprError();<br>
+<br>
+ return ExprFilter(Res.get());<br>
+ }<br>
+<br>
public:<br>
TransformTypos(Sema &SemaRef, llvm::function_ref<ExprResult(Expr *)> Filter)<br>
: BaseTransform(SemaRef), ExprFilter(Filter) {}<br>
@@ -6079,30 +6088,42 @@ public:<br>
ExprResult TransformLambdaExpr(LambdaExpr *E) { return Owned(E); }<br>
<br>
ExprResult Transform(Expr *E) {<br>
- ExprResult res;<br>
- bool error = false;<br>
+ ExprResult Res;<br>
while (true) {<br>
- Sema::SFINAETrap Trap(SemaRef);<br>
- res = TransformExpr(E);<br>
- error = Trap.hasErrorOccurred();<br>
-<br>
- if (!(error || res.isInvalid()))<br>
- res = ExprFilter(res.get());<br>
+ Res = TryTransform(E);<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>
+ if (!Res.isInvalid() ||<br>
!CheckAndAdvanceTypoExprCorrectionStreams())<br>
break;<br>
}<br>
<br>
+ // Ensure none of the TypoExprs have multiple typo correction candidates<br>
+ // with the same edit length that pass all the checks and filters.<br>
+ // TODO: Properly handle various permutations of possible corrections when<br>
+ // there is more than one potentially ambiguous typo correction.<br>
+ while (!AmbiguousTypoExprs.empty()) {<br>
+ auto TE = AmbiguousTypoExprs.back();<br>
+ auto Cached = TransformCache[TE];<br>
+ AmbiguousTypoExprs.pop_back();<br>
+ TransformCache.erase(TE);<br>
+ if (!TryTransform(E).isInvalid()) {<br>
+ SemaRef.getTypoExprState(TE).Consumer->resetCorrectionStream();<br>
+ TransformCache.erase(TE);<br>
+ Res = ExprError();<br>
+ break;<br>
+ } else<br>
+ TransformCache[TE] = Cached;<br>
+ }<br>
+<br>
// Ensure that all of the TypoExprs within the current Expr have been found.<br>
- if (!res.isUsable())<br>
+ if (!Res.isUsable())<br>
FindTypoExprs(TypoExprs).TraverseStmt(E);<br>
<br>
EmitAllDiagnostics();<br>
<br>
- return res;<br>
+ return Res;<br>
}<br>
<br>
ExprResult TransformTypoExpr(TypoExpr *E) {<br>
@@ -6124,21 +6145,15 @@ public:<br>
State.RecoveryHandler(SemaRef, E, TC) :<br>
attemptRecovery(SemaRef, *State.Consumer, TC);<br>
if (!NE.isInvalid()) {<br>
- // Check whether there is a second viable correction with the same edit<br>
- // distance--in which case do not suggest anything since both are<br>
- // equally good candidates for correcting the typo.<br>
- Sema::SFINAETrap LocalTrap(SemaRef);<br>
+ // Check whether there may be a second viable correction with the same<br>
+ // edit distance; if so, remember this TypoExpr may have an ambiguous<br>
+ // correction so it can be more thoroughly vetted later.<br>
TypoCorrection Next;<br>
- while ((Next = State.Consumer->peekNextCorrection()) &&<br>
- Next.getEditDistance(false) == TC.getEditDistance(false)) {<br>
- ExprResult Res =<br>
- State.RecoveryHandler<br>
- ? State.RecoveryHandler(SemaRef, E, Next)<br>
- : attemptRecovery(SemaRef, *State.Consumer, Next);<br>
- if (!Res.isInvalid()) {<br>
- NE = ExprError();<br>
- State.Consumer->getNextCorrection();<br>
- }<br>
+ if ((Next = State.Consumer->peekNextCorrection()) &&<br>
+ Next.getEditDistance(false) == TC.getEditDistance(false)) {<br>
+ AmbiguousTypoExprs.insert(E);<br>
+ } else {<br>
+ AmbiguousTypoExprs.remove(E);<br>
}<br>
assert(!NE.isUnset() &&<br>
"Typo was transformed into a valid-but-null ExprResult");<br>
<br>
Modified: cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp?rev=222549&r1=222548&r2=222549&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp?rev=222549&r1=222548&r2=222549&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp Fri Nov 21 12:47:58 2014<br>
@@ -48,3 +48,14 @@ void testNoCandidates() {<br>
callee(xxxxxx, // expected-error-re {{use of undeclared identifier 'xxxxxx'{{$}}}}<br>
zzzzzz); // expected-error-re {{use of undeclared identifier 'zzzzzz'{{$}}}}<br>
}<br>
+<br>
+class string {};<br>
+struct Item {<br>
+ void Nest();<br>
+ string text();<br>
+ Item* next(); // expected-note {{'next' declared here}}<br>
+};<br>
+void testExprFilter(Item *i) {<br>
+ Item *j;<br>
+ j = i->Next(); // expected-error {{no member named 'Next' in 'Item'; did you mean 'next'?}}<br>
+}<br></blockquote><div><br>I imagine this test should pass by picking the best candidate (the edit distance from Next -> next is shorter than Next->text or Next-Nest), right? If that's the case, is that not addressed (are we not picking the best candidate?)? Once it is, we might need a different test case to verify this tie handling - presumably a test where the best candidate has a tie?<br><br>(but I'm guessing your example had something else to test - since only the return type of 'next' would make this compile - the other two return types don't match - what was that about? Clearly I'm missing something here)</div></div></div></div>