ilya-biryukov created this revision.
ilya-biryukov added a reviewer: rsmith.
Herald added subscribers: usaxena95, erik.pilkington, kadircet, arphaman.
Herald added a project: clang.

Normally clang avoids creating expressions when it encounters semantic
errors, even if the parser knows which expression to produce.

This works well for the compiler. However, this is not ideal for
source-level tools that have to deal with broken code, e.g. clangd is
not able to provide navigation features even for names that compiler
knows how to resolve.

The new RecoveryExpr aims to capture the minimal set of information
useful for the tools that need to deal with incorrect code:

- source range of the expression being dropped,
- subexpressions of the expression.

We aim to make constructing RecoveryExprs as simple as possible to
ensure writing code to avoid dropping expressions is easy.

Producing RecoveryExprs can result in new code paths being taken in the
frontend. In particular, clang can produce some new diagnostics now and
we aim to suppress bogus ones based on `Stmt::containsErrors`.

We deliberately produce RecoveryExprs only in the parser for now to
minimize the code affected by this patch. Producing RecoveryExprs in
Sema potentially allows to preserve more information (e.g. type of an
expression), but also results in more code being affected. E.g.
SFINAE checks will have to take presence of RecoveryExprs into account.

Initial implementation only works in C++ mode, as it relies on compiler
postponing diagnostics on dependent expressions. C and ObjC often do not
do this, so they require more work to make sure we do not produce too
many bogus diagnostics on the new expressions.

See documentation of RecoveryExpr for more details.

This change is based on https://reviews.llvm.org/D61722


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69330

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/AST/ast-dump-expr-errors.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/Index/getcursor-recovery.cpp
  clang/test/Parser/cxx-concepts-ambig-constraint-expr.cpp
  clang/test/Parser/objcxx0x-lambda-expressions.mm
  clang/test/Parser/objcxx11-invalid-lambda.cpp
  clang/test/SemaCXX/builtin-operator-new-delete.cpp
  clang/test/SemaCXX/builtins.cpp
  clang/test/SemaCXX/cast-conversion.cpp
  clang/test/SemaCXX/cxx1z-copy-omission.cpp
  clang/test/SemaCXX/decltype-crash.cpp
  clang/test/SemaCXX/varargs.cpp
  clang/test/SemaOpenCLCXX/address-space-references.cl
  clang/test/SemaTemplate/instantiate-init.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===================================================================
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -289,6 +289,7 @@
   case Stmt::ObjCDictionaryLiteralClass:
   case Stmt::ObjCBoxedExprClass:
   case Stmt::ObjCSubscriptRefExprClass:
+  case Stmt::RecoveryExprClass:
     K = CXCursor_UnexposedExpr;
     break;
 
Index: clang/test/SemaTemplate/instantiate-init.cpp
===================================================================
--- clang/test/SemaTemplate/instantiate-init.cpp
+++ clang/test/SemaTemplate/instantiate-init.cpp
@@ -99,7 +99,7 @@
   void test() {
     integral_c<1> ic1 = array_lengthof(Description<int>::data);
     (void)sizeof(array_lengthof(Description<float>::data));
-
+    // expected-warning@+1{{expression result unused}}
     sizeof(array_lengthof( // expected-error{{no matching function for call to 'array_lengthof'}}
                           Description<int*>::data // expected-note{{in instantiation of static data member 'PR7985::Description<int *>::data' requested here}}
                           ));
Index: clang/test/SemaOpenCLCXX/address-space-references.cl
===================================================================
--- clang/test/SemaOpenCLCXX/address-space-references.cl
+++ clang/test/SemaOpenCLCXX/address-space-references.cl
@@ -11,5 +11,6 @@
 int bar(const unsigned int &i);
 
 void foo() {
-  bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
+  bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}} \
+         // expected-error{{expected ';' after expression}}
 }
Index: clang/test/SemaCXX/varargs.cpp
===================================================================
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -22,7 +22,8 @@
 // default ctor.
 void record_context(int a, ...) {
   struct Foo {
-    // expected-error@+1 {{'va_start' cannot be used outside a function}}
+    // expected-error@+2 {{'va_start' cannot be used outside a function}}
+    // expected-error@+1 {{default argument references parameter 'a'}}
     void meth(int a, int b = (__builtin_va_start(ap, a), 0)) {}
   };
 }
Index: clang/test/SemaCXX/decltype-crash.cpp
===================================================================
--- clang/test/SemaCXX/decltype-crash.cpp
+++ clang/test/SemaCXX/decltype-crash.cpp
@@ -3,5 +3,8 @@
 int& a();
 
 void f() {
-  decltype(a()) c; // expected-warning {{'decltype' is a keyword in C++11}} expected-error {{use of undeclared identifier 'decltype'}}
+  decltype(a()) c; // expected-warning {{'decltype' is a keyword in C++11}} \
+                   // expected-error {{use of undeclared identifier 'decltype'}} \
+                   // expected-error {{expected ';' after expression}} \
+                   // expected-error {{use of undeclared identifier 'c'}}
 }
Index: clang/test/SemaCXX/cxx1z-copy-omission.cpp
===================================================================
--- clang/test/SemaCXX/cxx1z-copy-omission.cpp
+++ clang/test/SemaCXX/cxx1z-copy-omission.cpp
@@ -100,15 +100,21 @@
   make_incomplete(); // expected-error {{incomplete}}
   make_incomplete().a; // expected-error {{incomplete}}
   make_incomplete().*(int Incomplete::*)nullptr; // expected-error {{incomplete}}
