[AST] Preserve the type in RecoveryExprs for broken function calls.

[hokein]

Summary

RecoveryExprs are modeled as dependent type to prevent bogus diagnostics
and crashes in clang.

This patch allows to preseve the type for broken calls when the
RecoveryEprs have a known type, e.g. a broken non-overloaded call, a
overloaded call when the all candidates have the same return type, so
that more features (code completion still work on "take2args(x).^") still
work.

However, adding the type is risky, which may result in more clang code being
affected leading to new crashes and hurt diagnostic, and it requires large
effort to minimize the affect (update all sites in clang to handle errorDepend
case), so we add a new flag (off by default) to allow us to develop/test
them incrementally.

This patch also has some trivial fixes to suppress diagnostics (to prevent regressions).

Tested:

• all existing tests are passed (when both "-frecovery-ast", "-frecovery-ast-type" flags are flipped on);

[hokein]

update

[hokein]

clang/lib/Sema/SemaOverload.cpp

12779

I steal this function from D61722.

clang/test/SemaCXX/enable_if.cpp

417–418

this diagnostic is changed slightly (without -frecovery-ast-type). I think this is acceptable.

before this patch:

/tmp/t5.cpp:7:10: error: no matching function for call to 'templated'
  return templated<N>();  // expected-error {{no matching function for call to 'templated'}} 
         ^~~~~~~~~~~~
/tmp/t5.cpp:13:5: note: in instantiation of function template specialization 'enable_if_diags::callTemplated<0>' requested here
    callTemplated<0>(); // expected-note {{in call to 'callTemplated()'}} expected-note@-6 {{subexpression not valid in a constant expression}}
    ^
