[clang] Fix an infinite loop during typo-correction

[hokein]

Summary

See https://bugs.llvm.org/show_bug.cgi?id=50797#c6

[sammccall]

Great analysis, and the bailout seems reasonable. The overall algorithm seems terribly confusing though :-(

clang/lib/Sema/SemaExprCXX.cpp

8338

Comment is good but maybe could mention the high-level effect of breaking out (treat as unambiguous)

[dgoldman]

clang/lib/Sema/SemaExprCXX.cpp

8338

At this point we know the tree is so invalid that transforming no longer works, so our correction didn't really help. Is it even worth suggesting a correction (if we treat it as ambiguous we won't)?

8339–8342

Is all of this needed - can you just change the while below to also be && Next != TC? Could move Next assignment up into the body if we want to change how we handle stalled progress

[dgoldman]

clang/lib/Sema/SemaExprCXX.cpp

8338

Nevermind, misunderstood, left some comments on the bug there. What happens if the right correction actually was ambiguous though? Would we skip it here?

clang/test/Sema/typo-correction-no-hang.c

5–15

Could you add another test case for the same thing but making the correction ambiguous, e.g. g_998 or similar?

[hokein]

refine the fix based on the discussion: https://bugs.llvm.org/show_bug.cgi?id=50797#c13

[dgoldman]

Thanks! Just to confirm, the non-simplified example is also fixed, right?

struct a {
  int xxx;
};

int g_107;
int g_108;
int g_109;

struct a g_999;

void b() { (g_910.xxx = g_910.xxx1); }

[hokein]

In D105533#2865803, @dgoldman wrote:

Thanks! Just to confirm, the non-simplified example is also fixed, right?

struct a {
  int xxx;
};

int g_107;
int g_108;
int g_109;

struct a g_999;

void b() { (g_910.xxx = g_910.xxx1); }

yeah, it works, added to the test.

[hokein]

add the non-simplified sample.

clang/lib/Sema/SemaExprCXX.cpp

if (!IsAmbiguous && !Res.isInvalid() && !AmbiguousTypoExprs.empty()) {
 
           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[TE] = SavedTransformCache[TE];
       }
       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.

clang/test/Sema/typo-correction-ambiguity.c

 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
 // Check the following typo correction behavior in C:
 // - no typos are diagnosed when a call expression has ambiguous (multiple) corrections
 
 int v_63;
 
 void v_2_0(int v_452, int v_454) {}
 
 int v_3_0() {
    for (int v_345 = 0 ; v_63;)
        v_2_0(v_195,  // expected-error {{use of undeclared identifier 'v_195'}}
              v_231);  // expected-error {{use of undeclared identifier 'v_231'}}
 }
+
+// Test: no typo-correction diagnostics are emitted for ambiguous typos.
+struct a {
+  int xxx;
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999;
+struct a g_998;
+void PR50797() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of undeclared identifier 'g_910'}}

clang/test/Sema/typo-correction-no-hang.c

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// PR50797
+struct a {
+  int xxx; // expected-note {{'xxx' declared here}}
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999; // expected-note 4{{'g_999' declared here}}
+
+void b() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of undeclared identifier 'g_910'; did you mean 'g_999'}}
+
+void c() { (g_910.xxx = g_910.xxx1); } //expected-error 2{{use of undeclared identifier 'g_910'; did you mean 'g_999'}} \
+                                         expected-error {{no member named 'xxx1' in 'struct a'; did you mean 'xxx'}}