Index: lib/Sema/SemaExprCXX.cpp =================================================================== --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -7584,15 +7584,22 @@ llvm::SmallDenseMap OverloadResolution; /// Emit diagnostics for all of the TypoExprs encountered. + /// /// If the TypoExprs were successfully corrected, then the diagnostics should /// suggest the corrections. Otherwise the diagnostics will not suggest /// anything (having been passed an empty TypoCorrection). - void EmitAllDiagnostics() { + /// + /// If we've failed to correct due to ambiguous corrections, we need to + /// be sure to pass empty corrections and replacements. Otherwise it's + /// possible that the Consumer has a TypoCorrection that failed to ambiguity + /// and we don't want to report those diagnostics. + void EmitAllDiagnostics(bool IsAmbiguous) { for (TypoExpr *TE : TypoExprs) { auto &State = SemaRef.getTypoExprState(TE); if (State.DiagHandler) { - TypoCorrection TC = State.Consumer->getCurrentCorrection(); - ExprResult Replacement = TransformCache[TE]; + TypoCorrection TC = IsAmbiguous + ? TypoCorrection() : State.Consumer->getCurrentCorrection(); + ExprResult Replacement = IsAmbiguous ? ExprError() : TransformCache[TE]; // Extract the NamedDecl from the transformed TypoExpr and add it to the // TypoCorrection, replacing the existing decls. This ensures the right @@ -7654,6 +7661,115 @@ return ExprFilter(Res.get()); } + // Since correcting typos may intoduce new TypoExprs, this function + // checks for new TypoExprs and recurses if it finds any. Note that it will + // only succeed if it is able to correct all typos in the given expression. + ExprResult CheckForRecursiveTypos(ExprResult Res, bool &IsAmbiguous) { + if (Res.isInvalid()) { + return Res; + } + // Check to see if any new TypoExprs were created. If so, we need to recurse + // to check their validity. + Expr *FixedExpr = Res.get(); + + auto SavedTypoExprs = std::move(TypoExprs); + auto SavedAmbiguousTypoExprs = std::move(AmbiguousTypoExprs); + TypoExprs.clear(); + AmbiguousTypoExprs.clear(); + + FindTypoExprs(TypoExprs).TraverseStmt(FixedExpr); + if (!TypoExprs.empty()) { + // Recurse to handle newly created TypoExprs. If we're not able to + // handle them, discard these TypoExprs. + ExprResult RecurResult = + RecursiveTransformLoop(FixedExpr, IsAmbiguous); + if (RecurResult.isInvalid()) { + Res = ExprError(); + // Recursive corrections didn't work, wipe them away and don't add + // them to the TypoExprs set. + for (auto TE : TypoExprs) { + TransformCache.erase(TE); + SemaRef.clearDelayedTypo(TE); + } + } else { + // TypoExpr is valid: add newly created TypoExprs since we were + // able to correct them. + Res = RecurResult; + SavedTypoExprs.set_union(TypoExprs); + } + } + + TypoExprs = std::move(SavedTypoExprs); + AmbiguousTypoExprs = std::move(SavedAmbiguousTypoExprs); + + return Res; + } + + // Try to transform the given expression, looping through the correction + // candidates with `CheckAndAdvanceTypoExprCorrectionStreams`. + // + // If valid ambiguous typo corrections are seen, `IsAmbiguous` is set to + // true and this method immediately will return an `ExprError`. + ExprResult RecursiveTransformLoop(Expr *E, bool &IsAmbiguous) { + ExprResult Res; + while (true) { + Res = CheckForRecursiveTypos(TryTransform(E), IsAmbiguous); + + // Recursion encountered an ambiguous correction. This means that our + // correction itself is ambiguous, so stop now. + if (IsAmbiguous) + break; + + // If the transform is still valid after checking for any new typos, + // it's good to go. + if (!Res.isInvalid()) + break; + + // The transform was invalid, see if we have any TypoExprs with untried + // correction candidates. + if (!CheckAndAdvanceTypoExprCorrectionStreams()) + break; + } + + // If we found a valid result, double check to make sure it's not ambiguous. + if (!IsAmbiguous && !Res.isInvalid() && !AmbiguousTypoExprs.empty()) { + auto SavedTransformCache = std::move(TransformCache); + TransformCache.clear(); + // Ensure none of the TypoExprs have multiple typo correction candidates + // with the same edit length that pass all the checks and filters. + while (!AmbiguousTypoExprs.empty()) { + auto TE = AmbiguousTypoExprs.back(); + + // TryTransform itself can create new Typos, adding them to the TypoExpr map + // and invalidating our TypoExprState, so always fetch it instead of storing. + SemaRef.getTypoExprState(TE).Consumer->saveCurrentPosition(); + + TypoCorrection TC = SemaRef.getTypoExprState(TE).Consumer->peekNextCorrection(); + TypoCorrection Next; + do { + ExprResult AmbigRes = CheckForRecursiveTypos(TryTransform(E), IsAmbiguous); + + if (!AmbigRes.isInvalid() || IsAmbiguous) { + SemaRef.getTypoExprState(TE).Consumer->resetCorrectionStream(); + SavedTransformCache.erase(TE); + Res = ExprError(); + IsAmbiguous = true; + break; + } + } while ((Next = SemaRef.getTypoExprState(TE).Consumer->peekNextCorrection()) && + Next.getEditDistance(false) == TC.getEditDistance(false)); + + if (IsAmbiguous) + break; + + AmbiguousTypoExprs.remove(TE); + SemaRef.getTypoExprState(TE).Consumer->restoreSavedPosition(); + } + TransformCache = std::move(SavedTransformCache); + } + return Res; + } + public: TransformTypos(Sema &SemaRef, VarDecl *InitDecl, llvm::function_ref Filter) : BaseTransform(SemaRef), InitDecl(InitDecl), ExprFilter(Filter) {} @@ -7681,49 +7797,13 @@ ExprResult TransformBlockExpr(BlockExpr *E) { return Owned(E); } ExprResult Transform(Expr *E) { - ExprResult Res; - while (true) { - 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 (!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. - // Also, disable typo correction while attempting the transform when - // handling potentially ambiguous typo corrections as any new TypoExprs will - // have been introduced by the application of one of the correction - // candidates and add little to no value if corrected. - SemaRef.DisableTypoCorrection = true; - while (!AmbiguousTypoExprs.empty()) { - auto TE = AmbiguousTypoExprs.back(); - auto Cached = TransformCache[TE]; - auto &State = SemaRef.getTypoExprState(TE); - State.Consumer->saveCurrentPosition(); - TransformCache.erase(TE); - if (!TryTransform(E).isInvalid()) { - State.Consumer->resetCorrectionStream(); - TransformCache.erase(TE); - Res = ExprError(); - break; - } - AmbiguousTypoExprs.remove(TE); - State.Consumer->restoreSavedPosition(); - TransformCache[TE] = Cached; - } - SemaRef.DisableTypoCorrection = false; + bool IsAmbiguous = false; + ExprResult Res = RecursiveTransformLoop(E, IsAmbiguous); - // Ensure that all of the TypoExprs within the current Expr have been found. if (!Res.isUsable()) FindTypoExprs(TypoExprs).TraverseStmt(E); - EmitAllDiagnostics(); + EmitAllDiagnostics(IsAmbiguous); return Res; } Index: test/Sema/typo-correction-recursive.cpp =================================================================== --- /dev/null +++ test/Sema/typo-correction-recursive.cpp @@ -0,0 +1,120 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +// Check the following typo correction behavior: +// - multiple typos in a single member call chain are all diagnosed +// - no typos are diagnosed for multiple typos in an expression when not all +// typos can be corrected + +class DeepClass +{ +public: + void trigger() const; // expected-note {{'trigger' declared here}} +}; + +class Y +{ +public: + const DeepClass& getX() const { return m_deepInstance; } // expected-note {{'getX' declared here}} +private: + DeepClass m_deepInstance; + int m_n; +}; + +class Z +{ +public: + const Y& getY0() const { return m_y0; } // expected-note {{'getY0' declared here}} + const Y& getActiveY() const { return m_y0; } + +private: + Y m_y0; + Y m_y1; +}; + +Z z_obj; + +void testMultipleCorrections() +{ + z_obj.getY2(). // expected-error {{no member named 'getY2' in 'Z'; did you mean 'getY0'}} + getM(). // expected-error {{no member named 'getM' in 'Y'; did you mean 'getX'}} + triggee(); // expected-error {{no member named 'triggee' in 'DeepClass'; did you mean 'trigger'}} +} + +void testNoCorrections() +{ + z_obj.getY2(). // expected-error {{no member named 'getY2' in 'Z'}} + getM(). + thisDoesntSeemToMakeSense(); +} + +struct C {}; +struct D { int value; }; +struct A { + C get_me_a_C(); +}; +struct B { + D get_me_a_D(); // expected-note {{'get_me_a_D' declared here}} +}; +class Scope { +public: + A make_an_A(); + B make_a_B(); // expected-note {{'make_a_B' declared here}} +}; + +Scope scope_obj; + +int testDiscardedCorrections() { + return scope_obj.make_an_E(). // expected-error {{no member named 'make_an_E' in 'Scope'; did you mean 'make_a_B'}} + get_me_a_Z().value; // expected-error {{no member named 'get_me_a_Z' in 'B'; did you mean 'get_me_a_D'}} +} + +class AmbiguousHelper { +public: + int helpMe(); + int helpBe(); +}; +class Ambiguous { +public: + int calculateA(); + int calculateB(); + + AmbiguousHelper getHelp1(); + AmbiguousHelper getHelp2(); +}; + +Ambiguous ambiguous_obj; + +int testDirectAmbiguousCorrection() { + return ambiguous_obj.calculateZ(); // expected-error {{no member named 'calculateZ' in 'Ambiguous'}} +} + +int testRecursiveAmbiguousCorrection() { + return ambiguous_obj.getHelp3(). // expected-error {{no member named 'getHelp3' in 'Ambiguous'}} + helpCe(); +} + + +class DeepAmbiguityHelper { +public: + DeepAmbiguityHelper& help1(); + DeepAmbiguityHelper& help2(); + + DeepAmbiguityHelper& methodA(); + DeepAmbiguityHelper& somethingMethodB(); + DeepAmbiguityHelper& functionC(); + DeepAmbiguityHelper& deepMethodD(); + DeepAmbiguityHelper& asDeepAsItGets(); +}; + +DeepAmbiguityHelper deep_obj; + +int testDeepAmbiguity() { + deep_obj. + methodB(). // expected-error {{no member named 'methodB' in 'DeepAmbiguityHelper'}} + somethingMethodC(). + functionD(). + deepMethodD(). + help3(). + asDeepASItGet(). + functionE(); +}