-  dynamic_cast<Incomplete&&>(make_incomplete()); // expected-error {{incomplete}}
-  const_cast<Incomplete&&>(make_incomplete()); // expected-error {{incomplete}}
+  dynamic_cast<Incomplete&&>(make_incomplete()); // expected-error {{incomplete}} \
+                                                 // expected-warning {{expression result unused}}
+  const_cast<Incomplete&&>(make_incomplete()); // expected-error {{incomplete}} \
+                                               // expected-warning {{expression result unused}}
 
   sizeof(Indestructible{}); // expected-error {{deleted}}
-  sizeof(make_indestructible()); // expected-error {{deleted}}
-  sizeof(make_incomplete()); // expected-error {{incomplete}}
+  sizeof(make_indestructible()); // expected-error {{deleted}} \
+                                 // expected-warning {{expression result unused}}
+  sizeof(make_incomplete()); // expected-error {{incomplete}} \
+                             // expected-warning {{expression result unused}}
   typeid(Indestructible{}); // expected-error {{deleted}}
-  typeid(make_indestructible()); // expected-error {{deleted}}
-  typeid(make_incomplete()); // expected-error {{incomplete}}
+  typeid(make_indestructible()); // expected-error {{deleted}} \
+                                 // expected-error {{need to include <typeinfo>}}
+  typeid(make_incomplete()); // expected-error {{incomplete}} \
+                             // expected-error {{need to include <typeinfo>}}
 
   // FIXME: The first two cases here are now also valid in C++17 onwards.
   using I = decltype(Indestructible()); // expected-error {{deleted}}
Index: clang/test/SemaCXX/cast-conversion.cpp
===================================================================
--- clang/test/SemaCXX/cast-conversion.cpp
+++ clang/test/SemaCXX/cast-conversion.cpp
@@ -41,7 +41,8 @@
   template <unsigned> float* &f0(); // expected-note{{candidate}}
 
   void f1() {
-    static_cast<float*>(f0<0>()); // expected-error{{ambiguous}}
+    static_cast<float*>(f0<0>()); // expected-error{{ambiguous}} \
+                                  // expected-warning{{result unused}}
   }
 };
 
Index: clang/test/SemaCXX/builtins.cpp
===================================================================
--- clang/test/SemaCXX/builtins.cpp
+++ clang/test/SemaCXX/builtins.cpp
@@ -14,8 +14,7 @@
 int equal(const char *s1, const char *s2) {
   return Compare(s1, s2) == 0;
 }
-// FIXME: Our error recovery here sucks
-template int equal<&__builtin_strcmp>(const char*, const char*); // expected-error {{builtin functions must be directly called}} expected-error {{expected unqualified-id}} expected-error {{expected ')'}} expected-note {{to match this '('}}
+template int equal<&__builtin_strcmp>(const char*, const char*); // expected-error {{builtin functions must be directly called}}
 
 // PR13195
 void f2() {
Index: clang/test/SemaCXX/builtin-operator-new-delete.cpp
===================================================================
--- clang/test/SemaCXX/builtin-operator-new-delete.cpp
+++ clang/test/SemaCXX/builtin-operator-new-delete.cpp
@@ -91,7 +91,7 @@
   // but LangOpts::AlignedAllocation is false. Should our overloads be considered
   // usual allocation/deallocation functions?
   void *p = __builtin_operator_new(42, std::align_val_t(2)); // expected-error {{call to '__builtin_operator_new' selects non-usual allocation function}}
-  __builtin_operator_delete(p, std::align_val_t(2));         // expected-error {{call to '__builtin_operator_delete' selects non-usual deallocation function}}
+  __builtin_operator_delete((void*)0, std::align_val_t(2));         // expected-error {{call to '__builtin_operator_delete' selects non-usual deallocation function}}
 #endif
 }
 
Index: clang/test/Parser/objcxx11-invalid-lambda.cpp
===================================================================
--- clang/test/Parser/objcxx11-invalid-lambda.cpp
+++ clang/test/Parser/objcxx11-invalid-lambda.cpp
@@ -1,10 +1,11 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -x objective-c++ -std=c++11 %s
 
