[AST] Add RecoveryExpr to retain expressions on semantic errors

[hokein]

Summary

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 Expr::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

[rsmith]

I would prefer that we have dedicated tests for them rather than scattering the tests throughout the existing test suite (for example, consider adding -Wno-unused to the tests producing "result unused" warnings and adding a dedicated test).

clang/include/clang/Sema/Sema.h

3724–3725

Generally the ActOn* interface is invoked by the parser in response to syntactic constructs, so ActOnSemanticError is a surprising function name. Maybe CreateRecoveryExpr would be better. Is that too narrow for the intended semantics of this function?

clang/lib/AST/TextNodeDumper.cpp

736–739 ↗

(On Diff #226105)

You shouldn't need to print anything here. The class name is automatically printed for you.

clang/lib/Sema/SemaExceptionSpec.cpp

1358

Should we return CT_Dependent for RecoveryExprClass, since we model it as being dependent?

clang/lib/Sema/SemaExpr.cpp

18154

We shouldn't create these in code synthesis contexts (eg, during SFINAE checks in template instantiations).

clang/lib/Sema/TreeTransform.h

9591

We should either transform the subexpressions or just return ExprError() here. With this approach, we can violate AST invariants (eg, by having the same Expr reachable through two different code paths in the same function body, or by retaining DeclRefExprs referring to declarations from the wrong context, etc).

clang/test/SemaCXX/builtin-operator-new-delete.cpp

94 ↗

(On Diff #226105)

What happens to the original testcase here? I wouldn't expect the invalid call expression in the initializer of p to affect whether we diagnose uses of p.

(Nit: remove spaces to realign the comments here.)

clang/test/SemaOpenCLCXX/address-space-references.cl

15

Consider just adding the missing semicolon, so that this test is only testing the thing it aims to test.

clang/test/SemaTemplate/instantiate-init.cpp

102

Add a cast-to-void, like on the previous line.

[ilya-biryukov]

In D69330#1746137, @rsmith wrote:

I would prefer that we have dedicated tests for them rather than scattering the tests throughout the existing test suite (for example, consider adding -Wno-unused to the tests producing "result unused" warnings and adding a dedicated test).

Most updated tests (including those with more "result unused" warnings) are actually not intended to test recovery expressions, they just happen to produce different results now and need to be updated.
The only new dedicated tests here are clang/test/AST/ast-dump-recovery.cpp and clang/test/Index/getcursor-recovery.cpp.

Could technically move them into the same directory, but wanted to make sure I got your point first. Could you elaborate on what testing strategy you'd prefer? Also eager for suggestions on more things I could test, but not sure what kinds of tests people normally add when adding new expressions. Any advice here is highly appreciated.

clang/include/clang/Sema/Sema.h

3724–3725

CreateRecoveryExpr looks good, the only additional logic in the function is not producing recovery expressions in the C mode. But I would expect this to go away in the future.

clang/lib/Sema/SemaExceptionSpec.cpp

1358

Makes total sense, thanks!

clang/lib/Sema/SemaExpr.cpp

18154

Good point, thanks. Added the corresponding check. Also made sure that we always typo-correct in arguments before checking whether we can produce recovery expressions.

I don't think we have any code that calls into CreateRecoveryExpression (formerly ActOnSemanticError) from SFINAE context, all calls are currently in the parser. Not 100% certain this does not happen with -fdelayed-template-parsing, though, so ended up returning null instead of asserting we're not in SFINAE context.

clang/lib/Sema/TreeTransform.h

9591

Thanks, I just blindly copied what TypoExpr was doing without considering the consequences here.

Added the code to rebuild RecoveryExpr.

Any good way to test this?

clang/test/SemaCXX/builtin-operator-new-delete.cpp

94 ↗

(On Diff #226105)

Sorry, that was a leftover from previous revisions. This was here by accident, I reverted this.

A bit more context on what happened here: if we mark any decl as invalid, references to those variables do not parse into DeclRefExpr, the corresponding sema function returns null instead.
This leads to diagnostics like this being lost. In one of my attempts, I marked more decls as invalid and, in turn, to these tests failing.

clang/test/SemaOpenCLCXX/address-space-references.cl

15

Good point, thanks.

clang/test/SemaTemplate/instantiate-init.cpp

102

Done. Do you think it's also worth suppressing same errors in other files (e.g. cxx1z-copy-omission.cpp) in a similar manner?

Or would you prefer adding -Wno-unused-result for tests like cxx1x-copy-omission.cpp?

[ilya-biryukov]

• Rename ActOnSemanticError to CreateRecoveryExpr
• Do not print anything for recovery expr in TextNodeDumper
• canThrow(RecoveryExpr) now returns CT_Dependent
• Store Expr* instead of Stmt* in RecoveryExpr
• Transform subexpressions of RecoveryExpr in TreeTransform
• Do not produce RecoveryExpr in SFINAE contexts
• Updated some test cases

[rsmith]

In D69330#1752089, @ilya-biryukov wrote:

In D69330#1746137, @rsmith wrote:

I would prefer that we have dedicated tests for them rather than scattering the tests throughout the existing test suite (for example, consider adding -Wno-unused to the tests producing "result unused" warnings and adding a dedicated test).

Most updated tests (including those with more "result unused" warnings) are actually not intended to test recovery expressions, they just happen to produce different results now and need to be updated.
The only new dedicated tests here are clang/test/AST/ast-dump-recovery.cpp and clang/test/Index/getcursor-recovery.cpp.

Could technically move them into the same directory, but wanted to make sure I got your point first. Could you elaborate on what testing strategy you'd prefer?

For the tests where you added expected-warning {{unused}} that are testing things other than parser recovery, I would instead add -Wno-unused or a cast to void. expects unrelated to the intent of tests make them less readable and more fragile.

I think it's fine to use test/AST/ast-dump-recovery.cpp as the primary test for the various forms of recovery that you added here. I've not thought of anything better, at least :)

clang/lib/Sema/TreeTransform.h

9591

Hmm, testing that the old failure mode doesn't happen seems a bit tricky; any test for that is going to be testing second-order effects of the code under test, so will be fragile. But you can test that we're substituting into the RecoveryExpr easily enough: put some expression for which substitution will fail into a context where we'll build a RecoveryExpr inside a context that we'll TreeTransform. For example, maybe:

template<typename T> int *p = &void(T::error); // should produce "cannot take address of void"
int *q = p<int>; // should produce "'int' cannot be used prior to '::'" in instantiation

[ilya-biryukov]

• Do not correct typos when creating recovery expr
• Simplify StmtPrinter::VisitRecoveryExpr
• Add -Wno-used and remove extra expect-warning directives

[ilya-biryukov]

@rsmith, could you also take a look at D65591?
It's really important to have the containsError() check in this patch that marks decls with undeduced types as invalid. Otherwise, they would have a "dependent auto type" and the constant evaluation code will fail when attempting to get sizes of those, e.g. when they're used inside sizeof().
There's actually a test in clang that crashes if this is not done (clang/test/SemaCXX/lambda-expressions.cpp)

I have also realized doing typo-correction in CreateRecoveryExpr is a bad idea, the contract for typo correction is to run it at particular points (e.g. on full expressions) and it's unclear why recovery expressions should be another case for this context. All code attempting to create recovery expressions should already handle typo correction properly, so there's no need to cross these two features.

[ilya-biryukov]

• Rebase

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[ilya-biryukov]

Fix compilation after rebase

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[hokein]

• rebase to master, fix more broken tests
  • preferType in CodeCompletTest
  • openMP diagnostics target_update_from_messages.cpp, target_update_to_messages.cpp
• RecoveryExpression RValue => LValue
• Fix a NULL assertion crash on gnu conditional operator ?:

[hokein]

rebase

[hokein]

all prerequisite patches have been landed, it is time to land this patch now. Would be nice to have a second look before committing it +@sammccall.

[sammccall]

Fantastic :-)

@rsmith and others: would appreciate feedback on giving this a feature-flag (to decouple from LangOpts.CPlusPlus):

• whether it's OK to have one
• whether it should be cc1-only
• the name -f[no]recovery-ast

clang/include/clang/AST/Expr.h

5910

AIUI this should be "for now" with the goal of eliminating the use of template dependence concepts, right?

5925

nit: NumStmts -> NumSubExprs?

clang/lib/AST/ComputeDependence.cpp

350

Probably worth echoing with a FIXME: drop type+value+instantiation once Error is sufficient to suppress checks.

clang/lib/Sema/SemaExpr.cpp

18150

I think we should strongly consider a LangOption with an associated flag. (e.g. LangOptions.RecoveryAST, -f[no]recovery-ast).
If we're going to pay tho cost of letting expr creation fail, we might as well use it
Use cases:

• Control rollout: we can check this in without (yet) flipping the flag on for all tools at once, if desired. If we flip the default and it causes problems for particular tools, we can turn it off for that tool rather than rolling the whole thing back.
• turn on and off in lit tests to precisely test behavior and avoid dependence on defaults
• allow incremental work on recovery in !CPlusPlus mode

If this makes sense to you, I'd suggest setting the default to off in this patch (and including some tests that pass -frecovery-ast), and then immediately following up with a patch that flips the default to on-for-C++.
This separation makes like easier for everyone if turning this on breaks something.

A bunch of the updates to existing tests would be deferred until that patch.

clang/lib/Sema/TreeTransform.h

9591

@hokein I don't think this test was added yet.

[hokein]

address some comments, add missing tree-transform testcase.

[hokein]

clang/include/clang/AST/Expr.h

5910

yeah, I think so.

clang/lib/Sema/SemaExpr.cpp

18150

+1, I think this is a good idea -- particularly letting us incrementally working on it without breaking existing tools, I was highly suspected that this patch will fail internal tests during build copping.

I will wait for a while for other feedback @rsmith before making the actual change.

clang/lib/Sema/TreeTransform.h

9591

good catch, done.

[hokein]

• add -frecovery-ast cc1 option
• defer changes for existing tests.

[sammccall]

clang/include/clang/Basic/LangOptions.def

151 ↗

(On Diff #252046)

this should be COMPATIBLE_LANGOPT I believe - it affects semantics but not in a breaking way.

clang/include/clang/Driver/CC1Options.td

569 ↗

(On Diff #252046)

We'll want the positive and negative version of this flag because the default will be "sometimes". See fpadding_on_unsigned_fixed_point for such a pair.

This could be deferred to the next patch (as the default is false for now) but seems cleaner to introduce them together.

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

2

This is probably a good test to add a second run line with -fno-recovery-ast and FileCheck -check-prefix=DISABLED to test that the flag controls the behaviour.

[hokein]

add -fno-recovery-ast, and negative tests.

[erichkeane]

I wasn't able to add you to the bug report, but I discovered this: https://bugs.llvm.org/show_bug.cgi?id=46837

I'm still working on it, but if you have a suggestion, I'd appreciate it.

clang/include/clang/AST/ComputeDependence.h

 class NoInitExpr;
 class ArrayInitLoopExpr;
 class ImplicitValueInitExpr;
 class InitListExpr;
 class ExtVectorElementExpr;
 class BlockExpr;
 class AsTypeExpr;
 class DeclRefExpr;
+class RecoveryExpr;
 class CXXRewrittenBinaryOperator;
 class CXXStdInitializerListExpr;
 class CXXTypeidExpr;
 class MSPropertyRefExpr;
 class MSPropertySubscriptExpr;
 class CXXUuidofExpr;
 class CXXThisExpr;
 class CXXThrowExpr;

 ExprDependence computeDependence(NoInitExpr *E);
 ExprDependence computeDependence(ArrayInitLoopExpr *E);
 ExprDependence computeDependence(ImplicitValueInitExpr *E);
 ExprDependence computeDependence(InitListExpr *E);
 ExprDependence computeDependence(ExtVectorElementExpr *E);
 ExprDependence computeDependence(BlockExpr *E);
 ExprDependence computeDependence(AsTypeExpr *E);
 ExprDependence computeDependence(DeclRefExpr *E, const ASTContext &Ctx);
+ExprDependence computeDependence(RecoveryExpr *E);
 ExprDependence computeDependence(CXXRewrittenBinaryOperator *E);
 ExprDependence computeDependence(CXXStdInitializerListExpr *E);
 ExprDependence computeDependence(CXXTypeidExpr *E);
 ExprDependence computeDependence(MSPropertyRefExpr *E);
 ExprDependence computeDependence(MSPropertySubscriptExpr *E);
 ExprDependence computeDependence(CXXUuidofExpr *E);
 ExprDependence computeDependence(CXXThisExpr *E);
 ExprDependence computeDependence(CXXThrowExpr *E);

clang/include/clang/AST/Expr.h

public:
   SourceLocation getBeginLoc() const LLVM_READONLY { return SourceLocation(); }
   SourceLocation getEndLoc() const LLVM_READONLY { return SourceLocation(); }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == TypoExprClass;
   }
 
 };
+
+/// 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, Expr *> {
+public:
+  static RecoveryExpr *Create(ASTContext &Ctx, SourceLocation BeginLoc,
+                              SourceLocation EndLoc, ArrayRef<Expr *> SubExprs);
+  static RecoveryExpr *CreateEmpty(ASTContext &Ctx, unsigned NumStmts);
+
+  ArrayRef<Expr *> subExpressions() {
+    auto *B = getTrailingObjects<Expr *>();
+    return llvm::makeArrayRef(B, B + NumExprs);
+  }
+
+  ArrayRef<const Expr *> subExpressions() const {
+    return const_cast<RecoveryExpr *>(this)->subExpressions();
+  }
+
+  child_range children() {
+    Stmt **B = reinterpret_cast<Stmt **>(getTrailingObjects<Expr *>());
+    return child_range(B, B + 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

clang/include/clang/AST/RecursiveASTVisitor.h

CXXRewrittenBinaryOperator::DecomposedForm Decomposed =
         S->getDecomposedForm();
     TRY_TO(TraverseStmt(const_cast<Expr*>(Decomposed.LHS)));
     TRY_TO(TraverseStmt(const_cast<Expr*>(Decomposed.RHS)));
     ShouldVisitChildren = false;
   }
 })
 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
 // iterating over the children.
 DEF_TRAVERSE_STMT(BinaryConditionalOperator, {})
 DEF_TRAVERSE_STMT(ConditionalOperator, {})
 DEF_TRAVERSE_STMT(UnaryOperator, {})
 DEF_TRAVERSE_STMT(BinaryOperator, {})

