[clang] dde98c8 - Fix issue in typo handling which could lead clang to hang

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2020-07-20T11:42:11-04:00
New Revision: dde98c82c0ad02410229e7e5c9efcbb0ab42a995

URL: https://github.com/llvm/llvm-project/commit/dde98c82c0ad02410229e7e5c9efcbb0ab42a995
DIFF: https://github.com/llvm/llvm-project/commit/dde98c82c0ad02410229e7e5c9efcbb0ab42a995.diff

LOG: Fix issue in typo handling which could lead clang to hang

Summary:
We need to detect when certain TypoExprs are not being transformed
due to invalid trees, otherwise we risk endlessly trying to fix it.

Reviewers: rsmith

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D84067

Added: 
    clang/test/Sema/typo-correction-no-hang.cpp

Modified: 
    clang/include/clang/Sema/SemaInternal.h
    clang/lib/Sema/SemaExprCXX.cpp
    clang/test/Sema/typo-correction-recursive.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Sema/SemaInternal.h b/clang/include/clang/Sema/SemaInternal.h
index cdaf7b70a92f..842eec099540 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -168,6 +168,11 @@ class TypoCorrectionConsumer : public VisibleDeclConsumer {
     return TC;
   }
 
+  /// In the case of deeply invalid expressions, `getNextCorrection()` will
+  /// never be called since the transform never makes progress. If we don't
+  /// detect this we risk trying to correct typos forever.
+  bool hasMadeAnyCorrectionProgress() const { return CurrentTCIndex != 0; }
+
   /// Reset the consumer's position in the stream of viable corrections
   /// (i.e. getNextCorrection() will return each of the previously returned
   /// corrections in order before returning any new corrections).

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 001fabff96ff..10da9595c658 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -7980,19 +7980,26 @@ class TransformTypos : public TreeTransform<TransformTypos> {
     }
   }
 
-  /// If corrections for the first TypoExpr have been exhausted for a
-  /// given combination of the other TypoExprs, retry those corrections against
-  /// the next combination of substitutions for the other TypoExprs by advancing
-  /// to the next potential correction of the second TypoExpr. For the second
-  /// and subsequent TypoExprs, if its stream of corrections has been exhausted,
-  /// the stream is reset and the next TypoExpr's stream is advanced by one (a
-  /// TypoExpr's correction stream is advanced by removing the TypoExpr from the
-  /// TransformCache). Returns true if there is still any untried combinations
-  /// of corrections.
+  /// Try to advance the typo correction state of the first unfinished TypoExpr.
+  /// We allow advancement of the correction stream by removing it from the
+  /// TransformCache which allows `TransformTypoExpr` to advance during the
+  /// next transformation attempt.
+  ///
+  /// Any substitution attempts for the previous TypoExprs (which must have been
+  /// finished) will need to be retried since it's possible that they will now
+  /// be invalid given the latest advancement.
+  ///
+  /// We need to be sure that we're making progress - it's possible that the
+  /// tree is so malformed that the transform never makes it to the
+  /// `TransformTypoExpr`.
+  ///
+  /// Returns true if there are any untried correction combinations.
   bool CheckAndAdvanceTypoExprCorrectionStreams() {
     for (auto TE : TypoExprs) {
       auto &State = SemaRef.getTypoExprState(TE);
       TransformCache.erase(TE);
+      if (!State.Consumer->hasMadeAnyCorrectionProgress())
+        return false;
       if (!State.Consumer->finished())
         return true;
       State.Consumer->resetCorrectionStream();

diff  --git a/clang/test/Sema/typo-correction-no-hang.cpp b/clang/test/Sema/typo-correction-no-hang.cpp
new file mode 100644
index 000000000000..3c591645be25
--- /dev/null
+++ b/clang/test/Sema/typo-correction-no-hang.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// From `test/Sema/typo-correction.c` but for C++ since the behavior varies
+// between the two languages.
+struct rdar38642201 {
+  int fieldName;
+};
+
+void rdar38642201_callee(int x, int y);
+void rdar38642201_caller() {
+  struct rdar38642201 structVar;
+  rdar38642201_callee(
+      structVar1.fieldName1.member1,  //expected-error{{use of undeclared identifier 'structVar1'}}
+      structVar2.fieldName2.member2); //expected-error{{use of undeclared identifier 'structVar2'}}
+}
+
+// Similar reproducer.
+class A {
+public:
+  int minut() const = delete;
+  int hour() const = delete;
+
+  int longit() const; //expected-note{{'longit' declared here}}
+  int latit() const;
+};
+
+class B {
+public:
+  A depar() const { return A(); }
+};
+
+int Foo(const B &b) {
+  return b.deparT().hours() * 60 + //expected-error{{no member named 'deparT' in 'B'}}
+         b.deparT().minutes();     //expected-error{{no member named 'deparT' in 'B'}}
+}
+
+int Bar(const B &b) {
+  return b.depar().longitude() + //expected-error{{no member named 'longitude' in 'A'; did you mean 'longit'?}}
+         b.depar().latitude();   //expected-error{{no member named 'latitude' in 'A'}}
+}

diff  --git a/clang/test/Sema/typo-correction-recursive.cpp b/clang/test/Sema/typo-correction-recursive.cpp
index 48bd3b80c599..b39beb5493f6 100644
--- a/clang/test/Sema/typo-correction-recursive.cpp
+++ b/clang/test/Sema/typo-correction-recursive.cpp
@@ -118,3 +118,15 @@ int testDeepAmbiguity() {
       asDeepASItGet().
       functionE();
 }
+
+struct Dog {
+  int age;  //expected-note{{'age' declared here}}
+  int size; //expected-note{{'size' declared here}}
+};
+
+int from_dog_years(int DogYears, int DogSize);
+int get_dog_years() {
+  struct Dog doggo;
+  return from_dog_years(doggo.agee,   //expected-error{{no member named 'agee' in 'Dog'; did you mean 'age'}}
+                        doggo.sizee); //expected-error{{no member named 'sizee' in 'Dog'; did you mean 'size'}}
+}