-void foo() {  // expected-note {{to match this '{'}}
+void foo() {
   int bar;
   auto baz = [
-      bar(  // expected-note {{to match this '('}} expected-note {{to match this '('}}
+      bar(  // expected-note 2{{to match this '('}} \
+            // expected-warning {{captures are a C++14 extension}}
         foo_undeclared() // expected-error{{use of undeclared identifier 'foo_undeclared'}}
       /* ) */
-    ] () { };   // expected-error{{expected ')'}}
-}               // expected-error{{expected ')'}} expected-error {{expected ',' or ']'}} expected-error{{expected ';' at end of declaration}} expected-error{{expected '}'}}
+    ] () { };   // expected-error 2{{expected ')'}}
+}
Index: clang/test/Parser/objcxx0x-lambda-expressions.mm
===================================================================
--- clang/test/Parser/objcxx0x-lambda-expressions.mm
+++ clang/test/Parser/objcxx0x-lambda-expressions.mm
@@ -11,7 +11,8 @@
 
     []; // expected-error {{expected body of lambda expression}}
     [=,foo+] {}; // expected-error {{expected ',' or ']' in lambda capture list}}
-    [&this] {}; // expected-error {{cannot take the address of an rvalue of type 'C *'}}
+    [&this] {}; // expected-error {{cannot take the address of an rvalue of type 'C *'}} \
+                // expected-error {{expected identifier}}
     [] {}; 
     [=] (int i) {}; 
     [&] (int) mutable -> void {}; 
@@ -24,7 +25,8 @@
     [foo{bar}] () {};
     [foo = {bar}] () {}; // expected-error {{<initializer_list>}}
 
-    [foo(bar) baz] () {}; // expected-error {{called object type 'int' is not a function}}
+    [foo(bar) baz] () {}; // expected-error {{called object type 'int' is not a function}} \
+                          // expected-error {{expected ';'}}
     [foo(bar), baz] () {}; // ok
 
     [foo = bar baz]; // expected-warning {{receiver type 'int'}} expected-warning {{instance method '-baz'}}
Index: clang/test/Parser/cxx-concepts-ambig-constraint-expr.cpp
===================================================================
--- clang/test/Parser/cxx-concepts-ambig-constraint-expr.cpp
+++ clang/test/Parser/cxx-concepts-ambig-constraint-expr.cpp
@@ -25,5 +25,6 @@
 struct A {
   static int t;
   template <typename T> requires bool(T())
-  (A(T (&t))) { } // expected-error {{called object type 'bool' is not a function or function pointer}}
+  (A(T (&t))) { } // expected-error {{called object type 'bool' is not a function or function pointer}} \
+                  // expected-error {{expected member name or ';' after declaration specifiers}}
 };
Index: clang/test/Index/getcursor-recovery.cpp
===================================================================
--- /dev/null
+++ clang/test/Index/getcursor-recovery.cpp
@@ -0,0 +1,16 @@
+int foo(int, int);
+int foo(int, double);
+int x;
+
+void testTypedRecoveryExpr() {
+  // Inner foo() is a RecoveryExpr, outer foo() is an overloaded call.
+  foo(x, foo(x));
+}
+// RUN: c-index-test -cursor-at=%s:7:3 %s | FileCheck -check-prefix=OUTER-FOO %s
+// OUTER-FOO: OverloadedDeclRef=foo[2:5, 1:5]
+// RUN: c-index-test -cursor-at=%s:7:7 %s | FileCheck -check-prefix=OUTER-X %s
+// OUTER-X: DeclRefExpr=x:3:5
+// RUN: c-index-test -cursor-at=%s:7:10 %s | FileCheck -check-prefix=INNER-FOO %s
+// INNER-FOO: OverloadedDeclRef=foo[2:5, 1:5]
+// RUN: c-index-test -cursor-at=%s:7:14 %s | FileCheck -check-prefix=INNER-X %s
+// INNER-X: DeclRefExpr=x:3:5
Index: clang/test/AST/ast-dump-recovery.cpp
===================================================================
--- /dev/null
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -0,0 +1,74 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -ast-dump %s | FileCheck -strict-whitespace %s
+
+int some_func(int *);
+
+// CHECK:     VarDecl {{.*}} invalid_call
+// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  |-UnresolvedLookupExpr {{.*}} 'some_func'
+// CHECK-NEXT:  `-IntegerLiteral {{.*}} 123
+int invalid_call = some_func(123);
+
+int ambig_func(double);
+int ambig_func(float);
+
+// CHECK:     VarDecl {{.*}} ambig_call
+// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  |-UnresolvedLookupExpr {{.*}} 'ambig_func'
+// CHECK-NEXT:  `-IntegerLiteral {{.*}} 123
+int ambig_call = ambig_func(123);
+
+// CHECK:     VarDecl {{.*}} unresolved_call1
+// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  `-UnresolvedLookupExpr {{.*}} 'bar'
+int unresolved_call1 = bar();
+
+// CHECK:     VarDecl {{.*}} unresolved_call2
+// CHECK-NEXT:`-CallExpr {{.*}} contains-errors
+// CHECK-NEXT:  |-UnresolvedLookupExpr {{.*}} 'bar'
+// CHECK-NEXT:  |-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  | `-UnresolvedLookupExpr {{.*}} 'baz'
+// CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:     `-UnresolvedLookupExpr {{.*}} 'qux'
+int unresolved_call2 = bar(baz(), qux());
+
+constexpr int a = 10;
+
+// CHECK:     VarDecl {{.*}} postfix_inc
+// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  `-DeclRefExpr {{.*}} 'a'
+int postfix_inc = a++;
+
+// CHECK:     VarDecl {{.*}} prefix_inc
+// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  `-DeclRefExpr {{.*}} 'a'
+int prefix_inc = ++a;
+
+// CHECK:     VarDecl {{.*}} unary_address
+// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  `-ParenExpr {{.*}}
+// CHECK-NEXT:    `-BinaryOperator {{.*}} '+'
+// CHECK-NEXT:      |-ImplicitCastExpr
+// CHECK-NEXT:      | `-DeclRefExpr {{.*}} 'a'
+int unary_address = &(a + 1);
+
+// CHECK:     VarDecl {{.*}} unary_bitinverse
+// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  `-ParenExpr {{.*}}
+// CHECK-NEXT:    `-BinaryOperator {{.*}} '+'
+// CHECK-NEXT:      |-ImplicitCastExpr
+// CHECK-NEXT:      | `-ImplicitCastExpr
+// CHECK-NEXT:      |   `-DeclRefExpr {{.*}} 'a'
+int unary_bitinverse = ~(a + 0.0);
+
+// CHECK:     VarDecl {{.*}} binary
+// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  |-DeclRefExpr {{.*}} 'a'
+// CHECK-NEXT:  `-CXXNullPtrLiteralExpr
+int binary = a + nullptr;
+
+// CHECK:     VarDecl {{.*}} ternary
+// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  |-DeclRefExpr {{.*}} 'a'
+// CHECK-NEXT:  |-CXXNullPtrLiteralExpr
+// CHECK-NEXT:  `-DeclRefExpr {{.*}} 'a'
+int ternary = a ? nullptr : a;
Index: clang/test/AST/ast-dump-expr-errors.cpp
===================================================================
--- /dev/null
+++ clang/test/AST/ast-dump-expr-errors.cpp
@@ -0,0 +1,46 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -ast-dump %s | FileCheck -strict-whitespace %s
+
+// Check errors flag is set for RecoveryExpr.
+//
+// CHECK:     VarDecl {{.*}} a
+// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  `-UnresolvedLookupExpr {{.*}} 'bar'
+int a = bar();
+
+// The flag propagates through more complicated calls.
+//
+// CHECK:     VarDecl {{.*}} b
+// CHECK-NEXT:`-CallExpr {{.*}} contains-errors
+// CHECK-NEXT:  |-UnresolvedLookupExpr {{.*}} 'bar'
+// CHECK-NEXT:  |-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  | `-UnresolvedLookupExpr {{.*}} 'baz'
+// CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:     `-UnresolvedLookupExpr {{.*}} 'qux'
+int b = bar(baz(), qux());
+
+// Also propagates through more complicated expressions.
+//
+// CHECK:     |-VarDecl {{.*}} c
+// CHECK-NEXT:| `-BinaryOperator {{.*}} '<dependent type>' contains-errors '*'
+// CHECK-NEXT:|   |-UnaryOperator {{.*}} '<dependent type>' contains-errors prefix '&'
+// CHECK-NEXT:|   | `-ParenExpr {{.*}} '<dependent type>' contains-errors
+// CHECK-NEXT:|   |   `-BinaryOperator {{.*}} '<dependent type>' contains-errors '+'
+// CHECK-NEXT:|   |     |-RecoveryExpr {{.*}} '<dependent type>' contains-errors
+// CHECK-NEXT:|   |     | `-UnresolvedLookupExpr {{.*}} 'bar'
+// CHECK-NEXT:|   |     `-RecoveryExpr {{.*}} '<dependent type>' contains-errors
+// CHECK-NEXT:|   |       `-UnresolvedLookupExpr {{.*}} 'baz'
+int c = &(bar() + baz()) * 10;
+
+// Errors flag propagates even when type is not dependent anymore.
+// CHECK:     |-VarDecl {{.*}} d
+// CHECK-NEXT:| `-CXXStaticCastExpr {{.*}} 'int' contains-errors
+// CHECK-NEXT:|   `-BinaryOperator {{.*}} '<dependent type>' contains-errors '+'
+// CHECK-NEXT:|     |-RecoveryExpr {{.*}} '<dependent type>' contains-errors <recovery-expr>
+// CHECK-NEXT:|     | `-UnresolvedLookupExpr {{.*}} 'bar'
+// CHECK-NEXT:|     `-IntegerLiteral {{.*}} 1
+int d = static_cast<int>(bar() + 1);
+
+// FIXME: store initializer even when 'auto' could not be deduced.
+// Expressions with errors currently do not keep initializers around.
+// CHECK:     `-VarDecl {{.*}} invalid e 'auto'
+auto e = bar();
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1220,6 +1220,7 @@
     case Stmt::UnresolvedLookupExprClass:
     case Stmt::UnresolvedMemberExprClass:
     case Stmt::TypoExprClass:
+    case Stmt::RecoveryExprClass:
     case Stmt::CXXNoexceptExprClass:
     case Stmt::PackExpansionExprClass:
     case Stmt::SubstNonTypeTemplateParmPackExprClass:
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===================================================================
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -11,6 +11,7 @@
 ///
 //===----------------------------------------------------------------------===//
 
+#include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
@@ -672,6 +673,16 @@
   Code = serialization::EXPR_CALL;
 }
 