clang/include/clang/Basic/StmtNodes.td

 def CUDAKernelCallExpr : StmtNode<CallExpr>;
 
 // Clang Extensions.
 def ShuffleVectorExpr : StmtNode<Expr>;
 def ConvertVectorExpr : StmtNode<Expr>;
 def BlockExpr : StmtNode<Expr>;
 def OpaqueValueExpr : StmtNode<Expr>;
 def TypoExpr : StmtNode<Expr>;
+def RecoveryExpr : StmtNode<Expr>;
 def BuiltinBitCastExpr : StmtNode<ExplicitCastExpr>;
 
 // Microsoft Extensions.
 def MSPropertyRefExpr : StmtNode<Expr>;
 def MSPropertySubscriptExpr : StmtNode<Expr>;
 def CXXUuidofExpr : StmtNode<Expr>;
 def SEHTryStmt : StmtNode<Stmt>;
 def SEHExceptStmt : StmtNode<Stmt>;

clang/include/clang/Sema/Sema.h

 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeOrdering.h"
 #include "clang/Basic/BitmaskEnum.h"
 #include "clang/Basic/ExpressionTraits.h"
 #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"
 #include "clang/Sema/AnalysisBasedWarnings.h"
 #include "clang/Sema/CleanupInfo.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/ExternalSemaSource.h"
 #include "clang/Sema/IdentifierResolver.h"
