vsapsai created this revision.
vsapsai added reviewers: arphaman, majnemer.

NumTypos guard value ~0U doesn't prevent from creating new delayed
typos. When you create new delayed typos during typo correction, value
~0U wraps around to 0. This state is inconsistent and depending on total
number of typos you can hit the assertion

> Assertion failed: (DelayedTypos.empty() && "Uncorrected typos!"), function 
> ~Sema, file clang/lib/Sema/Sema.cpp, line 366.

or have infinite typo correction loop.

Fix by disabling typo correction during performing typo correcting
transform. It disables the feature of having typo corrections on top of
other typo corrections because that feature is unreliable.

rdar://problem/38642201


https://reviews.llvm.org/D47341

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/Sema/typo-correction.c
  clang/test/SemaCXX/typo-correction-delayed.cpp

Index: clang/test/SemaCXX/typo-correction-delayed.cpp
===================================================================
--- clang/test/SemaCXX/typo-correction-delayed.cpp
+++ clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -137,13 +137,12 @@
 
 namespace PR21925 {
 struct X {
-  int get() { return 7; }  // expected-note {{'get' declared here}}
+  int get() { return 7; }
 };
 void test() {
-  X variable;  // expected-note {{'variable' declared here}}
+  X variable;
 
-  // expected-error@+2 {{use of undeclared identifier 'variableX'; did you mean 'variable'?}}
-  // expected-error@+1 {{no member named 'getX' in 'PR21925::X'; did you mean 'get'?}}
+  // expected-error@+1 {{use of undeclared identifier 'variableX'}}
   int x = variableX.getX();
 }
 }
Index: clang/test/Sema/typo-correction.c
===================================================================
--- clang/test/Sema/typo-correction.c
+++ clang/test/Sema/typo-correction.c
@@ -87,3 +87,16 @@
 void overloadable_callexpr(int arg) {
 	func_overloadable(ar); //expected-error{{use of undeclared identifier}}
 }
+
+// rdar://problem/38642201
+struct rdar38642201 {
+  int fieldName;
+};
+
+void rdar38642201_callee(int x, int y);
+void rdar38642201_caller() {
+  struct rdar38642201 structVar;
+  rdar38642201_callee(
+      structVarTypo.fieldNameTypo, //expected-error{{use of undeclared identifier 'structVarTypo'}}
+      structVarTypo2.fieldNameTypo2); //expected-error{{use of undeclared identifier 'structVarTypo2'}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7588,7 +7588,7 @@
     // 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;
+    Sema::DisableTypoCorrectionScope Scope(SemaRef);
     while (!AmbiguousTypoExprs.empty()) {
       auto TE  = AmbiguousTypoExprs.back();
       auto Cached = TransformCache[TE];
@@ -7605,7 +7605,6 @@
       State.Consumer->restoreSavedPosition();
       TransformCache[TE] = Cached;
     }
-    SemaRef.DisableTypoCorrection = false;
 
     // Ensure that all of the TypoExprs within the current Expr have been found.
     if (!Res.isUsable())
@@ -7668,11 +7667,14 @@
   if (E && !ExprEvalContexts.empty() && ExprEvalContexts.back().NumTypos &&
       (E->isTypeDependent() || E->isValueDependent() ||
        E->isInstantiationDependent())) {
+    DisableTypoCorrectionScope DisableRecurrentCorrectionScope(*this);
     auto TyposInContext = ExprEvalContexts.back().NumTypos;
     assert(TyposInContext < ~0U && "Recursive call of CorrectDelayedTyposInExpr");
     ExprEvalContexts.back().NumTypos = ~0U;
     auto TyposResolved = DelayedTypos.size();
     auto Result = TransformTypos(*this, InitDecl, Filter).Transform(E);
+    assert(ExprEvalContexts.back().NumTypos == ~0U &&
+           "Unexpected NumTypos modification");
     ExprEvalContexts.back().NumTypos = TyposInContext;
     TyposResolved -= DelayedTypos.size();
     if (Result.isInvalid() || Result.get() != E) {
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -7561,6 +7561,22 @@
     }
   };
 
+  /// RAII class used to disable typo correction temporarily.
+  class DisableTypoCorrectionScope {
+    Sema &SemaRef;
+    bool PrevDisableTypoCorrection;
+
+  public:
+    explicit DisableTypoCorrectionScope(Sema &SemaRef)
+        : SemaRef(SemaRef),
+          PrevDisableTypoCorrection(SemaRef.DisableTypoCorrection) {
+      SemaRef.DisableTypoCorrection = true;
+    }
+    ~DisableTypoCorrectionScope() {
+      SemaRef.DisableTypoCorrection = PrevDisableTypoCorrection;
+    }
+  };
+
   /// The current instantiation scope used to store local
   /// variables.
   LocalInstantiationScope *CurrentInstantiationScope;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D47341: [... Volodymyr Sapsai via Phabricator via cfe-commits

Reply via email to