+void ASTStmtWriter::VisitRecoveryExpr(RecoveryExpr *E) {
+  VisitExpr(E);
+  Record.push_back(std::distance(E->children().begin(), E->children().end()));
+  Record.AddSourceLocation(E->getBeginLoc());
+  Record.AddSourceLocation(E->getEndLoc());
+  for (Stmt *Child : E->children())
+    Record.AddStmt(Child);
+  Code = serialization::EXPR_RECOVERY;
+}
+
 void ASTStmtWriter::VisitMemberExpr(MemberExpr *E) {
   VisitExpr(E);
 
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1904,6 +1904,19 @@
   llvm_unreachable("Cannot read TypoExpr nodes");
 }
 
+void ASTStmtReader::VisitRecoveryExpr(RecoveryExpr *E) {
+  VisitExpr(E);
+  unsigned NumArgs = Record.readInt();
+  E->BeginLoc = ReadSourceLocation();
+  E->EndLoc = ReadSourceLocation();
+  assert(
+      (NumArgs == std::distance(E->children().begin(), E->children().end())) &&
+      "Wrong NumArgs!");
+  (void)NumArgs;
+  for (Stmt *&Child : E->children())
+    Child = Record.readSubStmt();
+}
+
 //===----------------------------------------------------------------------===//
 // Microsoft Expressions and Statements
 //===----------------------------------------------------------------------===//