public:
   void FilterLookupForScope(LookupResult &R, DeclContext *Ctx, Scope *S,
                             bool ConsiderLinkage, bool AllowInlineNamespace);
 
   bool CheckRedeclarationModuleOwnership(NamedDecl *New, NamedDecl *Old);
 
   void DiagnoseAmbiguousLookup(LookupResult &Result);
   //@}
 
+  /// Attempts to produce a RecoveryExpr after some AST node cannot be created.
+  ExprResult CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
+                                ArrayRef<Expr *> SubExprs);
+
   ObjCInterfaceDecl *getObjCInterfaceDecl(IdentifierInfo *&Id,
                                           SourceLocation IdLoc,
                                           bool TypoCorrection = false);
   NamedDecl *LazilyCreateBuiltin(IdentifierInfo *II, unsigned ID,
                                  Scope *S, bool ForRedeclaration,
                                  SourceLocation Loc);
   NamedDecl *ImplicitlyDefineFunction(SourceLocation Loc, IdentifierInfo &II,
                                       Scope *S);

clang/include/clang/Serialization/ASTBitCodes.h

enum StmtCode {
       EXPR_GENERIC_SELECTION,
 
       /// A PseudoObjectExpr record.
       EXPR_PSEUDO_OBJECT,
 
       /// An AtomicExpr record.
       EXPR_ATOMIC,
 
+      /// A RecoveryExpr record.
+      EXPR_RECOVERY,
+
       // Objective-C
 
       /// An ObjCStringLiteral record.
       EXPR_OBJC_STRING_LITERAL,
 