/tmp/t5.cpp:2:32: note: candidate disabled: <no message provided>
template <int N> constexpr int templated() __attribute__((enable_if(N, ""))) {
                               ^                                    ~
/tmp/t5.cpp:13:5: error: constexpr variable 'B' must be initialized by a constant expression
    callTemplated<0>(); // expected-note {{in call to 'callTemplated()'}} expected-note@-6 {{subexpression not valid in a constant expression}}
    ^~~~~~~~~~~~~~~~~~
2 errors generated.

vs after this patch:

/tmp/t5.cpp:7:10: error: no matching function for call to 'templated'
  return templated<N>();  // expected-error {{no matching function for call to 'templated'}} 
         ^~~~~~~~~~~~
/tmp/t5.cpp:13:5: note: in instantiation of function template specialization 'enable_if_diags::callTemplated<0>' requested here
    callTemplated<0>(); // expected-note {{in call to 'callTemplated()'}} expected-note@-6 {{subexpression not valid in a constant expression}}
    ^
/tmp/t5.cpp:2:32: note: candidate disabled: <no message provided>
template <int N> constexpr int templated() __attribute__((enable_if(N, ""))) {
                               ^                                    ~
/tmp/t5.cpp:12:15: error: constexpr variable 'B' must be initialized by a constant expression
constexpr int B = 10 +  // expected-error {{constexpr variable 'B' must be initialized by a constant expression}}
              ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/t5.cpp:7:10: note: subexpression not valid in a constant expression
  return templated<N>();  // expected-error {{no matching function for call to 'templated'}} 
         ^
/tmp/t5.cpp:13:5: note: in call to 'callTemplated()'
    callTemplated<0>(); // expected-note {{in call to 'callTemplated()'}} expected-note@-6 {{subexpression not valid in a constant expression}}
    ^
2 errors generated.

clang/test/SemaTemplate/instantiate-function-params.cpp

1

this test is very tricky, and hard for human to understand. It was added to test clang not crashing.

Different compilers (gcc, msvc) emit different diagnostics, with this patch, clang emits 3 more errs no type named 'type'..., it seems reasonable, and gcc also emits them.

[sammccall]

clang/include/clang/AST/Expr.h

6042

Comment is stale.

6042

We should mention something about optional type preservation.

clang/lib/AST/ComputeDependence.cpp

491

Dropping type-dependence seems like the point here and is risky, doesn't dropping value- and instantiation-dependence introduce unneccesary extra risk?

Probably correct and maybe useful, but splitting it into a separate change and maybe delaying it might make it easier to get the principal change to stick.

clang/lib/AST/Expr.cpp

4656

Why is this in ifndef?
Actually, why is the loop in ifndef? surely the compiler can inline and remove an empty loop over an arrayref?

clang/lib/Sema/SemaDecl.cpp

16427

why is this needed/does it help?
I would have thought in the cases where we now don't consider the type dependent, the *type* wouldn't contain errors (rather the expression owning the type would).

Is this just for decltype?

clang/lib/Sema/SemaOverload.cpp

12904

nit: try to recover
(or just "Overload resolution failed", what the code is doing is obvious enough here)

clang/test/CodeCompletion/member-access.cpp

276

I think there's an interesting test of the "viable" case where you have a const/non-const overload set with different return types:

class Collection {
  const char *find(int) const;
  char* find(int) const;
};
void test1(const Collection &C, Collection &NC) {
 C.find(); // missing arg, type const char*
 NC.find(); // missing arg, is type NC or is it unresolved?
}

(Not sure if it's best written as a codecompletion test, I guess AST dump is better)

clang/test/Index/getcursor-recovery.cpp

6

nit: unresolved overloaded call?

19

nit: a valid foo(int, int) call?
(just to draw the contrast)

clang/test/SemaCXX/enable_if.cpp

417–418

Yeah, this is noisier but seems OK.
Can you take a look at the blame for the split-line test and see if there was any special code to support it that can be cleaned up?

clang/test/SemaCXX/recovery-expr-type.cpp

12

what if we make this constexpr and try to static-assert on it, use it as the size of an array etc?

clang/test/SemaTemplate/instantiate-function-params.cpp

1

Ugh, I remember this test. It's partially reduced from boost IIRC.

[hokein]

address comments.

[hokein]

clang/lib/AST/ComputeDependence.cpp

491

Dropping type-dependence seems like the point here and is risky, doesn't dropping value- and instantiation-dependence introduce unneccesary extra risk?

maybe, but you are right, we should be conservative.

separating this patch seems good to me:

1. create a new patch like what we do here,
2. this patch just getting rid of the type bit (still set the value/instantiation bits)

with 1, we will get rid of the value/instantiation-depend bits (for the default Ctx.DependentType case), and we get closer to our goal.

clang/lib/AST/Expr.cpp

4656

removed.

clang/lib/Sema/SemaDecl.cpp

16427

yeah, decltype is the only case we have been discovered so far.

clang/test/CodeCompletion/member-access.cpp

276

oh, this is a good point, this patch doesn't capture the type for member functions, it is in a different code path (probably we should handle that in BuildCallToMemberFunction).

I'd prefer to handle it in a separate patch.

clang/test/SemaCXX/enable_if.cpp

417–418

git blame showed https://github.com/llvm/llvm-project/commit/c7d3e609fbf05d1a1236f99efd1e2fd344554f4b, but I didn't see any special code that we need clean up.

clang/test/SemaCXX/recovery-expr-type.cpp

12

added tests

[sammccall]

clang/lib/AST/ComputeDependence.cpp

488

This comment is hard to understand in isolation: it describes what the patch is doing (compared to the baseline, and to some alternative), rather than what the code is doing.

I'd suggest something like:

Mark the expression as value- and instantiation- dependent to reuse existing suppressions for dependent code, e.g. avoiding constant-evaluation.

clang/lib/AST/Expr.cpp

3309

This can move into the later patch I think - we're always instantiation-dependent where it matters.

clang/lib/Sema/SemaDecl.cpp

12807

again - can move into later patch

clang/lib/Sema/SemaDeclCXX.cpp

15887

can defer this too

clang/test/CodeCompletion/member-access.cpp

283

nit: CHECK-RECOVERY or so?

[hokein]

address comments.

[hokein]

rebase to master (not the D78350)

[hokein]

add a missing test file during the rebase.

[hokein]

clang/test/SemaCXX/enable_if.cpp

417–418

actually we don't need this change if we land this patch before the enable-recovery-expr patch, defer this change to D78350.

clang/test/SemaTemplate/instantiate-function-params.cpp

1

the same to this file, defer it to D78350.

clang/include/clang/AST/Expr.h

 /// 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.
 ///
 /// For now, 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 *Create(ASTContext &Ctx, QualType T,
+                              SourceLocation BeginLoc, SourceLocation EndLoc,
+                              ArrayRef<Expr *> SubExprs);
   static RecoveryExpr *CreateEmpty(ASTContext &Ctx, unsigned NumSubExprs);
 
   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(ASTContext &Ctx, QualType T, SourceLocation BeginLoc,
+               SourceLocation EndLoc, ArrayRef<Expr *> SubExprs);
   RecoveryExpr(EmptyShell Empty, unsigned NumSubExprs)
       : Expr(RecoveryExprClass, Empty), NumExprs(NumSubExprs) {}
 
   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/Basic/LangOptions.def

 LANGOPT(GNUAsm            , 1, 1, "GNU-style inline assembly")
 LANGOPT(Coroutines        , 1, 0, "C++20 coroutines")
 LANGOPT(DllExportInlines  , 1, 1, "dllexported classes dllexport inline methods")
 LANGOPT(RelaxedTemplateTemplateArgs, 1, 0, "C++17 relaxed matching of template template arguments")
 
 LANGOPT(DoubleSquareBracketAttributes, 1, 0, "'[[]]' attributes extension for all language standard modes")
 
 COMPATIBLE_LANGOPT(RecoveryAST, 1, CPlusPlus, "Preserve expressions in AST when encountering errors")
+COMPATIBLE_LANGOPT(RecoveryASTType, 1, 0, "Preserve the type in recovery expressions")
 
 BENIGN_LANGOPT(ThreadsafeStatics , 1, 1, "thread-safe static initializers")
 LANGOPT(POSIXThreads      , 1, 0, "POSIX thread support")
 LANGOPT(Blocks            , 1, 0, "blocks extension to C")
 BENIGN_LANGOPT(EmitAllDecls      , 1, 0, "emitting all declarations")
 LANGOPT(MathErrno         , 1, 1, "errno in math functions")
 BENIGN_LANGOPT(HeinousExtensions , 1, 0, "extensions that we really don't like and may be ripped out at any time")
 LANGOPT(Modules           , 1, 0, "modules semantics")

clang/include/clang/Driver/CC1Options.td

 def fno_concept_satisfaction_caching : Flag<["-"],
                                             "fno-concept-satisfaction-caching">,
   HelpText<"Disable satisfaction caching for C++2a Concepts.">;
 
 def frecovery_ast : Flag<["-"], "frecovery-ast">,
   HelpText<"Preserve expressions in AST rather than dropping them when "
            "encountering semantic errors">;
 def fno_recovery_ast : Flag<["-"], "fno-recovery-ast">;
+def frecovery_ast_type : Flag<["-"], "frecovery-ast-type">,
+  HelpText<"Preserve the type for recovery expressions when possible "
+          "(experimental) ">;
+def fno_recovery_ast_type : Flag<["-"], "fno-recovery-ast-type">;
 
 let Group = Action_Group in {
 
 def Eonly : Flag<["-"], "Eonly">,
   HelpText<"Just run preprocessor, no output (for timings)">;
 def dump_raw_tokens : Flag<["-"], "dump-raw-tokens">,
   HelpText<"Lex file in raw mode and dump raw tokens">;
 def analyze : Flag<["-"], "analyze">,

clang/include/clang/Sema/Sema.h

public:
 
   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);
+                                ArrayRef<Expr *> SubExprs,
+                                QualType T = QualType());
 
   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,

clang/lib/AST/ComputeDependence.cpp

ExprDependence clang::computeDependence(DeclRefExpr *E, const ASTContext &Ctx) {
   //      -  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) {
   // FIXME: drop type+value+instantiation once Error is sufficient to suppress
   // bogus dianostics.
-  auto D = ExprDependence::TypeValueInstantiation | ExprDependence::Error;
+  auto D =
+      toExprDependence(E->getType()->getDependence()) | ExprDependence::Error;
   for (auto *S : E->subExpressions())
     D |= S->getDependence();
   return D;
 }
 
 ExprDependence clang::computeDependence(PredefinedExpr *E) {
   return toExprDependence(E->getType()->getDependence()) &
          ~ExprDependence::UnexpandedPack;

clang/lib/AST/Expr.cpp

 bool Expr::HasSideEffects(const ASTContext &Ctx,
                           bool IncludePossibleEffects) const {
   // In circumstances where we care about definite side effects instead of
   // potential side effects, we want to ignore expressions that are part of a
   // macro expansion as a potential side effect.
   if (!IncludePossibleEffects && getExprLoc().isMacroID())
     return false;
 
-  if (isInstantiationDependent())
+  if (isInstantiationDependent() || containsErrors())
     return IncludePossibleEffects;
 
   switch (getStmtClass()) {
   case NoStmtClass:
   #define ABSTRACT_STMT(Type)
   #define STMT(Type, Base) case Type##Class:
   #define EXPR(Type, Base)
   #include "clang/AST/StmtNodes.inc"
for (unsigned Cnt = 0; Cnt < ArraySectionCount; ++Cnt) {
     else {
       assert (OriginalTy->isArrayType());
       OriginalTy = OriginalTy->castAsArrayTypeUnsafe()->getElementType();
     }
   }
   return OriginalTy;
 }
 
-RecoveryExpr::RecoveryExpr(ASTContext &Ctx, SourceLocation BeginLoc,
+RecoveryExpr::RecoveryExpr(ASTContext &Ctx, QualType T, SourceLocation BeginLoc,
                            SourceLocation EndLoc, ArrayRef<Expr *> SubExprs)
-    : Expr(RecoveryExprClass, Ctx.DependentTy, VK_LValue, OK_Ordinary),
-      BeginLoc(BeginLoc), EndLoc(EndLoc), NumExprs(SubExprs.size()) {
+    : Expr(RecoveryExprClass, T, VK_LValue, OK_Ordinary), BeginLoc(BeginLoc),
+      EndLoc(EndLoc), NumExprs(SubExprs.size()) {
 #ifndef NDEBUG
+  assert(!T.isNull());
   for (auto *E : SubExprs)
     assert(E != nullptr);
 #endif
 
   llvm::copy(SubExprs, getTrailingObjects<Expr *>());
   setDependence(computeDependence(this));
 }
 
-RecoveryExpr *RecoveryExpr::Create(ASTContext &Ctx, SourceLocation BeginLoc,
+RecoveryExpr *RecoveryExpr::Create(ASTContext &Ctx, QualType T,
+                                   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);
+  return new (Mem) RecoveryExpr(Ctx, T, BeginLoc, EndLoc, SubExprs);
 }
 
 RecoveryExpr *RecoveryExpr::CreateEmpty(ASTContext &Ctx, unsigned NumSubExprs) {
   void *Mem = Ctx.Allocate(totalSizeToAlloc<Expr *>(NumSubExprs),
                            alignof(RecoveryExpr));
   return new (Mem) RecoveryExpr(EmptyShell(), NumSubExprs);
 }
 

clang/lib/Frontend/CompilerInvocation.cpp

if (Opts.NewAlignOverride && !llvm::isPowerOf2_32(Opts.NewAlignOverride)) {
     Opts.NewAlignOverride = 0;
   }
   Opts.ConceptSatisfactionCaching =
       !Args.hasArg(OPT_fno_concept_satisfaction_caching);
   if (Args.hasArg(OPT_fconcepts_ts))
     Diags.Report(diag::warn_fe_concepts_ts_flag);
   Opts.RecoveryAST =
       Args.hasFlag(OPT_frecovery_ast, OPT_fno_recovery_ast, Opts.CPlusPlus);
+  Opts.RecoveryASTType =
+      Args.hasFlag(OPT_frecovery_ast_type, OPT_fno_recovery_ast_type, false);
   Opts.HeinousExtensions = Args.hasArg(OPT_fheinous_gnu_extensions);
   Opts.AccessControl = !Args.hasArg(OPT_fno_access_control);
   Opts.ElideConstructors = !Args.hasArg(OPT_fno_elide_constructors);
   Opts.MathErrno = !Opts.OpenCL && Args.hasArg(OPT_fmath_errno);
   Opts.InstantiationDepth =
       getLastArgIntValue(Args, OPT_ftemplate_depth, 1024, Diags);
   Opts.ArrowDepth =
       getLastArgIntValue(Args, OPT_foperator_arrow_depth, 256, Diags);

clang/lib/Sema/SemaDecl.cpp

void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
 
   if (var->hasAttr<BlocksAttr>())
     getCurFunction()->addByrefBlockVar(var);
 
   Expr *Init = var->getInit();
   bool IsGlobal = GlobalStorage && !var->isStaticLocal();
   QualType baseType = Context.getBaseElementType(type);
 
-  if (Init && !Init->isValueDependent()) {
+  if (Init && !Init->isValueDependent() && !Init->containsErrors()) {
     if (var->isConstexpr()) {
       SmallVector<PartialDiagnosticAt, 8> Notes;
       if (!var->evaluateValue(Notes) || !var->isInitICE()) {
         SourceLocation DiagLoc = var->getLocation();
         // If the note doesn't add any useful information other than a source
         // location, fold it into the primary diagnostic.
         if (Notes.size() == 1 && Notes[0].second.getDiagID() ==
               diag::note_invalid_subexpr_in_const_expr) {
FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T,
   // If we receive a broken type, recover by assuming 'int' and
   // marking this declaration as invalid.
   if (T.isNull()) {
     InvalidDecl = true;
     T = Context.IntTy;
   }
 
   QualType EltTy = Context.getBaseElementType(T);
-  if (!EltTy->isDependentType()) {
+  if (!EltTy->isDependentType() && !EltTy->containsErrors()) {
     if (RequireCompleteSizedType(Loc, EltTy,
                                  diag::err_field_incomplete_or_sizeless)) {
       // Fields of incomplete type force their record to be invalid.
       Record->setInvalidDecl();
       InvalidDecl = true;
     } else {
       NamedDecl *Def;
       EltTy->isIncompleteType(&Def);

clang/lib/Sema/SemaDeclCXX.cpp

 
 Decl *Sema::BuildStaticAssertDeclaration(SourceLocation StaticAssertLoc,
                                          Expr *AssertExpr,
                                          StringLiteral *AssertMessage,
                                          SourceLocation RParenLoc,
                                          bool Failed) {
   assert(AssertExpr != nullptr && "Expected non-null condition");
   if (!AssertExpr->isTypeDependent() && !AssertExpr->isValueDependent() &&
-      !Failed) {
+      !AssertExpr->containsErrors() && !Failed) {
     // In a static_assert-declaration, the constant-expression shall be a
     // constant expression that can be contextually converted to bool.
     ExprResult Converted = PerformContextuallyConvertToBool(AssertExpr);
     if (Converted.isInvalid())
       Failed = true;
 
     ExprResult FullAssertExpr =
         ActOnFinishFullExpr(Converted.get(), StaticAssertLoc,

clang/lib/Sema/SemaExpr.cpp

 }
 
 bool Sema::IsDependentFunctionNameExpr(Expr *E) {
   assert(E->isTypeDependent());
   return isa<UnresolvedLookupExpr>(E);
 }
 
 ExprResult Sema::CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
-                                    ArrayRef<Expr *> SubExprs) {
+                                    ArrayRef<Expr *> SubExprs, QualType T) {
   // FIXME: enable it for 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().RecoveryAST)
     return ExprError();
 
   if (isSFINAEContext())
     return ExprError();
 
-  return RecoveryExpr::Create(Context, Begin, End, SubExprs);
+  if (T.isNull() || !Context.getLangOpts().RecoveryASTType)
+    // We don't know the concrete type, fallback to dependent type.
+    T = Context.DependentTy;
+  return RecoveryExpr::Create(Context, T, Begin, End, SubExprs);
 }

clang/lib/Sema/SemaOverload.cpp

#endif
 
   if (CandidateSet->empty())
     return false;
 
   UnbridgedCasts.restore();
   return false;
 }
 
+// Guess at what the return type for an unresolvable overload should be.
+static QualType chooseRecoveryType(OverloadCandidateSet &CS,
+                                   OverloadCandidateSet::iterator *Best) {
+  llvm::Optional<QualType> Result;
+  // Adjust Type after seeing a candidate.
+  auto ConsiderCandidate = [&](const OverloadCandidate &Candidate) {
+    if (!Candidate.Function)
+      return;
+    QualType T = Candidate.Function->getCallResultType();
+    if (T.isNull())
+      return;
+    if (!Result)
+      Result = T;
+    else if (Result != T)
+      Result = QualType();
+  };
+
+  // Look for an unambiguous type from a progressively larger subset.
+  // e.g. if types disagree, but all *viable* overloads return int, choose int.
+  //
+  // First, consider only the best candidate.
+  if (Best && *Best != CS.end())
+    ConsiderCandidate(**Best);
+  // Next, consider only viable candidates.
+  if (!Result)
+    for (const auto &C : CS)
+      if (C.Viable)
+        ConsiderCandidate(C);
+  // Finally, consider all candidates.
+  if (!Result)
+    for (const auto &C : CS)
+      ConsiderCandidate(C);
+
+  return Result.getValueOr(QualType());
+}
+
 /// FinishOverloadedCallExpr - given an OverloadCandidateSet, builds and returns
 /// the completed call expression. If overload resolution fails, emits
 /// diagnostics and returns ExprError()
 static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
                                            UnresolvedLookupExpr *ULE,
                                            SourceLocation LParenLoc,
                                            MultiExprArg Args,
                                            SourceLocation RParenLoc,
case OR_Deleted: {
     FunctionDecl *FDecl = (*Best)->Function;
     Fn = SemaRef.FixOverloadedFunctionReference(Fn, (*Best)->FoundDecl, FDecl);
     return SemaRef.BuildResolvedCallExpr(Fn, FDecl, LParenLoc, Args, RParenLoc,
                                          ExecConfig, /*IsExecConfig=*/false,
                                          (*Best)->IsADLCandidate);
   }
   }
 
-  // Overload resolution failed.
-  return ExprError();
+  // Overload resolution failed, try to recovery.
+  SmallVector<Expr *, 8> SubExprs = {Fn};
+  SubExprs.append(Args.begin(), Args.end());
+  return SemaRef.CreateRecoveryExpr(Fn->getBeginLoc(), RParenLoc, SubExprs,
+                                    chooseRecoveryType(*CandidateSet, Best));
 }
 
 static void markUnaddressableCandidatesUnviable(Sema &S,
                                                 OverloadCandidateSet &CS) {
   for (auto I = CS.begin(), E = CS.end(); I != E; ++I) {
     if (I->Viable &&
         !S.checkAddressOfFunctionIsAvailable(I->Function, /*Complain=*/false)) {
       I->Viable = false;

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

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

clang/test/CodeCompletion/member-access.cpp

struct S {
     int operator=(int);
   } s;
   return s.
 }
 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:270:12 %s -o - | FileCheck -check-prefix=CHECK-OPER %s \
 // RUN: --implicit-check-not="[#char#]operator=("
 // CHECK-OPER: [#int#]operator=(
 
+struct S { int member; };
+S overloaded(int);
+S overloaded(double);
+void foo() {
+  // No overload matches, but we have recovery-expr with the correct type.
+  overloaded().
+}
+// RUN: not %clang_cc1 -fsyntax-only -frecovery-ast -frecovery-ast-type -code-completion-at=%s:281:16 %s -o - | FileCheck -check-prefix=CHECK-CC10 %s
+// CHECK-CC10: [#int#]member
 No newline at end of file

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));
+void testTypedRecoveryExpr1() {
+  // Inner bar() is a RecoveryExpr, outer foo() is an overloaded call.
+  foo(x, bar(x));
 }
-// RUN: c-index-test -cursor-at=%s:7:3 %s -Xclang -frecovery-ast | FileCheck -check-prefix=OUTER-FOO %s
+// RUN: c-index-test -cursor-at=%s:7:3 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=OUTER-FOO %s
 // OUTER-FOO: OverloadedDeclRef=foo[2:5, 1:5]
-// RUN: c-index-test -cursor-at=%s:7:7 %s -Xclang -frecovery-ast | FileCheck -check-prefix=OUTER-X %s
+// RUN: c-index-test -cursor-at=%s:7:7 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=OUTER-X %s
 // OUTER-X: DeclRefExpr=x:3:5
-// RUN: c-index-test -cursor-at=%s:7:10 %s -Xclang -frecovery-ast | FileCheck -check-prefix=INNER-FOO %s
-// INNER-FOO: OverloadedDeclRef=foo[2:5, 1:5]
-// RUN: c-index-test -cursor-at=%s:7:14 %s -Xclang -frecovery-ast | FileCheck -check-prefix=INNER-X %s
+// RUN: c-index-test -cursor-at=%s:7:10 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=INNER-FOO %s
+// INNER-FOO: OverloadedDeclRef=bar
+// RUN: c-index-test -cursor-at=%s:7:14 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=INNER-X %s
 // INNER-X: DeclRefExpr=x:3:5
+
+void testTypedRecoveryExpr2() {
+  // Inner foo() is a RecoveryExpr (with int type), outer foo() is a "foo(int, int)" call.
+  foo(x, foo(x));
+}
+// RUN: c-index-test -cursor-at=%s:20:3 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=TEST2-OUTER %s
+// TEST2-OUTER: DeclRefExpr=foo:1:5
+// RUN: c-index-test -cursor-at=%s:20:10 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=TEST2-INNER %s
+// TEST2-INNER: OverloadedDeclRef=foo[2:5, 1:5]

clang/test/SemaCXX/enable_if.cpp

template <int N> constexpr int templated() __attribute__((enable_if(N, ""))) {
   return 1;
 }
 
 constexpr int A = templated<0>(); // expected-error{{no matching function for call to 'templated'}} expected-note@-4{{candidate disabled}}
 static_assert(templated<1>() == 1, "");
 
 template <int N> constexpr int callTemplated() { return templated<N>(); }
 
-constexpr int B = 10 + // the carat for the error should be pointing to the problematic call (on the next line), not here.
-    callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-3{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-10{{candidate disabled}}
+constexpr int B = 10 + // expected-error {{initialized by a constant expression}}
+    callTemplated<0>(); // expected-error@-3{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-10{{candidate disabled}} expected-note {{in call to 'callTemplated()'}} expected-note@-3 {{subexpression not valid in a constant expression}}
 static_assert(callTemplated<1>() == 1, "");
 }
 
 namespace variadic {
 void foo(int a, int b = 0, ...) __attribute__((enable_if(a && b, ""))); // expected-note 6{{disabled}}
 
 void testFoo() {
   foo(1, 1);

clang/test/SemaCXX/recovery-expr-type.cpp

+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -frecovery-ast -frecovery-ast-type -o - %s -fsyntax-only -verify
+
+namespace test0 {
+struct Indestructible {
+  // Indestructible();
+  ~Indestructible() = delete; // expected-note {{deleted}}
+};
+Indestructible make_indestructible();
+
+void test() {
+  // no crash.
+  int s = sizeof(make_indestructible()); // expected-error {{deleted}}
+}
+}
+
+namespace test1 {
+constexpr int foo() { return 1; } // expected-note {{candidate function not viable}}
+// verify the "not an integral constant expression" diagnostic is suppressed.
+static_assert(1 == foo(1), ""); // expected-error {{no matching function}}
+}
+
+namespace test2 {
+void foo(); // expected-note 3{{requires 0 arguments}}
+void func() {
+  // verify that "field has incomplete type" diagnostic is suppressed.
+  typeof(foo(42)) var; // expected-error {{no matching function}}
+
+  // FIXME: suppress the "cannot initialize a variable" diagnostic.
+  int a = foo(1); // expected-error {{no matching function}} \
+                  // expected-error {{cannot initialize a variable of type}}
+
+  // FIXME: suppress the "invalid application" diagnostic.
+  int s = sizeof(foo(42)); // expected-error {{no matching function}} \
+                           // expected-error {{invalid application of 'sizeof'}}
+};
+}
+
+namespace test3 {
+template <int N>
+constexpr int templated() __attribute__((enable_if(N, ""))) { // expected-note {{candidate disabled}}
+  return 1;
+}
+// verify that "constexpr variable must be initialized" diagnostic is suppressed.
+constexpr int A = templated<0>(); // expected-error{{no matching function}}
+
+template <typename T>
+struct AA {
+  template <typename U>
+  static constexpr int getB() { // expected-note{{candidate template ignored}}
+    return 2;
+  }
+  static constexpr int foo2() {
+    return AA<T>::getB(); // expected-error{{no matching function for call to 'getB'}} \
+                          // expected-note {{subexpression not valid in a constant expression}}
+  }
+};
+// FIXME: should we suppress the "be initialized by a constant expression" diagnostic?
+constexpr auto x2 = AA<int>::foo2(); // expected-error {{be initialized by a constant expression}} \
+                                     // expected-note {{in instantiation of member function}} \
+                                     // expected-note {{in call to}}
+}

clang/test/SemaTemplate/instantiate-function-params.cpp

 // RUN: %clang_cc1 -triple i686-unknown-unknown -fsyntax-only -verify %s
 
 // PR6619
 template<bool C> struct if_c { };
 template<typename T1> struct if_ {
-  typedef if_c< static_cast<bool>(T1::value)> almost_type_; // expected-note 5{{in instantiation}}
+  typedef if_c< static_cast<bool>(T1::value)> almost_type_; // expected-note 7{{in instantiation}}
 };
 template <class Model, void (Model::*)()> struct wrap_constraints { };
 template <class Model> 
 inline char has_constraints_(Model* , // expected-note 3{{candidate template ignored}}
-                               wrap_constraints<Model,&Model::constraints>* = 0); // expected-note 2{{in instantiation}}
+                               wrap_constraints<Model,&Model::constraints>* = 0); // expected-note 4{{in instantiation}}
 
 template <class Model> struct not_satisfied {
   static const bool value = sizeof( has_constraints_((Model*)0)  == 1); // expected-error 3{{no matching function}} \
-  // expected-note 2{{while substituting deduced template arguments into function template 'has_constraints_' [with }}
+  // expected-note 4{{while substituting deduced template arguments into function template 'has_constraints_' [with }}
 };
 template <class ModelFn> struct requirement_;
 template <void(*)()> struct instantiate {
 };
-template <class Model> struct requirement_<void(*)(Model)>                           : if_<       not_satisfied<Model>         >::type { // expected-note 5{{in instantiation}}
+template <class Model> struct requirement_<void(*)(Model)>                           : if_<       not_satisfied<Model>         >::type { // expected-error 3{{no type named 'type' in}} expected-note 7{{in instantiation}}
 };
 template <class Model> struct usage_requirements {
 };
 template < typename TT > struct InputIterator                            {
-    typedef  instantiate< & requirement_<void(*)(usage_requirements<InputIterator> x)>::failed> boost_concept_check1; // expected-note {{in instantiation}}
+    typedef  instantiate< & requirement_<void(*)(usage_requirements<InputIterator> x)>::failed> boost_concept_check1; // expected-note 2{{in instantiation}}
 };
-template < typename TT > struct ForwardIterator                              : InputIterator<TT>                              { // expected-note {{in instantiation}}
-  typedef instantiate< & requirement_<void(*)(usage_requirements<ForwardIterator> x)>::failed> boost_concept_check2; // expected-note {{in instantiation}}
+template < typename TT > struct ForwardIterator                              : InputIterator<TT>                              { // expected-note 2{{in instantiation}}
+  typedef instantiate< & requirement_<void(*)(usage_requirements<ForwardIterator> x)>::failed> boost_concept_check2; // expected-note 2{{in instantiation}}
 
 };
-typedef instantiate< &requirement_<void(*)(ForwardIterator<char*> x)>::failed> boost_concept_checkX;// expected-note 3{{in instantiation}}
+typedef instantiate< &requirement_<void(*)(ForwardIterator<char*> x)>::failed> boost_concept_checkX;// expected-note 6{{in instantiation}}
 
 template<typename T> struct X0 { };
 template<typename R, typename A1> struct X0<R(A1 param)> { };
 
 template<typename T, typename A1, typename A2>
 void instF0(X0<T(A1)> x0a, X0<T(A2)> x0b) {
   X0<T(A1)> x0c;
   X0<T(A2)> x0d;