@@ -2642,6 +2655,11 @@
           Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
       break;
 
+    case EXPR_RECOVERY:
+      S = RecoveryExpr::CreateEmpty(
+          Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields]);
+      break;
+
     case EXPR_MEMBER:
       S = MemberExpr::CreateEmpty(Context, Record[ASTStmtReader::NumExprFields],
                                   Record[ASTStmtReader::NumExprFields + 1],
Index: clang/lib/Sema/TreeTransform.h
===================================================================
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -9465,6 +9465,11 @@
   return E;
 }
 
+template <typename Derived>
+ExprResult TreeTransform<Derived>::TransformRecoveryExpr(RecoveryExpr *E) {
+  return E;
+}
+
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformPseudoObjectExpr(PseudoObjectExpr *E) {
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17960,3 +17960,20 @@
   return new (Context)
       ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
 }
+
+ExprResult Sema::ActOnSemanticError(SourceLocation Begin, SourceLocation End,
+                                    ArrayRef<Expr *> SubExprs) {
+  // Only enable RecoveryExpr in C++, RecoveryExpr is type-dependent to suppress
+  // bogus diagnostics and this trick does not work in C.
+  // FIXME: use containsErrors() to suppress unwanted diags in C.
+  if (!Context.getLangOpts().CPlusPlus)
+    return ExprError();
+
+  llvm::SmallVector<Expr *, 8> NoTypos;
+  for (auto E : SubExprs) {
+    ExprResult Corrected = CorrectDelayedTyposInExpr(E);
+    if (!Corrected.isInvalid())
+      NoTypos.push_back(Corrected.get());
+  }
+  return RecoveryExpr::Create(Context, Begin, End, NoTypos);
+}
Index: clang/lib/Sema/SemaExceptionSpec.cpp
===================================================================
--- clang/lib/Sema/SemaExceptionSpec.cpp
+++ clang/lib/Sema/SemaExceptionSpec.cpp
@@ -1282,6 +1282,7 @@
   case Expr::UnresolvedLookupExprClass:
   case Expr::UnresolvedMemberExprClass:
   case Expr::TypoExprClass:
+  case Expr::RecoveryExprClass:
     // FIXME: Can any of the above throw?  If so, when?
     return CT_Cannot;
 
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/NonTrivialTypeVisitor.h"
 #include "clang/AST/StmtCXX.h"
@@ -11214,6 +11215,7 @@
 
 bool Sema::DeduceVariableDeclarationType(VarDecl *VDecl, bool DirectInit,
                                          Expr *Init) {
+  assert(!Init || !Init->containsErrors());
   QualType DeducedType = deduceVarTypeFromInitializer(
       VDecl, VDecl->getDeclName(), VDecl->getType(), VDecl->getTypeSourceInfo(),
       VDecl->getSourceRange(), DirectInit, Init);
@@ -11545,7 +11547,7 @@
     // be deduced based on the chosen correction if the original init contains a
     // TypoExpr.
     ExprResult Res = CorrectDelayedTyposInExpr(Init, VDecl);
-    if (!Res.isUsable()) {
+    if (!Res.isUsable() || Res.get()->containsErrors()) {
       RealDecl->setInvalidDecl();
       return;
     }
Index: clang/lib/Parse/ParseExpr.cpp
===================================================================
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -498,13 +498,25 @@
                          SourceRange(Actions.getExprRange(LHS.get()).getBegin(),
                                      Actions.getExprRange(RHS.get()).getEnd()));
 
-        LHS = Actions.ActOnBinOp(getCurScope(), OpToken.getLocation(),
-                                 OpToken.getKind(), LHS.get(), RHS.get());
-
+        ExprResult BinOp =
+            Actions.ActOnBinOp(getCurScope(), OpToken.getLocation(),
+                               OpToken.getKind(), LHS.get(), RHS.get());
+        if (BinOp.isInvalid())
+          BinOp = Actions.ActOnSemanticError(LHS.get()->getBeginLoc(),
+                                             RHS.get()->getEndLoc(),
+                                             {LHS.get(), RHS.get()});
+
+        LHS = BinOp;
       } else {
-        LHS = Actions.ActOnConditionalOp(OpToken.getLocation(), ColonLoc,
-                                         LHS.get(), TernaryMiddle.get(),
-                                         RHS.get());
+        ExprResult CondOp = Actions.ActOnConditionalOp(
+            OpToken.getLocation(), ColonLoc, LHS.get(), TernaryMiddle.get(),
+            RHS.get());
+        if (CondOp.isInvalid())
+          CondOp = Actions.ActOnSemanticError(
+              LHS.get()->getBeginLoc(), RHS.get()->getEndLoc(),
+              {LHS.get(), TernaryMiddle.get(), RHS.get()});
+
+        LHS = CondOp;
       }
       // In this case, ActOnBinOp or ActOnConditionalOp performed the
       // CorrectDelayedTyposInExpr check.