       EXPR_OBJC_BOXED_EXPRESSION,
       EXPR_OBJC_ARRAY_LITERAL,
       EXPR_OBJC_DICTIONARY_LITERAL,

clang/lib/AST/ComputeDependence.cpp

ExprDependence clang::computeDependence(DeclRefExpr *E, const ASTContext &Ctx) {
   // (VD) - FIXME: Missing from the standard:
   //      -  a member function or a static data member of the current
   //         instantiation
   if (isa<CXXMethodDecl>(Decl) && Decl->getDeclContext()->isDependentContext())
     Deps |= ExprDependence::ValueInstantiation;
   return Deps;
 }
 
+ExprDependence clang::computeDependence(RecoveryExpr *E) {
+  auto Deps = ExprDependence::TypeValueInstantiation | ExprDependence::Error;
+  for (auto *S : E->subExpressions())
+    Deps |= S->getDependence();
+  return Deps;
+}
+
 ExprDependence clang::computeDependence(CXXRewrittenBinaryOperator *E) {
   return E->getSemanticForm()->getDependence();
 }
 
 ExprDependence clang::computeDependence(CXXStdInitializerListExpr *E) {
   auto D = E->getSubExpr()->getDependence() & ~ExprDependence::Type;
   if (E->getType()->isDependentType())
     D |= ExprDependence::Type;
ExprDependence clang::computeDependence(ObjCMessageExpr *E) {
   if (auto *R = E->getInstanceReceiver())
     D |= R->getDependence();
   else
     D |= toExprDependence(E->getType()->getDependence());
   for (auto *A : E->arguments())
     D |= A->getDependence();
   return D;
 }
 No newline at end of file

clang/lib/AST/Expr.cpp

if (const Decl *FD = CE->getCalleeDecl()) {
       }
     }
     return false;
   }
 
   // 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:
   case CXXConstructExprClass: {
     if (const CXXRecordDecl *Type = getType()->getAsCXXRecordDecl()) {
       const auto *WarnURAttr = Type->getAttr<WarnUnusedResultAttr>();
       if (Type->hasAttr<WarnUnusedAttr>() ||
           (WarnURAttr && WarnURAttr->IsCXX11NoDiscard())) {
bool Expr::HasSideEffects(const ASTContext &Ctx,
   case CXXUnresolvedConstructExprClass:
   case CXXDependentScopeMemberExprClass:
   case UnresolvedLookupExprClass:
   case UnresolvedMemberExprClass:
   case PackExpansionExprClass:
   case SubstNonTypeTemplateParmPackExprClass:
   case FunctionParmPackExprClass:
   case TypoExprClass:
+  case RecoveryExprClass:
   case CXXFoldExprClass:
     llvm_unreachable("shouldn't see dependent / unresolved nodes here");
 
   case DeclRefExprClass:
   case ObjCIvarRefExprClass:
   case PredefinedExprClass:
   case IntegerLiteralClass:
   case FixedPointLiteralClass:
if (OriginalTy->isAnyPointerType())
       OriginalTy = OriginalTy->getPointeeType();
     else {
       assert (OriginalTy->isArrayType());
       OriginalTy = OriginalTy->castAsArrayTypeUnsafe()->getElementType();
     }
   }
   return OriginalTy;
 }
+
+RecoveryExpr::RecoveryExpr(ASTContext &Ctx, SourceLocation BeginLoc,
+                           SourceLocation EndLoc, ArrayRef<Expr *> SubExprs)
+    : Expr(RecoveryExprClass, Ctx.DependentTy, VK_RValue, OK_Ordinary),
+      BeginLoc(BeginLoc), EndLoc(EndLoc), NumExprs(SubExprs.size()) {
+#ifndef NDEBUG
+  for (auto *E : SubExprs)
+    assert(E != nullptr);
+#endif
+
+  llvm::copy(SubExprs, getTrailingObjects<Expr *>());
+  setDependencies(computeDependence(this));
+}
+
+RecoveryExpr *RecoveryExpr::Create(ASTContext &Ctx, SourceLocation BeginLoc,
+                                   SourceLocation EndLoc,
+                                   ArrayRef<Expr *> SubExprs) {
+  void *Mem = Ctx.Allocate(totalSizeToAlloc<Expr *>(SubExprs.size()),
+                           alignof(RecoveryExpr));
+  return new (Mem) RecoveryExpr(Ctx, BeginLoc, EndLoc, SubExprs);
+}
+
+RecoveryExpr *RecoveryExpr::CreateEmpty(ASTContext &Ctx, unsigned NumStmts) {
+  void *Mem =
+      Ctx.Allocate(totalSizeToAlloc<Expr *>(NumStmts), alignof(RecoveryExpr));
+  return new (Mem) RecoveryExpr(EmptyShell());
+}

clang/lib/AST/ExprClassification.cpp

#include "clang/AST/StmtNodes.inc"
   case Expr::ObjCPropertyRefExprClass:
     // C++ [expr.typeid]p1: The result of a typeid expression is an lvalue of...
   case Expr::CXXTypeidExprClass:
     // Unresolved lookups and uncorrected typos get classified as lvalues.
     // FIXME: Is this wise? Should they get their own kind?
   case Expr::UnresolvedLookupExprClass:
   case Expr::UnresolvedMemberExprClass:
   case Expr::TypoExprClass:
+  case Expr::RecoveryExprClass:
   case Expr::DependentCoawaitExprClass:
   case Expr::CXXDependentScopeMemberExprClass:
   case Expr::DependentScopeDeclRefExprClass:
     // ObjC instance variables are lvalues
     // FIXME: ObjC++0x might have different rules
   case Expr::ObjCIvarRefExprClass:
   case Expr::FunctionParmPackExprClass:
   case Expr::MSPropertyRefExprClass:

clang/lib/AST/ExprConstant.cpp

#include "clang/AST/StmtNodes.inc"
   case Expr::UserDefinedLiteralClass:
   case Expr::CXXThisExprClass:
   case Expr::CXXThrowExprClass:
   case Expr::CXXNewExprClass:
   case Expr::CXXDeleteExprClass:
   case Expr::CXXPseudoDestructorExprClass:
   case Expr::UnresolvedLookupExprClass:
   case Expr::TypoExprClass:
+  case Expr::RecoveryExprClass:
   case Expr::DependentScopeDeclRefExprClass:
   case Expr::CXXConstructExprClass:
   case Expr::CXXInheritedCtorInitExprClass:
   case Expr::CXXStdInitializerListExprClass:
   case Expr::CXXBindTemporaryExprClass:
   case Expr::ExprWithCleanupsClass:
   case Expr::CXXTemporaryObjectExprClass:
   case Expr::CXXUnresolvedConstructExprClass:

clang/lib/AST/ItaniumMangle.cpp

#include "clang/AST/StmtNodes.inc"
   case Expr::ImplicitValueInitExprClass:
   case Expr::ArrayInitLoopExprClass:
   case Expr::ArrayInitIndexExprClass:
   case Expr::NoInitExprClass:
   case Expr::ParenListExprClass:
   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");
 
   case Expr::ConstantExprClass:
     E = cast<ConstantExpr>(E)->getSubExpr();
     goto recurse;
 

clang/lib/AST/StmtPrinter.cpp

void StmtPrinter::VisitOpaqueValueExpr(OpaqueValueExpr *Node) {
   PrintExpr(Node->getSourceExpr());
 }
 
 void StmtPrinter::VisitTypoExpr(TypoExpr *Node) {
   // TODO: Print something reasonable for a TypoExpr, if necessary.
   llvm_unreachable("Cannot print TypoExpr nodes");
 }
 
+void StmtPrinter::VisitRecoveryExpr(RecoveryExpr *Node) {
+  OS << "<recovery-expr>(";
+  const char *Sep = "";
+  for (Expr *E : Node->subExpressions()) {
+    OS << Sep;
+    PrintExpr(E);
+    Sep = ", ";
+  }
+  OS << ')';
+}
+
 void StmtPrinter::VisitAsTypeExpr(AsTypeExpr *Node) {
   OS << "__builtin_astype(";
   PrintExpr(Node->getSrcExpr());
   OS << ", ";
   Node->getType().print(OS, Policy);
   OS << ")";
 }
 

clang/lib/AST/StmtProfile.cpp

 void StmtProfiler::VisitTypoExpr(const TypoExpr *E) {
   VisitExpr(E);
 }
 
 void StmtProfiler::VisitSourceLocExpr(const SourceLocExpr *E) {
   VisitExpr(E);
 }
 
+void StmtProfiler::VisitRecoveryExpr(const RecoveryExpr *E) { VisitExpr(E); }
+
 void StmtProfiler::VisitObjCStringLiteral(const ObjCStringLiteral *S) {
   VisitExpr(S);
 }
 
 void StmtProfiler::VisitObjCBoxedExpr(const ObjCBoxedExpr *E) {
   VisitExpr(E);
 }
 

clang/lib/Parse/ParseExpr.cpp

if (!LHS.isInvalid()) {
         // argument list (in C++98), suggest the addition of
         // parentheses so that the code remains well-formed in C++0x.
         if (!GreaterThanIsOperator && OpToken.is(tok::greatergreater))
           SuggestParentheses(OpToken.getLocation(),
                              diag::warn_cxx11_right_shift_in_template_arg,
                          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(),
+        if (BinOp.isInvalid())
+          BinOp = Actions.CreateRecoveryExpr(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(),
+        if (CondOp.isInvalid())
+          CondOp = Actions.CreateRecoveryExpr(
+              LHS.get()->getBeginLoc(), RHS.get()->getEndLoc(),
+              {LHS.get(), TernaryMiddle.get(), RHS.get()});
+
+        LHS = CondOp;
       }
       // In this case, ActOnBinOp or ActOnConditionalOp performed the
       // CorrectDelayedTyposInExpr check.
       if (!getLangOpts().CPlusPlus)
         continue;
     }
 
     // Ensure potential typos aren't left undiagnosed.
Res = ParseCastExpression(getLangOpts().CPlusPlus ?
                               NotTypeCast);
     if (NotCastExpr) {
       // If we return with NotCastExpr = true, we must not consume any tokens,
       // so put the token back where we found it.
       assert(Res.isInvalid());
       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.CreateRecoveryExpr(SavedTok.getLocation(),
+                                         Arg->getEndLoc(), Arg);
+    }
     return Res;
   }
   case tok::amp: {         // unary-expression: '&' cast-expression
     if (NotPrimaryExpression)
       *NotPrimaryExpression = true;
     // Special treatment because of member pointers
     SourceLocation SavedLoc = ConsumeToken();
     PreferredType.enterUnary(Actions, Tok.getLocation(), tok::amp, SavedLoc);
     Res = ParseCastExpression(AnyCastExpr, 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.CreateRecoveryExpr(Tok.getLocation(), Arg->getEndLoc(),
+                                         Arg);
+    }
     return Res;
   }
 
   case tok::star:          // unary-expression: '*' cast-expression
   case tok::plus:          // unary-expression: '+' cast-expression
   case tok::minus:         // unary-expression: '-' cast-expression
   case tok::tilde:         // unary-expression: '~' cast-expression
   case tok::exclaim:       // unary-expression: '!' cast-expression
   case tok::kw___real:     // unary-expression: '__real' cast-expression [GNU]
   case tok::kw___imag: {   // unary-expression: '__imag' cast-expression [GNU]
     if (NotPrimaryExpression)
       *NotPrimaryExpression = true;
     SourceLocation SavedLoc = ConsumeToken();
     PreferredType.enterUnary(Actions, Tok.getLocation(), SavedKind, SavedLoc);
     Res = ParseCastExpression(AnyCastExpr);
-    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.CreateRecoveryExpr(SavedLoc, Arg->getEndLoc(), Arg);
+    }
     return Res;
   }
 
   case tok::kw_co_await: {  // unary-expression: 'co_await' cast-expression
     if (NotPrimaryExpression)
       *NotPrimaryExpression = true;
     SourceLocation CoawaitLoc = ConsumeToken();
     Res = ParseCastExpression(AnyCastExpr);
case tok::lesslessless: {  // p-e: p-e '<<<' argument-expression-list '>>>'
         // the unmatched l_paren.
         if (HadDelayedTypo)
           SkipUntil(tok::r_paren, StopAtSemi);
         else
           PT.consumeClose();
         LHS = ExprError();
       } else {
         assert((ArgExprs.size() == 0 ||
-                ArgExprs.size()-1 == CommaLocs.size())&&
+                ArgExprs.size() - 1 == CommaLocs.size()) &&
                 "Unexpected number of commas!");
-        LHS = Actions.ActOnCallExpr(getCurScope(), LHS.get(), Loc,
-                                    ArgExprs, Tok.getLocation(),
+        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.CreateRecoveryExpr(Fn->getBeginLoc(), RParLoc, ArgExprs);
+        }
         PT.consumeClose();
       }
 
       break;
     }
     case tok::arrow:
     case tok::period: {
       // postfix-expression: p-e '->' template[opt] id-expression
case tok::period: {
                                                    : nullptr);
       if (!LHS.isInvalid() && Tok.is(tok::less))
         checkPotentialAngleBracket(LHS);
       break;
     }
     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.CreateRecoveryExpr(Arg->getBeginLoc(),
+                                           Tok.getLocation(), Arg);
       }
       ConsumeToken();
       break;
     }
   }
 }
 
 /// ParseExprAfterUnaryExprOrTypeTrait - We parsed a typeof/sizeof/alignof/

