On Fri, Nov 21, 2014 at 10:47 AM, Kaelyn Takata <[email protected]> wrote:
> Author: rikka > Date: Fri Nov 21 12:47:58 2014 > New Revision: 222549 > > URL: http://llvm.org/viewvc/llvm-project?rev=222549&view=rev > Log: > Use the full-Expr filter to disambiguate equidistant correction > candidates. > > Modified: > cfe/trunk/lib/Sema/SemaExprCXX.cpp > cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp > > Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=222549&r1=222548&r2=222549&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) > +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Fri Nov 21 12:47:58 2014 > @@ -5992,7 +5992,7 @@ class TransformTypos : public TreeTransf > typedef TreeTransform<TransformTypos> BaseTransform; > > llvm::function_ref<ExprResult(Expr *)> ExprFilter; > - llvm::SmallSetVector<TypoExpr *, 2> TypoExprs; > + llvm::SmallSetVector<TypoExpr *, 2> TypoExprs, AmbiguousTypoExprs; > llvm::SmallDenseMap<TypoExpr *, ExprResult, 2> TransformCache; > llvm::SmallDenseMap<OverloadExpr *, Expr *, 4> OverloadResolution; > > @@ -6059,6 +6059,15 @@ class TransformTypos : public TreeTransf > return nullptr; > } > > + ExprResult TryTransform(Expr *E) { > + Sema::SFINAETrap Trap(SemaRef); > + ExprResult Res = TransformExpr(E); > + if (Trap.hasErrorOccurred() || Res.isInvalid()) > + return ExprError(); > + > + return ExprFilter(Res.get()); > + } > + > public: > TransformTypos(Sema &SemaRef, llvm::function_ref<ExprResult(Expr *)> > Filter) > : BaseTransform(SemaRef), ExprFilter(Filter) {} > @@ -6079,30 +6088,42 @@ public: > ExprResult TransformLambdaExpr(LambdaExpr *E) { return Owned(E); } > > ExprResult Transform(Expr *E) { > - ExprResult res; > - bool error = false; > + ExprResult Res; > while (true) { > - Sema::SFINAETrap Trap(SemaRef); > - res = TransformExpr(E); > - error = Trap.hasErrorOccurred(); > - > - if (!(error || res.isInvalid())) > - res = ExprFilter(res.get()); > + Res = TryTransform(E); > > // Exit if either the transform was valid or if there were no > TypoExprs > // to transform that still have any untried correction candidates.. > - if (!(error || res.isInvalid()) || > + if (!Res.isInvalid() || > !CheckAndAdvanceTypoExprCorrectionStreams()) > break; > } > > + // Ensure none of the TypoExprs have multiple typo correction > candidates > + // with the same edit length that pass all the checks and filters. > + // TODO: Properly handle various permutations of possible corrections > when > + // there is more than one potentially ambiguous typo correction. > + while (!AmbiguousTypoExprs.empty()) { > + auto TE = AmbiguousTypoExprs.back(); > + auto Cached = TransformCache[TE]; > + AmbiguousTypoExprs.pop_back(); > + TransformCache.erase(TE); > + if (!TryTransform(E).isInvalid()) { > + SemaRef.getTypoExprState(TE).Consumer->resetCorrectionStream(); > + TransformCache.erase(TE); > + Res = ExprError(); > + break; > + } else > + TransformCache[TE] = Cached; > + } > + > // Ensure that all of the TypoExprs within the current Expr have been > found. > - if (!res.isUsable()) > + if (!Res.isUsable()) > FindTypoExprs(TypoExprs).TraverseStmt(E); > > EmitAllDiagnostics(); > > - return res; > + return Res; > } > > ExprResult TransformTypoExpr(TypoExpr *E) { > @@ -6124,21 +6145,15 @@ public: > State.RecoveryHandler(SemaRef, E, TC) : > attemptRecovery(SemaRef, *State.Consumer, TC); > if (!NE.isInvalid()) { > - // Check whether there is a second viable correction with the > same edit > - // distance--in which case do not suggest anything since both are > - // equally good candidates for correcting the typo. > - Sema::SFINAETrap LocalTrap(SemaRef); > + // Check whether there may be a second viable correction with the > same > + // edit distance; if so, remember this TypoExpr may have an > ambiguous > + // correction so it can be more thoroughly vetted later. > TypoCorrection Next; > - while ((Next = State.Consumer->peekNextCorrection()) && > - Next.getEditDistance(false) == TC.getEditDistance(false)) { > - ExprResult Res = > - State.RecoveryHandler > - ? State.RecoveryHandler(SemaRef, E, Next) > - : attemptRecovery(SemaRef, *State.Consumer, Next); > - if (!Res.isInvalid()) { > - NE = ExprError(); > - State.Consumer->getNextCorrection(); > - } > + if ((Next = State.Consumer->peekNextCorrection()) && > + Next.getEditDistance(false) == TC.getEditDistance(false)) { > + AmbiguousTypoExprs.insert(E); > + } else { > + AmbiguousTypoExprs.remove(E); > } > assert(!NE.isUnset() && > "Typo was transformed into a valid-but-null ExprResult"); > > Modified: cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp?rev=222549&r1=222548&r2=222549&view=diff > > ============================================================================== > --- cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp (original) > +++ cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp Fri Nov 21 12:47:58 > 2014 > @@ -48,3 +48,14 @@ void testNoCandidates() { > callee(xxxxxx, // expected-error-re {{use of undeclared identifier > 'xxxxxx'{{$}}}} > zzzzzz); // expected-error-re {{use of undeclared identifier > 'zzzzzz'{{$}}}} > } > + > +class string {}; > +struct Item { > + void Nest(); > + string text(); > + Item* next(); // expected-note {{'next' declared here}} > +}; > +void testExprFilter(Item *i) { > + Item *j; > + j = i->Next(); // expected-error {{no member named 'Next' in 'Item'; > did you mean 'next'?}} > +} > 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? (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)
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
