From 21ccc684ff4c8563e7b20bed4ae7dc7d18fe03f3 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu@gmail.com>
Date: Thu, 28 May 2020 13:43:35 +0200
Subject: [PATCH] [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Summary:
For a none-function-like unresolved expression, clang builds a TypoExpr
for it, and tries to correct it afterwards. If the typo-correction
fails, clang just drops the whole expr.

This patch improves the recovery strategy -- if the typo-correction
fails, we preserve the AST by degrading the typo exprs to recovery
exprs.

This would improve toolings for "undef_var" broken cases:
```
void foo();
void test() {
  fo^o(undef_var); // go-to-def, hover still works.
}
```

TESTED=ran tests with this patch + turn-on-recovery-ast patch, it breaks
one declare_variant_messages testcase (the diagnostics are slightly
changed), I think it is acceptable.

```
Error: 'error' diagnostics seen but not expected:
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 16: expected 'match' clause on 'omp declare variant' directive
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 57: expected 'match' clause on 'omp declare variant' directive
error: 'warning' diagnostics expected but not seen:
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 47: the context selector 'kind' in the context set 'device' cannot have a score ('<invalid>'); score ignored
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 87: the context selector 'kind' in the context set 'device' cannot have a score ('<invalid>'); score ignored
error: 'warning' diagnostics seen but not expected:
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 47: the context selector 'kind' in the context set 'device' cannot have a score ('<recovery-expr>()'); score ignored
  File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 87: the context selector 'kind' in the context set 'device' cannot have a score ('<recovery-expr>()'); score ignored
6 errors generated.
```

Reviewers: sammccall, jdoerfert

Subscribers: sstefan1, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D80733
---
 clang/lib/Sema/SemaExprCXX.cpp       | 17 +++++++++++++++--
 clang/test/AST/ast-dump-recovery.cpp | 19 +++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index b655c82..9c4e5b4 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,21 @@ ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC,
   }
 
   FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
-  if (FullExpr.isInvalid())
-    return ExprError();
+  if (FullExpr.isInvalid()) {
+    // Typo-correction fails, we rebuild the broken AST with the typos degraded
+    // to RecoveryExpr.
+    // FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+    // track source locations.
+    struct TyposReplace : TreeTransform<TyposReplace> {
+      TyposReplace(Sema &SemaRef) : TreeTransform(SemaRef) {}
+      ExprResult TransformTypoExpr(TypoExpr *E) {
+        return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+                                                E->getEndLoc(), {});
+      }
+    } TT(*this);
+
+    return TT.TransformExpr(FE);
+  }
 
   CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
 
diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index a212ff4..6d271ce 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@ int some_func(int *);
 // CHECK-NEXT:    `-IntegerLiteral {{.*}} 123
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+  // CHECK:      CallExpr {{.*}} '<dependent type>' contains-errors
+  // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT: |-RecoveryExpr {{.*}} <<invalid sloc>>
+  // CHECK-NEXT: `-BinaryOperator {{.*}} <<invalid sloc>, col:28>
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} <<invalid sloc>>
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}} <col:28> 'int' 1
+  some_func(undef1, undef2+1);
+
+  // CHECK:      BinaryOperator {{.*}} '<dependent type>' contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+  // CHECK-NEXT: `-CallExpr {{.*}} '<dependent type>' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
+  s = some_func(undef1);
+  // CHECK: `-VarDecl {{.*}} invalid var 'int'
+  // FIXME: preserve the broken call.
+  int var = some_func(undef1);
+}
 
 int ambig_func(double);
 int ambig_func(float);
-- 
2.7.4