clang/lib/Sema/SemaDecl.cpp

 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/CommentDiagnostic.h"
 #include "clang/AST/DeclCXX.h"
 #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"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/HeaderSearch.h" // TODO: Sema shouldn't depend on Lex
if (!inTemplateInstantiation() && !DefaultedAnyToId && !IsInitCapture &&
     Diag(Loc, diag::warn_auto_var_is_id) << VN << Range;
   }
 
   return DeducedType;
 }
 
 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);
   if (DeducedType.isNull()) {
     VDecl->setInvalidDecl();
     return true;
   }
 
void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
   }
 
   // C++11 [decl.spec.auto]p6. Deduce the type which 'auto' stands in for.
   if (VDecl->getType()->isUndeducedType()) {
     // Attempt typo correction early so that the type of the init expression can
     // 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;
     }
     Init = Res.get();
 
     if (DeduceVariableDeclarationType(VDecl, DirectInit, Init))
       return;
   }

clang/lib/Sema/SemaExceptionSpec.cpp

if (cast<GenericSelectionExpr>(S)->isResultDependent())
       return CT_Dependent;
     return canThrow(cast<GenericSelectionExpr>(S)->getResultExpr());
 
     // Some expressions are always dependent.
   case Expr::CXXDependentScopeMemberExprClass:
   case Expr::CXXUnresolvedConstructExprClass:
   case Expr::DependentScopeDeclRefExprClass:
   case Expr::CXXFoldExprClass:
