[PATCH] D80733: [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80733

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-recovery.cpp


Index: clang/test/AST/ast-dump-recovery.cpp
===================================================================
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
 // 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);
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,22 @@
   }
 
   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> {
+      using Base = TreeTransform<TyposReplace>;
+      TyposReplace(Sema &SemaRef) : Base(SemaRef) {}
+      ExprResult TransformTypoExpr(TypoExpr *E) {
+        return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+                                                E->getEndLoc(), {});
+      }
+    } TT(*this);
+
+    return TT.TransformExpr(FE);
+  }
 
   CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80733.266902.patch
Type: text/x-patch
Size: 2343 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200528/3e9d860d/attachment-0001.bin>