@@ -1142,9 +1154,14 @@
       UnconsumeToken(SavedTok);
       return ExprError();
     }
-    if (!Res.isInvalid())
+    if (!Res.isInvalid()) {
+      Expr *Arg = Res.get();
       Res = Actions.ActOnUnaryOp(getCurScope(), SavedTok.getLocation(),
-                                 SavedKind, Res.get());
+                                 SavedKind, Arg);
+      if (Res.isInvalid())
+        Res = Actions.ActOnSemanticError(SavedTok.getLocation(),
+                                         Arg->getEndLoc(), Arg);
+    }
     return Res;
   }
   case tok::amp: {         // unary-expression: '&' cast-expression
@@ -1152,8 +1169,13 @@
     SourceLocation SavedLoc = ConsumeToken();
     PreferredType.enterUnary(Actions, Tok.getLocation(), tok::amp, SavedLoc);
     Res = ParseCastExpression(false, true);
-    if (!Res.isInvalid())
-      Res = Actions.ActOnUnaryOp(getCurScope(), SavedLoc, SavedKind, Res.get());
+    if (!Res.isInvalid()) {
+      Expr *Arg = Res.get();
+      Res = Actions.ActOnUnaryOp(getCurScope(), SavedLoc, SavedKind, Arg);
+      if (Res.isInvalid())
+        Res = Actions.ActOnSemanticError(Tok.getLocation(), Arg->getEndLoc(),
+                                         Arg);
+    }
     return Res;
   }
 
@@ -1167,8 +1189,12 @@
     SourceLocation SavedLoc = ConsumeToken();
     PreferredType.enterUnary(Actions, Tok.getLocation(), SavedKind, SavedLoc);
     Res = ParseCastExpression(false);
-    if (!Res.isInvalid())
-      Res = Actions.ActOnUnaryOp(getCurScope(), SavedLoc, SavedKind, Res.get());
+    if (!Res.isInvalid()) {
+      Expr *Arg = Res.get();
+      Res = Actions.ActOnUnaryOp(getCurScope(), SavedLoc, SavedKind, Arg);
+      if (Res.isInvalid())
+        Res = Actions.ActOnSemanticError(SavedLoc, Arg->getEndLoc(), Arg);
+    }
     return Res;
   }
 
@@ -1717,11 +1743,17 @@
         LHS = ExprError();
       } else {
         assert((ArgExprs.size() == 0 ||
-                ArgExprs.size()-1 == CommaLocs.size())&&
-               "Unexpected number of commas!");
-        LHS = Actions.ActOnCallExpr(getCurScope(), LHS.get(), Loc,
-                                    ArgExprs, Tok.getLocation(),
+                ArgExprs.size() - 1 == CommaLocs.size()) &&
+                "Unexpected number of commas!");
+        Expr *Fn = LHS.get();
+        SourceLocation RParLoc = Tok.getLocation();
+        LHS = Actions.ActOnCallExpr(getCurScope(), Fn, Loc, ArgExprs, RParLoc,
                                     ExecConfig);
+        if (LHS.isInvalid()) {
+          ArgExprs.insert(ArgExprs.begin(), Fn);
+          LHS =
+              Actions.ActOnSemanticError(Fn->getBeginLoc(), RParLoc, ArgExprs);
+        }
         PT.consumeClose();
       }
 
@@ -1844,8 +1876,12 @@
     case tok::plusplus:    // postfix-expression: postfix-expression '++'
     case tok::minusminus:  // postfix-expression: postfix-expression '--'
       if (!LHS.isInvalid()) {
+        Expr *Arg = LHS.get();
         LHS = Actions.ActOnPostfixUnaryOp(getCurScope(), Tok.getLocation(),
-                                          Tok.getKind(), LHS.get());
+                                          Tok.getKind(), Arg);
+        if (LHS.isInvalid())
+          LHS = Actions.ActOnSemanticError(Arg->getBeginLoc(),
+                                           Tok.getLocation(), Arg);
       }
       ConsumeToken();
       break;
Index: clang/lib/AST/TextNodeDumper.cpp
===================================================================
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -733,6 +733,11 @@
   }
 }
 
+void TextNodeDumper::VisitRecoveryExpr(const RecoveryExpr *Node) {
+  OS << " "
+     << "<recovery-expr>";
+}
+
 void TextNodeDumper::VisitUnresolvedLookupExpr(
     const UnresolvedLookupExpr *Node) {
   OS << " (";
Index: clang/lib/AST/StmtProfile.cpp
===================================================================
--- clang/lib/AST/StmtProfile.cpp
+++ clang/lib/AST/StmtProfile.cpp
@@ -1899,6 +1899,8 @@
   VisitExpr(E);
 }
 
+void StmtProfiler::VisitRecoveryExpr(const RecoveryExpr *E) { VisitExpr(E); }
+
 void StmtProfiler::VisitObjCStringLiteral(const ObjCStringLiteral *S) {
   VisitExpr(S);
 }
Index: clang/lib/AST/StmtPrinter.cpp
===================================================================
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2401,6 +2401,20 @@
   llvm_unreachable("Cannot print TypoExpr nodes");
 }
 