+  case Expr::RecoveryExprClass:
     return CT_Dependent;
 
   case Expr::AsTypeExprClass:
   case Expr::BinaryConditionalOperatorClass:
   case Expr::BlockExprClass:
   case Expr::CUDAKernelCallExprClass:
   case Expr::DeclRefExprClass:
   case Expr::ObjCBridgedCastExprClass:
   case Expr::ObjCIndirectCopyRestoreExprClass:
   case Expr::ObjCProtocolExprClass:
   case Expr::ObjCSelectorExprClass:
   case Expr::ObjCAvailabilityCheckExprClass:
   case Expr::OffsetOfExprClass:
   case Expr::PackExpansionExprClass:
   case Expr::SubstNonTypeTemplateParmExprClass:
   case Expr::SubstNonTypeTemplateParmPackExprClass:
   case Expr::FunctionParmPackExprClass:
   case Expr::UnaryExprOrTypeTraitExprClass:
   case Expr::UnresolvedLookupExprClass:
   case Expr::UnresolvedMemberExprClass:
   case Expr::TypoExprClass:
     // FIXME: Many of the above can throw.
     return CT_Cannot;
 
   case Expr::AddrLabelExprClass:
   case Expr::ArrayTypeTraitExprClass:
   case Expr::AtomicExprClass:
   case Expr::TypeTraitExprClass:
   case Expr::CXXBoolLiteralExprClass:
   case Expr::CXXNoexceptExprClass:
   case Expr::CXXNullPtrLiteralExprClass:

clang/lib/Sema/SemaExpr.cpp

ExprResult Sema::ActOnObjCAvailabilityCheckExpr(
   return new (Context)
       ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
 }
 
 bool Sema::IsDependentFunctionNameExpr(Expr *E) {
   assert(E->isTypeDependent());
   return isa<UnresolvedLookupExpr>(E);
 }
+
+ExprResult Sema::CreateRecoveryExpr(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();
+  if (isSFINAEContext())
+    return ExprError();
+
+  return RecoveryExpr::Create(Context, Begin, End, SubExprs);
+}

clang/lib/Sema/TreeTransform.h

ExprResult RebuildAtomicExpr(SourceLocation BuiltinLoc, MultiExprArg SubExprs,
                                SourceLocation RParenLoc) {
     // Use this for all of the locations, since we don't know the difference
     // between the call and the expr at this point.
     SourceRange Range{BuiltinLoc, RParenLoc};
     return getSema().BuildAtomicExpr(Range, Range, RParenLoc, SubExprs, Op,
                                      Sema::AtomicArgumentOrder::AST);
   }
 
