dgoldman updated this revision to Diff 206011.
dgoldman added a comment.
- Add another test and fix up comment
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62648/new/
https://reviews.llvm.org/D62648
Files:
lib/Sema/SemaExprCXX.cpp
test/Sema/typo-correction-recursive.cpp
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();
+}
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7613,12 +7613,12 @@
/// 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() {
+ 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
@@ -7680,6 +7680,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 &outIsAmbiguous) {
+ 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 = TypoExprs;
+ auto SavedAmbiguousTypoExprs = AmbiguousTypoExprs;
+ llvm::SmallSetVector<TypoExpr *, 2>
+ RecursiveTypoExprs, RecursiveAmbiguousTypoExprs;
+ TypoExprs = RecursiveTypoExprs;
+ AmbiguousTypoExprs = RecursiveAmbiguousTypoExprs;
+
+ 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, outIsAmbiguous);
+ 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 = SavedTypoExprs;
+ AmbiguousTypoExprs = SavedAmbiguousTypoExprs;
+
+ return Res;
+ }
+
+ // Try to transform the given expression, looping through the correction
+ // candidates with `CheckAndAdvanceTypoExprCorrectionStreams`.
+ //
+ // If valid ambiguous typo corrections are seen, `outIsAmbiguous` is set to
+ // true and this method immediately will return an `ExprError`.
+ ExprResult RecursiveTransformLoop(Expr *E, bool &outIsAmbiguous) {
+ ExprResult Res;
+ while (true) {
+ Res = CheckForRecursiveTypos(TryTransform(E), outIsAmbiguous);
+
+ // Recursion encountered an ambiguous correction. This means that our
+ // correction itself is ambiguous, so stop now.
+ if (outIsAmbiguous)
+ 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 (!outIsAmbiguous && !Res.isInvalid() && !AmbiguousTypoExprs.empty()) {
+ llvm::SmallDenseMap<TypoExpr *, ExprResult, 2> NewTransformCache;
+ auto SavedTransformCache = TransformCache;
+ TransformCache = NewTransformCache;
+ // 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();
+ auto &State = SemaRef.getTypoExprState(TE);
+ State.Consumer->saveCurrentPosition();
+
+ TypoCorrection TC = State.Consumer->peekNextCorrection();
+ TypoCorrection Next;
+ do {
+ ExprResult AmbigRes = CheckForRecursiveTypos(TryTransform(TE), outIsAmbiguous);
+ if (!AmbigRes.isInvalid() || outIsAmbiguous) {
+ State.Consumer->resetCorrectionStream();
+ SavedTransformCache.erase(TE);
+ Res = ExprError();
+ outIsAmbiguous = true;
+ break;
+ }
+ } while((Next = State.Consumer->peekNextCorrection()) &&
+ Next.getEditDistance(false) == TC.getEditDistance(false));
+
+ if (!outIsAmbiguous) {
+ AmbiguousTypoExprs.remove(TE);
+ State.Consumer->restoreSavedPosition();
+ } else
+ break;
+ }
+ TransformCache = SavedTransformCache;
+ }
+ return Res;
+ }
+
public:
TransformTypos(Sema &SemaRef, VarDecl *InitDecl, llvm::function_ref<ExprResult(Expr *)> Filter)
: BaseTransform(SemaRef), InitDecl(InitDecl), ExprFilter(Filter) {}
@@ -7707,49 +7816,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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