+void StmtPrinter::VisitRecoveryExpr(RecoveryExpr *Node) {
+  OS << "<recovery-expr>(";
+  const char *Sep = "";
+  for (Stmt *S : Node->children()) {
+    OS << Sep;
+    if (Expr *E = dyn_cast<Expr>(S))
+      PrintExpr(E);
+    else
+      PrintStmt(S);
+    Sep = ", ";
+  }
+  OS << ')';
+}
+
 void StmtPrinter::VisitAsTypeExpr(AsTypeExpr *Node) {
   OS << "__builtin_astype(";
   PrintExpr(Node->getSrcExpr());
Index: clang/lib/AST/ItaniumMangle.cpp
===================================================================
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3623,7 +3623,8 @@
   case Expr::LambdaExprClass:
   case Expr::MSPropertyRefExprClass:
   case Expr::MSPropertySubscriptExprClass:
-  case Expr::TypoExprClass:  // This should no longer exist in the AST by now.
+  case Expr::TypoExprClass: // This should no longer exist in the AST by now.
+  case Expr::RecoveryExprClass:
   case Expr::OMPArraySectionExprClass:
   case Expr::CXXInheritedCtorInitExprClass:
     llvm_unreachable("unexpected statement kind");
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13846,6 +13846,7 @@
   case Expr::CXXPseudoDestructorExprClass:
   case Expr::UnresolvedLookupExprClass:
   case Expr::TypoExprClass:
+  case Expr::RecoveryExprClass:
   case Expr::DependentScopeDeclRefExprClass:
   case Expr::CXXConstructExprClass:
   case Expr::CXXInheritedCtorInitExprClass:
Index: clang/lib/AST/ExprClassification.cpp
===================================================================
--- clang/lib/AST/ExprClassification.cpp
+++ clang/lib/AST/ExprClassification.cpp
@@ -129,6 +129,7 @@
   case Expr::UnresolvedLookupExprClass:
   case Expr::UnresolvedMemberExprClass:
   case Expr::TypoExprClass:
+  case Expr::RecoveryExprClass:
   case Expr::DependentCoawaitExprClass:
   case Expr::CXXDependentScopeMemberExprClass:
   case Expr::DependentScopeDeclRefExprClass:
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2574,6 +2574,7 @@
   // If we don't know precisely what we're looking at, let's not warn.
   case UnresolvedLookupExprClass:
   case CXXUnresolvedConstructExprClass:
+  case RecoveryExprClass:
     return false;
 
   case CXXTemporaryObjectExprClass:
@@ -3378,6 +3379,7 @@
   case SubstNonTypeTemplateParmPackExprClass:
   case FunctionParmPackExprClass:
   case TypoExprClass:
+  case RecoveryExprClass:
   case CXXFoldExprClass:
     llvm_unreachable("shouldn't see dependent / unresolved nodes here");
 
@@ -4698,3 +4700,39 @@
   }
   return OriginalTy;
 }
+
+RecoveryExpr::RecoveryExpr(ASTContext &Ctx, SourceLocation BeginLoc,
+                           SourceLocation EndLoc, ArrayRef<Expr *> SubExprs)
+    : Expr(RecoveryExprClass, Ctx.DependentTy, VK_RValue, OK_Ordinary,
+           /*IsTypeDependent=*/true,
+           /*IsValueDependent=*/true,
+           /*IsInstantiationDependent=*/true,
+           /*ContainsUnexpandedParameterPack=*/false, /*ContainsErrors=*/true),
+      BeginLoc(BeginLoc), EndLoc(EndLoc), NumExprs(SubExprs.size()) {
+#ifndef NDEBUG
+  for (auto *E : SubExprs)
+    assert(E != nullptr);
+#endif
+
+  llvm::copy(SubExprs, getTrailingObjects<Stmt *>());
+  for (auto *E : SubExprs) {
+    if (E->containsUnexpandedParameterPack()) {
+      setContainsUnexpandedParameterPack();
+      break;
+    }
+  }
+}
+
+RecoveryExpr *RecoveryExpr::Create(ASTContext &Ctx, SourceLocation BeginLoc,
+                                   SourceLocation EndLoc,
+                                   ArrayRef<Expr *> SubExprs) {
+  void *Mem = Ctx.Allocate(totalSizeToAlloc<Stmt *>(SubExprs.size()),
+                           alignof(RecoveryExpr));
+  return new (Mem) RecoveryExpr(Ctx, BeginLoc, EndLoc, SubExprs);
+}
+
+RecoveryExpr *RecoveryExpr::CreateEmpty(ASTContext &Ctx, unsigned NumStmts) {
+  void *Mem =
+      Ctx.Allocate(totalSizeToAlloc<Stmt *>(NumStmts), alignof(RecoveryExpr));
+  return new (Mem) RecoveryExpr(EmptyShell());
+}
Index: clang/include/clang/Serialization/ASTBitCodes.h
===================================================================
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1759,6 +1759,9 @@
       /// An AtomicExpr record.
       EXPR_ATOMIC,
 
+      /// A RecoveryExpr record.
+      EXPR_RECOVERY,
+
       // Objective-C
 
       /// An ObjCStringLiteral record.
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -34,6 +34,7 @@
 #include "clang/Basic/Module.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/PragmaKinds.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TemplateKinds.h"
 #include "clang/Basic/TypeTraits.h"
@@ -3562,6 +3563,11 @@
   void DiagnoseAmbiguousLookup(LookupResult &Result);
   //@}
 