+  ExprResult RebuildRecoveryExpr(SourceLocation BeginLoc, SourceLocation EndLoc,
+                                 ArrayRef<Expr *> SubExprs) {
+    return getSema().CreateRecoveryExpr(BeginLoc, EndLoc, SubExprs);
+  }
+
 private:
   TypeLoc TransformTypeInObjectScope(TypeLoc TL,
                                      QualType ObjectType,
                                      NamedDecl *FirstQualifierInScope,
                                      CXXScopeSpec &SS);
 
   TypeSourceInfo *TransformTypeInObjectScope(TypeSourceInfo *TSInfo,
                                              QualType ObjectType,

 }
 
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformTypoExpr(TypoExpr *E) {
   return E;
 }
 
-template<typename Derived>
+template <typename Derived>
+ExprResult TreeTransform<Derived>::TransformRecoveryExpr(RecoveryExpr *E) {
+  llvm::SmallVector<Expr *, 8> Children;
+  bool Changed = false;
+  for (Expr *C : E->subExpressions()) {
+    ExprResult NewC = getDerived().TransformExpr(C);
+    if (NewC.isInvalid())
+      return ExprError();
+    Children.push_back(NewC.get());
+
+    Changed |= NewC.get() != C;
+  }
+  if (!getDerived().AlwaysRebuild() && !Changed)
+    return E;
+  return getDerived().RebuildRecoveryExpr(E->getBeginLoc(), E->getEndLoc(),
+                                          Children);
+}
+
+template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformPseudoObjectExpr(PseudoObjectExpr *E) {
   // Rebuild the syntactic form.  The original syntactic form has
   // opaque-value expressions in it, so strip those away and rebuild
   // the result.  This is a really awful way of doing this, but the
   // better solution (rebuilding the semantic expressions and
   // rebinding OVEs as necessary) doesn't work; we'd need
   // TreeTransform to not strip away implicit conversions.

clang/lib/Serialization/ASTReaderStmt.cpp

void ASTStmtReader::VisitOpaqueValueExpr(OpaqueValueExpr *E) {
   E->OpaqueValueExprBits.Loc = readSourceLocation();
   E->setIsUnique(Record.readInt());
 }
 
 void ASTStmtReader::VisitTypoExpr(TypoExpr *E) {
   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
 //===----------------------------------------------------------------------===//
 void ASTStmtReader::VisitMSPropertyRefExpr(MSPropertyRefExpr *E) {
   VisitExpr(E);
   E->IsArrow = (Record.readInt() != 0);
   E->BaseExpr = Record.readSubExpr();
   E->QualifierLoc = Record.readNestedNameSpecifierLoc();
case EXPR_OMP_ARRAY_SECTION:
       S = new (Context) OMPArraySectionExpr(Empty);
       break;
 
     case EXPR_CALL:
       S = CallExpr::CreateEmpty(
           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],
                                   Record[ASTStmtReader::NumExprFields + 2],
                                   Record[ASTStmtReader::NumExprFields + 3]);
       break;
 
     case EXPR_BINARY_OPERATOR:

clang/lib/Serialization/ASTWriterStmt.cpp

void ASTStmtWriter::VisitCallExpr(CallExpr *E) {
   Record.AddStmt(E->getCallee());
   for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
        Arg != ArgEnd; ++Arg)
     Record.AddStmt(*Arg);
   Record.push_back(static_cast<unsigned>(E->getADLCallKind()));
   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);
 
   bool HasQualifier = E->hasQualifier();
   bool HasFoundDecl =
       E->hasQualifierOrFoundDecl() &&
       (E->getFoundDecl().getDecl() != E->getMemberDecl() ||
        E->getFoundDecl().getAccess() != E->getMemberDecl()->getAccess());

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

switch (S->getStmtClass()) {
     case Stmt::MSPropertySubscriptExprClass:
     case Stmt::CXXUnresolvedConstructExprClass:
     case Stmt::DependentScopeDeclRefExprClass:
     case Stmt::ArrayTypeTraitExprClass:
     case Stmt::ExpressionTraitExprClass:
     case Stmt::UnresolvedLookupExprClass:
     case Stmt::UnresolvedMemberExprClass:
     case Stmt::TypoExprClass:
+    case Stmt::RecoveryExprClass:
     case Stmt::CXXNoexceptExprClass:
     case Stmt::PackExpansionExprClass:
     case Stmt::SubstNonTypeTemplateParmPackExprClass:
     case Stmt::FunctionParmPackExprClass:
     case Stmt::CoroutineBodyStmtClass:
     case Stmt::CoawaitExprClass:
     case Stmt::DependentCoawaitExprClass:
     case Stmt::CoreturnStmtClass:

clang/test/AST/ast-dump-expr-errors.cpp

+// 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 
+// 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();

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

+// 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;

clang/test/Index/getcursor-recovery.cpp

+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

clang/test/Parser/cxx-concepts-ambig-constraint-expr.cpp

 // RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ %s -verify
 
 // Test parsing of constraint-expressions in cases where the grammar is
 // ambiguous with the expectation that the longest token sequence which matches
 // the syntax is consumed without backtracking.
 
 // type-specifier-seq in conversion-type-id
-template <typename T> requires T::operator short
-unsigned int foo(); // expected-error {{C++ requires a type specifier for all declarations}}
+template <typename T> requires (bool)&T::operator short
+unsigned int foo(); // expected-error {{C++ requires a type specifier for all declarations}}
+
+// type-specifier-seq in new-type-id
+template <typename T> requires (bool)sizeof new (T::f()) short
+unsigned int bar(); // expected-error {{C++ requires a type specifier for all declarations}}
+
+template<typename T> requires (bool)sizeof new (T::f()) unsigned // expected-error {{'struct' cannot be signed or unsigned}}
+struct X { }; // expected-error {{'X' cannot be defined in a type specifier}}
+
+// C-style cast
+// of function call on function-style cast
+template <typename T> requires (bool(T()))
+T (*fp)(); // expected-error {{use of undeclared identifier 'fp'}}
+
+// function-style cast
+// as the callee in a function call
+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}} \
+                  // expected-error {{expected member name or ';' after declaration specifiers}}
+};

