[AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.

[hokein]

Summary

For a no-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.

[sammccall]

Nice!

clang/lib/Sema/SemaExprCXX.cpp

8310

This seems reasonably straightforward to fix - there shouldn't be a lot of TypoExprs so we can just add a member. Worth doing I think.

8314

Can't you just write this as : TreeTransform(SemaRef) {}, and drop the using?

[hokein]

Address comments.

[hokein]

clang/lib/Sema/SemaExprCXX.cpp

8310

yeah, agree. Will address it in a followup.

8314

yes.

clang/lib/Sema/SemaExprCXX.cpp

if (DiscardedValue) {
     FullExpr = IgnoredValueConversions(FullExpr.get());
     if (FullExpr.isInvalid())
       return ExprError();
 
     DiagnoseUnusedExprResult(FullExpr.get());
   }
 
   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);
 
   // At the end of this full expression (which could be a deeply nested
   // lambda), if there is a potential capture within the nested lambda,
   // have the outer capture-able lambda try and capture it.
   // Consider the following code:
   // void f(int, int);

clang/test/AST/ast-dump-recovery.cpp

 // RUN: not %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -frecovery-ast -frecovery-ast-type -ast-dump %s | FileCheck -strict-whitespace %s
 // RUN: not %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -fno-recovery-ast -ast-dump %s | FileCheck --check-prefix=DISABLED -strict-whitespace %s
 
 int some_func(int *);
 
 // CHECK:     VarDecl {{.*}} invalid_call
 // CHECK-NEXT: `-ImplicitCastExpr {{.*}} 'int' contains-errors
 // CHECK-NEXT:  `-RecoveryExpr {{.*}} 'int' contains-errors
 // CHECK-NEXT:    |-UnresolvedLookupExpr {{.*}} 'some_func'
 // 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);
 
 // CHECK:     VarDecl {{.*}} ambig_call
 // CHECK-NEXT: `-ImplicitCastExpr {{.*}} 'int' contains-errors
 // CHECK-NEXT:  `-RecoveryExpr {{.*}} 'int' contains-errors
 // CHECK-NEXT:    |-UnresolvedLookupExpr {{.*}} 'ambig_func'