+  /// Attempts to produce a RecoveryExpr after some AST node cannot be created.
+  /// Corrects typos inside \p SubExprs.
+  ExprResult ActOnSemanticError(SourceLocation Begin, SourceLocation End,
+                                ArrayRef<Expr *> SubExprs);
+
   ObjCInterfaceDecl *getObjCInterfaceDecl(IdentifierInfo *&Id,
                                           SourceLocation IdLoc,
                                           bool TypoCorrection = false);
Index: clang/include/clang/Basic/StmtNodes.td
===================================================================
--- clang/include/clang/Basic/StmtNodes.td
+++ clang/include/clang/Basic/StmtNodes.td
@@ -192,6 +192,7 @@
 def BlockExpr : DStmt<Expr>;
 def OpaqueValueExpr : DStmt<Expr>;
 def TypoExpr : DStmt<Expr>;
+def RecoveryExpr : DStmt<Expr>;
 def BuiltinBitCastExpr : DStmt<ExplicitCastExpr>;
 
 // Microsoft Extensions.
Index: clang/include/clang/AST/TextNodeDumper.h
===================================================================
--- clang/include/clang/AST/TextNodeDumper.h
+++ clang/include/clang/AST/TextNodeDumper.h
@@ -260,6 +260,7 @@
   void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *Node);
   void VisitExprWithCleanups(const ExprWithCleanups *Node);
   void VisitUnresolvedLookupExpr(const UnresolvedLookupExpr *Node);
+  void VisitRecoveryExpr(const RecoveryExpr *Node);
   void VisitSizeOfPackExpr(const SizeOfPackExpr *Node);
   void
   VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *Node);
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===================================================================
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2606,6 +2606,7 @@
 DEF_TRAVERSE_STMT(CXXOperatorCallExpr, {})
 DEF_TRAVERSE_STMT(OpaqueValueExpr, {})
 DEF_TRAVERSE_STMT(TypoExpr, {})
+DEF_TRAVERSE_STMT(RecoveryExpr, {})
 DEF_TRAVERSE_STMT(CUDAKernelCallExpr, {})
 
 // These operators (all of them) do not need any action except
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -5978,6 +5978,66 @@
   }
 
 };
+
+/// Frontend produces RecoveryExprs on semantic errors that prevent creating
+/// other well-formed expressions. E.g. when type-checking of a binary operator
+/// fails, we cannot produce a BinaryOperator expression. Instead, we can choose
+/// to produce a recovery expression storing left and right operands.
+///
+/// RecoveryExpr does not have any semantic meaning in C++, it is only useful to
+/// preserve expressions in AST that would otherwise be dropped. It captures
+/// subexpressions of some expression that we could not construct and source
+/// range covered by the expression.
+///
+/// RecoveryExpr is type-, value- and instantiation-dependent to take advantage
+/// of existing machinery to deal with dependent code in C++, e.g. RecoveryExpr
+/// is preserved in `decltype(<broken-expr>)` as part of the
+/// `DependentDecltypeType`. In addition to that, clang does not report most
+/// errors on dependent expressions, so we get rid of bogus errors for free.
+/// However, note that unlike other dependent expressions, RecoveryExpr can be
+/// produced in non-template contexts.
+///
+/// One can also reliably suppress all bogus errors on expressions containing
+/// recovery expressions by examining results of Expr::containsErrors().
+class RecoveryExpr final : public Expr,
+                           private llvm::TrailingObjects<RecoveryExpr, Stmt *> {
+public:
+  static RecoveryExpr *Create(ASTContext &Ctx, SourceLocation BeginLoc,
+                              SourceLocation EndLoc, ArrayRef<Expr *> SubExprs);
+  static RecoveryExpr *CreateEmpty(ASTContext &Ctx, unsigned NumStmts);
+
+  child_range children() {
+    const_child_range CCR = const_cast<const RecoveryExpr *>(this)->children();
+    return child_range(cast_away_const(CCR.begin()),
+                       cast_away_const(CCR.end()));
+  }
+  const_child_range children() const {
+    Stmt *const *CS = const_cast<Stmt *const *>(
+        reinterpret_cast<const Stmt *const *>(getTrailingObjects<Stmt *>()));
+    return const_child_range(CS, CS + NumExprs);
+  }
+
+  SourceLocation getBeginLoc() const { return BeginLoc; }
+  SourceLocation getEndLoc() const { return EndLoc; }
+
+  static bool classof(const Stmt *T) {
+    return T->getStmtClass() == RecoveryExprClass;
+  }
+
+private:
+  RecoveryExpr(ASTContext &Ctx, SourceLocation BeginLoc, SourceLocation EndLoc,
+               ArrayRef<Expr *> SubExprs);
+  RecoveryExpr(EmptyShell Empty) : Expr(RecoveryExprClass, Empty) {}
+
+  size_t numTrailingObjects(OverloadToken<Stmt *>) const { return NumExprs; }
+
+  SourceLocation BeginLoc, EndLoc;
+  unsigned NumExprs;
+  friend TrailingObjects;
+  friend class ASTStmtReader;
+  friend class ASTStmtWriter;
+};
+
 } // end namespace clang
 
 #endif // LLVM_CLANG_AST_EXPR_H
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D69330: [... Ilya Biryukov via Phabricator via cfe-commits

Reply via email to