clang/test/Parser/objcxx0x-lambda-expressions.mm

 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-unused-value -Wno-c++1y-extensions -std=c++11 %s
 
 class C {
   id get(int);
 
   void f() {
     int foo, bar, baz;
 
     // fail to parse as a lambda introducer, so we get objc message parsing errors instead
     [foo,+] {}; // expected-error {{expected expression}}
 
     []; // 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 {}; 
     [foo,bar] () { return 3; }; 
     [=,&foo] () {}; 
     [this] () {}; 
 
     [foo(bar)] () {};
     [foo = bar] () {};
     [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'}}
 
     [get(bar) baz]; // expected-warning {{instance method '-baz'}}
     [get(bar), baz]; // expected-error {{expected body of lambda}}
 
     [foo = bar ++ baz]; // expected-warning {{receiver type 'int'}} expected-warning {{instance method '-baz'}}

clang/test/Parser/objcxx11-invalid-lambda.cpp

 // 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 ')'}}
+}

clang/test/SemaCXX/builtins.cpp

 // RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++11 -fcxx-exceptions
 // RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++1z -fcxx-exceptions
 typedef const struct __CFString * CFStringRef;
 #define CFSTR __builtin___CFStringMakeConstantString
 
 void f() {
   (void)CFStringRef(CFSTR("Hello"));
 }
 
 void a() { __builtin_va_list x, y; ::__builtin_va_copy(x, y); }
 
 // <rdar://problem/10063539>
 template<int (*Compare)(const char *s1, const char *s2)>
 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() {
   __builtin_isnan; // expected-error {{builtin functions must be directly called}}
 }
 
 // pr14895
 typedef __typeof(sizeof(int)) size_t;

clang/test/SemaCXX/cast-conversion.cpp

-// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown -verify %s -std=c++11
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown -verify %s -std=c++11 -Wno-unused
 
 struct R {
   R(int);
 };
 
 struct A {
   A(R);
 };

clang/test/SemaCXX/cxx1z-copy-omission.cpp

-// RUN: %clang_cc1 -std=c++1z -verify %s
+// RUN: %clang_cc1 -std=c++1z -verify -Wno-unused %s
 
 struct Noncopyable {
   Noncopyable();
   Noncopyable(const Noncopyable &) = delete; // expected-note 1+{{deleted}} expected-note 1+ {{not viable}}
   virtual ~Noncopyable();
 };
 struct Derived : Noncopyable {};
 struct NoncopyableAggr { // expected-note 3{{candidate}}
void test_expressions(bool b) {
   make_incomplete().*(int Incomplete::*)nullptr; // expected-error {{incomplete}}
   dynamic_cast<Incomplete&&>(make_incomplete()); // expected-error {{incomplete}}
   const_cast<Incomplete&&>(make_incomplete()); // expected-error {{incomplete}}
 
   sizeof(Indestructible{}); // expected-error {{deleted}}
   sizeof(make_indestructible()); // expected-error {{deleted}}
   sizeof(make_incomplete()); // expected-error {{incomplete}}
   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}}
   using I = decltype(Indestructible{}); // expected-error {{deleted}}
   using I = decltype(make_indestructible());
   using J = decltype(make_incomplete());
 
   Noncopyable cond1 = b ? Noncopyable() : make();

clang/test/SemaCXX/decltype-crash.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat -std=c++98 %s
 
 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'}}
 }

clang/test/SemaCXX/varargs.cpp

void no_params(...) {
   int a;
   __builtin_va_start(ap, a); // expected-warning {{second argument to 'va_start' is not the last named parameter}}
 }
 
 // Reject this. The __builtin_va_start would execute in Foo's non-variadic
 // 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)) {}
   };
 }
 
 #if __cplusplus >= 201103L
 // We used to have bugs identifying the correct enclosing function scope in a
 // lambda.
 

clang/test/SemaOpenCLCXX/address-space-references.cl

 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -pedantic -verify -fsyntax-only
 
 __global const int& f(__global float &ref) {
   return ref; // expected-error{{reference of type 'const __global int &' cannot bind to a temporary object because of address space mismatch}}
 }
 
 int bar(const __global unsigned int &i); // expected-note{{passing argument to parameter 'i' here}}
 //FIXME: With the overload below the call should be resolved
 // successfully. However, current overload resolution logic
 // can't detect this case and therefore fails.
 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}}
 }

clang/test/SemaTemplate/instantiate-init.cpp

namespace PR7985 {
   const Data<T> Description<T>::data[] = {{ 1 }}; // expected-error{{cannot initialize a member subobject of type 'int *' with an rvalue of type 'int'}}
 
   template<>
   const Data<float*> Description<float*>::data[];
 
   void test() {
     integral_c<1> ic1 = array_lengthof(Description<int>::data);
     (void)sizeof(array_lengthof(Description<float>::data));
 
-    sizeof(array_lengthof( // expected-error{{no matching function for call to 'array_lengthof'}}
+    (void)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}}
                           ));
 
     array_lengthof(Description<float*>::data); // expected-error{{no matching function for call to 'array_lengthof'}}
   }
 }
 
 namespace PR13064 {

clang/tools/libclang/CXCursor.cpp

CXCursor cxcursor::MakeCXCursor(const Stmt *S, const Decl *Parent,
   case Stmt::ShuffleVectorExprClass:
   case Stmt::SourceLocExprClass:
   case Stmt::ConvertVectorExprClass:
   case Stmt::VAArgExprClass:
   case Stmt::ObjCArrayLiteralClass:
   case Stmt::ObjCDictionaryLiteralClass:
   case Stmt::ObjCBoxedExprClass:
   case Stmt::ObjCSubscriptRefExprClass:
+  case Stmt::RecoveryExprClass:
     K = CXCursor_UnexposedExpr;
     break;
 
   case Stmt::OpaqueValueExprClass:
     if (Expr *Src = cast<OpaqueValueExpr>(S)->getSourceExpr())
       return MakeCXCursor(Src, Parent, TU, RegionOfInterest);
     K = CXCursor_UnexposedExpr;
     break;