r369427 - [Sema][Typo] Fix assertion failure for expressions with multiple typos
David Goldman via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 20 12:03:16 PDT 2019
Author: dgoldman
Date: Tue Aug 20 12:03:15 2019
New Revision: 369427
URL: http://llvm.org/viewvc/llvm-project?rev=369427&view=rev
Log:
[Sema][Typo] Fix assertion failure for expressions with multiple typos
Summary:
As Typo Resolution can create new TypoExprs while resolving typos,
it is necessary to recurse through the expression to search for more
typos.
This should fix the assertion failure in `clang::Sema::~Sema()`:
`DelayedTypos.empty() && "Uncorrected typos!"`
Notes:
- In case some TypoExprs are created but thrown away, Sema
now has a Vector that is used to keep track of newly created
typos.
- For expressions with multiple typos, we only give suggestions
if we are able to resolve all typos in the expression
- This patch is similar to D37521 except that it does not eagerly
commit to a correction for the first typo in the expression.
Instead, it will search for corrections which fix all of the
typos in the expression.
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D62648
Added:
cfe/trunk/test/Sema/typo-correction-recursive.cpp
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=369427&r1=369426&r2=369427&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Aug 20 12:03:15 2019
@@ -405,6 +405,12 @@ public:
/// Source location for newly created implicit MSInheritanceAttrs
SourceLocation ImplicitMSInheritanceAttrLoc;
+ /// Holds TypoExprs that are created from `createDelayedTypo`. This is used by
+ /// `TransformTypos` in order to keep track of any TypoExprs that are created
+ /// recursively during typo correction and wipe them away if the correction
+ /// fails.
+ llvm::SmallVector<TypoExpr *, 2> TypoExprs;
+
/// pragma clang section kind
enum PragmaClangSectionKind {
PCSK_Invalid = 0,
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=369427&r1=369426&r2=369427&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Aug 20 12:03:15 2019
@@ -7580,15 +7580,22 @@ class TransformTypos : public TreeTransf
llvm::SmallDenseMap<OverloadExpr *, Expr *, 4> 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
@@ -7650,6 +7657,145 @@ class TransformTypos : public TreeTransf
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. Remove them from Sema's TypoExpr list
+ // since we don't want to clear them twice. Note: it's possible the
+ // TypoExprs were created recursively and thus won't be in our
+ // Sema's TypoExprs - they were created in our `RecursiveTransformLoop`.
+ auto &SemaTypoExprs = SemaRef.TypoExprs;
+ for (auto TE : TypoExprs) {
+ TransformCache.erase(TE);
+ SemaRef.clearDelayedTypo(TE);
+
+ auto SI = find(SemaTypoExprs, TE);
+ if (SI != SemaTypoExprs.end()) {
+ SemaTypoExprs.erase(SI);
+ }
+ }
+ } 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;
+ auto SavedTypoExprs = std::move(SemaRef.TypoExprs);
+ SemaRef.TypoExprs.clear();
+
+ 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);
+ }
+
+ // Wipe away any newly created TypoExprs that we don't know about. Since we
+ // clear any invalid TypoExprs in `CheckForRecursiveTypos`, this is only
+ // possible if a `TypoExpr` is created during a transformation but then
+ // fails before we can discover it.
+ auto &SemaTypoExprs = SemaRef.TypoExprs;
+ for (auto Iterator = SemaTypoExprs.begin(); Iterator != SemaTypoExprs.end();) {
+ auto TE = *Iterator;
+ auto FI = find(TypoExprs, TE);
+ if (FI != TypoExprs.end()) {
+ Iterator++;
+ continue;
+ }
+ SemaRef.clearDelayedTypo(TE);
+ Iterator = SemaTypoExprs.erase(Iterator);
+ }
+ SemaRef.TypoExprs = std::move(SavedTypoExprs);
+
+ return Res;
+ }
+
public:
TransformTypos(Sema &SemaRef, VarDecl *InitDecl, llvm::function_ref<ExprResult(Expr *)> Filter)
: BaseTransform(SemaRef), InitDecl(InitDecl), ExprFilter(Filter) {}
@@ -7677,49 +7823,13 @@ public:
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;
}
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=369427&r1=369426&r2=369427&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue Aug 20 12:03:15 2019
@@ -5434,6 +5434,8 @@ TypoExpr *Sema::createDelayedTypo(std::u
State.Consumer = std::move(TCC);
State.DiagHandler = std::move(TDG);
State.RecoveryHandler = std::move(TRC);
+ if (TE)
+ TypoExprs.push_back(TE);
return TE;
}
Added: cfe/trunk/test/Sema/typo-correction-recursive.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/typo-correction-recursive.cpp?rev=369427&view=auto
==============================================================================
--- cfe/trunk/test/Sema/typo-correction-recursive.cpp (added)
+++ cfe/trunk/test/Sema/typo-correction-recursive.cpp Tue Aug 20 12:03:15 2019
@@ -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();
+}
More information about the cfe-commits
mailing list