diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -6097,21 +6097,25 @@ /// 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()` as part of the +/// By default, 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()` 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. +/// In addition, we will preserve the type in RecoveryExpr when the type is +/// known, e.g. preserving the return type for a broken non-overloaded function +/// call, a overloaded call where all candidates have the same return type. /// /// 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 { public: - static RecoveryExpr *Create(ASTContext &Ctx, SourceLocation BeginLoc, - SourceLocation EndLoc, ArrayRef SubExprs); + static RecoveryExpr *Create(ASTContext &Ctx, QualType T, + SourceLocation BeginLoc, SourceLocation EndLoc, + ArrayRef SubExprs); static RecoveryExpr *CreateEmpty(ASTContext &Ctx, unsigned NumSubExprs); ArrayRef subExpressions() { @@ -6136,8 +6140,8 @@ } private: - RecoveryExpr(ASTContext &Ctx, SourceLocation BeginLoc, SourceLocation EndLoc, - ArrayRef SubExprs); + RecoveryExpr(ASTContext &Ctx, QualType T, SourceLocation BeginLoc, + SourceLocation EndLoc, ArrayRef SubExprs); RecoveryExpr(EmptyShell Empty, unsigned NumSubExprs) : Expr(RecoveryExprClass, Empty), NumExprs(NumSubExprs) {} diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -149,6 +149,7 @@ LANGOPT(DoubleSquareBracketAttributes, 1, 0, "'[[]]' attributes extension for all language standard modes") COMPATIBLE_LANGOPT(RecoveryAST, 1, 0, "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") diff --git a/clang/include/clang/Driver/CC1Options.td b/clang/include/clang/Driver/CC1Options.td --- a/clang/include/clang/Driver/CC1Options.td +++ b/clang/include/clang/Driver/CC1Options.td @@ -565,6 +565,10 @@ 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 { diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3890,7 +3890,8 @@ /// Attempts to produce a RecoveryExpr after some AST node cannot be created. ExprResult CreateRecoveryExpr(SourceLocation Begin, SourceLocation End, - ArrayRef SubExprs); + ArrayRef SubExprs, + QualType T = QualType()); ObjCInterfaceDecl *getObjCInterfaceDecl(IdentifierInfo *&Id, SourceLocation IdLoc, diff --git a/clang/lib/AST/ComputeDependence.cpp b/clang/lib/AST/ComputeDependence.cpp --- a/clang/lib/AST/ComputeDependence.cpp +++ b/clang/lib/AST/ComputeDependence.cpp @@ -485,9 +485,13 @@ } ExprDependence clang::computeDependence(RecoveryExpr *E) { + // Mark the expression as value- and instantiation- dependent to reuse + // existing suppressions for dependent code, e.g. avoiding + // constant-evaluation. // 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::ValueInstantiation | ExprDependence::Error; for (auto *S : E->subExpressions()) D |= S->getDependence(); return D; diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -4681,25 +4681,25 @@ return OriginalTy; } -RecoveryExpr::RecoveryExpr(ASTContext &Ctx, SourceLocation BeginLoc, +RecoveryExpr::RecoveryExpr(ASTContext &Ctx, QualType T, SourceLocation BeginLoc, SourceLocation EndLoc, ArrayRef SubExprs) - : Expr(RecoveryExprClass, Ctx.DependentTy, VK_LValue, OK_Ordinary), - BeginLoc(BeginLoc), EndLoc(EndLoc), NumExprs(SubExprs.size()) { -#ifndef NDEBUG + : Expr(RecoveryExprClass, T, VK_LValue, OK_Ordinary), BeginLoc(BeginLoc), + EndLoc(EndLoc), NumExprs(SubExprs.size()) { + assert(!T.isNull()); for (auto *E : SubExprs) assert(E != nullptr); -#endif llvm::copy(SubExprs, getTrailingObjects()); setDependence(computeDependence(this)); } -RecoveryExpr *RecoveryExpr::Create(ASTContext &Ctx, SourceLocation BeginLoc, +RecoveryExpr *RecoveryExpr::Create(ASTContext &Ctx, QualType T, + SourceLocation BeginLoc, SourceLocation EndLoc, ArrayRef SubExprs) { void *Mem = Ctx.Allocate(totalSizeToAlloc(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) { diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -2890,6 +2890,8 @@ Diags.Report(diag::warn_fe_concepts_ts_flag); Opts.RecoveryAST = Args.hasFlag(OPT_frecovery_ast, OPT_fno_recovery_ast, false); + 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); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -16426,7 +16426,7 @@ } 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. diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -18950,7 +18950,7 @@ } ExprResult Sema::CreateRecoveryExpr(SourceLocation Begin, SourceLocation End, - ArrayRef SubExprs) { + ArrayRef 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. @@ -18960,5 +18960,8 @@ 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); } diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -12776,6 +12776,42 @@ return false; } +// Guess at what the return type for an unresolvable overload should be. +static QualType chooseRecoveryType(OverloadCandidateSet &CS, + OverloadCandidateSet::iterator *Best) { + llvm::Optional 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() @@ -12865,8 +12901,11 @@ } } - // Overload resolution failed. - return ExprError(); + // Overload resolution failed, try to recover. + SmallVector SubExprs = {Fn}; + SubExprs.append(Args.begin(), Args.end()); + return SemaRef.CreateRecoveryExpr(Fn->getBeginLoc(), RParenLoc, SubExprs, + chooseRecoveryType(*CandidateSet, Best)); } static void markUnaddressableCandidatesUnviable(Sema &S, diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp --- a/clang/test/AST/ast-dump-recovery.cpp +++ b/clang/test/AST/ast-dump-recovery.cpp @@ -1,12 +1,13 @@ -// 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); @@ -14,14 +15,15 @@ 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 {{.*}} '' contains-errors // CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} 'bar' // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors int unresolved_call1 = bar(); diff --git a/clang/test/CodeCompletion/member-access.cpp b/clang/test/CodeCompletion/member-access.cpp --- a/clang/test/CodeCompletion/member-access.cpp +++ b/clang/test/CodeCompletion/member-access.cpp @@ -273,3 +273,12 @@ // 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-RECOVERY %s +// CHECK-RECOVERY: [#int#]member diff --git a/clang/test/Index/getcursor-recovery.cpp b/clang/test/Index/getcursor-recovery.cpp --- a/clang/test/Index/getcursor-recovery.cpp +++ b/clang/test/Index/getcursor-recovery.cpp @@ -2,15 +2,24 @@ 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 an unresolved overloaded call, 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 valid "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]